-
Notifications
You must be signed in to change notification settings - Fork 201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
report error creating cron #732
base: master
Are you sure you want to change the base?
Conversation
Test Results621 tests +3 611 ✅ +3 7m 42s ⏱️ +3s Results for commit c6b0034. ± Comparison against base commit 735435f. This pull request removes 1 and adds 4 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
Uploaded ArtifactsTo use these artifacts in your Gradle project, paste the following lines in your build.gradle.
|
@@ -790,6 +792,10 @@ public void onJobClusterInitialize(JobClusterProto.InitializeJobClusterRequest i | |||
cronManager = new CronManager(name, getSelf(), jobClusterMetadata.getJobClusterDefinition().getSLA()); | |||
} catch (Exception e) { | |||
logger.warn("Exception initializing cron", e); | |||
getSender().tell(new JobClusterManagerProto.CreateJobClusterResponse( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe separate this catch to known error (SchedulerException) and other error (exception) and use different error code + message.
Context
Currently, initializing a job cluster with an incorrect cron schedule that cannot be parsed by Quartz silently errors with a warning log. We should return a response instead
Checklist
./gradlew build
compiles code correctly./gradlew test
passes all tests