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.

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

Add Skip First Run option #8

Merged
merged 4 commits into from
Aug 13, 2017

Conversation

ChaoticBoredom
Copy link

Working w/ clockwork on Heroku, we often want to delay the first run of some jobs on deploys. Usually things like metric collection, particularly those triggering alerts. If they are run at the moment of deploy, they return bad data.
We'd like to delay the first run of these alerts for a single cycle.
I'm putting this PR up, not sure how in demand this particular feature is, but I'm open to alternatives if this isn't the route the maintainer wants to go.

@mikker
Copy link

mikker commented May 12, 2017

Any plans to merge this? Been using it for a while with no trouble.

@MarcLapierre
Copy link

MarcLapierre commented May 23, 2017

👍 I would love to see this merged. It was not immediately obvious that all jobs would be run on deploy even if at is set. This is causing issues for us as we have a lot of staggered high intensity jobs that we do not want to run on each deploy as we deploy several times per day.

@MarcLapierre
Copy link

We've tried this out over the last few days and we have found the following issues:

  • This breaks clockwork-test. Calling Clockwork::Test.run(max_ticks: 1) causes this line to lockup forever.

  • Any scheduled jobs that use at: will never trigger.

@ChaoticBoredom
Copy link
Author

Thanks Marc, will look into fixes for those issues later today :)

@kevin-j-m
Copy link

Maintainer of clockwork-test here. I'm on vacation, so I can't take a look now at the correlation that causes the gem to fail with this change.

However, that's on us to figure out & maintain, so don't let it hold up development here. PRs or advise on maintaining compatibility are always welcome!

Thanks for taking a look at it @MarcLapierre.

@MarcLapierre
Copy link

Hey guys -- I am looking for a follow-up here. Are we looking at resolving and merging this?

@ChaoticBoredom
Copy link
Author

@kevin-j-m thanks, I misread that comment, thought it was breaking tests in the main clockwork gem. I'm not sure how to move this forward at this point :/

@Rykian
Copy link
Owner

Rykian commented Jun 28, 2017

@ChaoticBoredom Could you add some documentation to the README ? Thanks ;)

@ChaoticBoredom
Copy link
Author

@Rykian sorry for the delay, updated the README, let me know if more detail is needed, or if there's a section I've missed :)

@mikker
Copy link

mikker commented Jul 23, 2017

Let's go! ☺️

@Rykian Rykian merged commit 1c8146a into Rykian:master Aug 13, 2017
@MarcLapierre
Copy link

Hey @ChaoticBoredom -- Did we ever resolve the issue around Any scheduled jobs that use at: will never trigger? I don't see any commits regarding that issue.

@ChaoticBoredom
Copy link
Author

@MarcLapierre sorry for the delay, that was not addressed. Also, until today I misunderstood how clockwork worked using :at, so I'll put something up to handle that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants