Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Make newScheduledExecutor public; move to new class in lifecycle package #109

Closed
sleberknight opened this issue Mar 22, 2021 · 1 comment · Fixed by #112
Closed

Make newScheduledExecutor public; move to new class in lifecycle package #109

sleberknight opened this issue Mar 22, 2021 · 1 comment · Fixed by #112
Assignees
@sleberknight
Labels
new feature A new feature such as a new class, method, package, group of classes, etc.
Milestone
0.8.0

Comments

@sleberknight
Copy link
Member

The existing newScheduledExecutor method (originally named buildScheduledExecutor) in MonitoredJobs is generic and useful enough to make it public. Since it is related to the Dropwizard lifecycle, it should be moved into the lifecycle package in a new class (which will initially contain only this one method).
@sleberknight sleberknight added the new feature A new feature such as a new class, method, package, group of classes, etc. label Mar 22, 2021
@sleberknight sleberknight added this to the 0.8.0 milestone Mar 22, 2021
@sleberknight sleberknight self-assigned this Mar 23, 2021
@sleberknight
Copy link
Member Author

Actually, instead of creating a new class, this can go in the existing StandardLifecycles class which is already in the lifecycle package.

sleberknight added a commit that referenced this issue Mar 23, 2021
@sleberknight
Make newScheduledExecutor public and move to StandardLifecycles
e289caf 
* Move newScheduledExecutor from MonitoredJobs to StandardLifecycles
* Update the javadocs in StandardLifecycles since it is no longer just
  about setting up lifecycle listeners
* Since MonitoredJobsTest and StandardLifecyclesTest had the same code
  to shut down the ScheduledExecutorService, moved that code into a
  new shared test utility, TestExecutors, which ensures the executor
  is gracefully shut down once the test is finished with it

Closes #109
@sleberknight sleberknight mentioned this issue Mar 23, 2021
Merged
chrisrohr pushed a commit that referenced this issue Mar 23, 2021
@sleberknight @chrisrohr
Make newScheduledExecutor public and move to StandardLifecycles
af4176c 
* Move newScheduledExecutor from MonitoredJobs to StandardLifecycles
* Update the javadocs in StandardLifecycles since it is no longer just
  about setting up lifecycle listeners
* Since MonitoredJobsTest and StandardLifecyclesTest had the same code
  to shut down the ScheduledExecutorService, moved that code into a
  new shared test utility, TestExecutors, which ensures the executor
  is gracefully shut down once the test is finished with it

Closes #109
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sleberknight sleberknight

Labels
new feature A new feature such as a new class, method, package, group of classes, etc.
Projects
None yet
Milestone
0.8.0
1 participant
@sleberknight