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

New command check-collections #29

Merged
merged 2 commits into from
Jan 24, 2019
Merged

New command check-collections #29

merged 2 commits into from
Jan 24, 2019

Conversation

odscjames
Copy link

#18

@odscjames odscjames self-assigned this Jan 22, 2019
@odscjames odscjames requested a review from kindly January 22, 2019 12:56
@odscjames
Copy link
Author

@kindly , this builds on #18

As noted in #18 , "Checks and Transforms are currently run by the check-collection and transform-collection command. This is clunky as you have to run that command by hand, and you have to run it for each collection individually."

This adds a new check-collections command. The idea is this can be run from a cron entry - I won't say more on that here as hopefully the docs page I added is clear enough on that.

I'm not saying this is the final form a solution to #18 will take - we talked about message ques before - but this is a relatively simple fix that means people don't have to log in and run the 'check-collection' command by hand any more. That's:
A) A good "improving one step at a time" thing for us for now.
B) If we do do something with message ques later but someone else using the software doesn't want to install those, this is a ok solution they can use instead.

If your happy with this, I'll do something similar and make a "check-transforms" command too.

Copy link

@kindly kindly left a comment

Choose a reason for hiding this comment

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

I am fairly happy about this.

I would like the advise to be clearer that we do not expect a check to take more than a minute so that it is safe to run a cron one minute after the time you set.

I think killing the process a minute after you specify, so it could be good to add something horrible like the below set 1 minute after the specified time.
https://stackoverflow.com/questions/20775624/end-python-code-after-60-seconds#answers-header
as that would give more assurance the the other job was definitely over.

@odscjames
Copy link
Author

Yeah, I'm happy to update docs and add the killer thing

@odscjames odscjames merged commit 3110882 into master Jan 24, 2019
@odscjames odscjames deleted the cron branch January 24, 2019 11:54
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.

2 participants