-
Notifications
You must be signed in to change notification settings - Fork 6
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
Scheduled queries google dbm #158
base: dev
Are you sure you want to change the base?
Changes from 6 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -69,6 +69,24 @@ def check_date_range_definition_conformity(start_date: date, end_date: date, dat | |
raise DateDefinitionException("Report end date should be equal or ulterior to report start date.") | ||
|
||
|
||
def check_scheduled_parameters_definition_conformity( | ||
scheduled_start_date: date, scheduled_end_date: date, frequency: str, date_range: str | ||
): | ||
|
||
if not date_range: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From what I see it's almost the same code as check_date_range_definition_conformity, is it possible to call this function from the new one by doing the new tests before calling check_date_range_definition_conformity? It should avoid code duplication There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The logic is not the same for a scheduled report and for a normal report. I could indeed use check_date_range_definition_conformity(scheduled_start_date, scheduled_end_date, None) after my else, but it seems to me that it is a "hack" just to avoid code deduplication that doesn't explain the logic behind the check. What do you think @AlexisVLRT ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems logical to me, thanks for the explanation |
||
raise DateDefinitionException("You must define a date_range for a scheduled report") | ||
elif not frequency: | ||
raise DateDefinitionException("You must define a frequency for a scheduled report") | ||
else: | ||
if not all([scheduled_start_date, scheduled_end_date]): | ||
raise DateDefinitionException("You must define a couple (scheduled-start-date, scheduled-end-date)") | ||
elif scheduled_end_date < scheduled_start_date: | ||
raise DateDefinitionException( | ||
"Report scheduled-end-date should be equal or ulterior to report \ | ||
scheduled-start-date." | ||
) | ||
|
||
|
||
def get_date_start_and_date_stop_from_date_range(date_range: str) -> Tuple[date, date]: | ||
"""Returns date start and date stop based on the date range provided | ||
and the current date. | ||
|
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.
Could you add this new option to the documentation ? A small guide on how to use sphinx is available here.
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.
Yes thank you