-
-
Notifications
You must be signed in to change notification settings - Fork 73
Checks for last entry in the report script #270
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
Conversation
scripts/3-report/gcs_report.py
Outdated
| if os.path.exists(last_entry) and not args.force: | ||
| LOGGER.info(f"{last_entry} already exists. Script completed") | ||
| return |
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.
The only thing this function does is issue a log message (also the return is redundant, it would return at end of function). It should also cause the script to exit. Instead of using LOGGER, use an exception.
For example:
Lines 44 to 46 in dceb663
| raise QuantifyingException( | |
| f"Processed data already exists for {QUARTER}", 0 | |
| ) |
scripts/3-report/github_report.py
Outdated
| if os.path.exists(last_entry) and not args.force: | ||
| LOGGER.info(f"{last_entry} already exists. Script completed") | ||
| return |
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.
The only thing this function does is issue a log message (also the return is redundant, it would return at end of function). It should also cause the script to exit. Instead of using LOGGER, use an exception.
For example:
Lines 44 to 46 in dceb663
| raise QuantifyingException( | |
| f"Processed data already exists for {QUARTER}", 0 | |
| ) |
scripts/3-report/wikipedia_report.py
Outdated
| if os.path.exists(last_entry) and not args.force: | ||
| LOGGER.info(f"{last_entry} already exists. Script completed") | ||
| return |
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.
The only thing this function does is issue a log message (also the return is redundant, it would return at end of function). It should also cause the script to exit. Instead of using LOGGER, use an exception.
For example:
Lines 44 to 46 in dceb663
| raise QuantifyingException( | |
| f"Processed data already exists for {QUARTER}", 0 | |
| ) |
TimidRobot
left a comment
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.
I like the methodology. Please resolve comments and add a docstring explaining it is checking last created plot.
TimidRobot
left a comment
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.
this looks great!
now that it is all together, i realize that it would also make a good candidate to move to shared.py, but let's handle that in a new PR. I think it can be combined with other work. i'll write up an issue and send it to you.
Fixes
Description
Added a function in shared.py to check if the last thing the script does exist, in order to avoid regeneration of script output.
Checklist
Update index.md).mainormaster).visible errors.
Developer Certificate of Origin
For the purposes of this DCO, "license" is equivalent to "license or public domain dedication," and "open source license" is equivalent to "open content license or public domain dedication."
Developer Certificate of Origin