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

Cron optimizations #8

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Cron optimizations #8

wants to merge 3 commits into from

Conversation

gtozzi
Copy link

@gtozzi gtozzi commented Oct 31, 2016

Hi,

thank you for sharing this script!

I've added a couple of features to have it better run as a cron job. Feel free to merge them if you are interested.

- It is now possible to use "0" as interval to have the script just run once
  and then exit (useful in cron jobs)
- Fixed a bug where script was not sleeping on error before trying again
@ldx
Copy link
Owner

ldx commented Nov 27, 2017

Interesting idea, thanks! Can you rebase this on current master?

Conflicts:
	dbdownload/dbdownload.py
@gtozzi
Copy link
Author

gtozzi commented Nov 27, 2017

It should work now, didn't test it.

@ldx ldx requested a review from coandco December 14, 2017 23:34
@ldx
Copy link
Owner

ldx commented Dec 14, 2017

@coandco LGTM, can you also take a look? Thanks!

Copy link
Collaborator

@coandco coandco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One quick fix, and I think this should be good to go.

self._logger.debug('sleeping for %d seconds' % self.sleep)
time.sleep(self.sleep)
# Done processing delta
return
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the "while True" is in start() now, there's no reason to have a "while True" loop in _monitor. Remove the _monitor while loop, and I think these changes should be good to go.

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.

3 participants