john-bodley commented on code in PR #19941:
URL: https://github.com/apache/superset/pull/19941#discussion_r864431581
##########
superset/reports/commands/execute.py:
##########
@@ -94,35 +94,39 @@ def __init__(
self._start_dttm = datetime.utcnow()
self._execution_id = execution_id
- def set_state_and_log(
+ def update_report_schedule_and_log(
Review Comment:
I felt that `update_report_schedule_and_log` was more reflective of the
function logic.
##########
superset/reports/commands/execute.py:
##########
@@ -94,35 +94,39 @@ def __init__(
self._start_dttm = datetime.utcnow()
self._execution_id = execution_id
- def set_state_and_log(
+ def update_report_schedule_and_log(
self,
state: ReportState,
error_message: Optional[str] = None,
) -> None:
"""
- Updates current ReportSchedule state and TS. If on final state writes
the log
- for this execution
+ Update the report schedule state et al. and reflect the change in the
execution
+ log.
"""
- now_dttm = datetime.utcnow()
- self.set_state(state, now_dttm)
- self.create_log(
- state,
- error_message=error_message,
- )
- def set_state(self, state: ReportState, dttm: datetime) -> None:
+ self.update_report_schedule(state)
+ self.create_log(error_message)
+
+ def update_report_schedule(self, state: ReportState) -> None:
"""
- Set the current report schedule state, on this case we want to
- commit immediately
+ Update the report schedule state et al.
+
+ When the report state is WORKING we must ensure that the values from
the last
+ execution run are cleared to ensure that they are not propagated to the
+ execution log.
"""
+
+ if state == ReportState.WORKING:
+ self._report_schedule.last_value = None
+ self._report_schedule.last_value_row_json = None
+
self._report_schedule.last_state = state
- self._report_schedule.last_eval_dttm = dttm
+ self._report_schedule.last_eval_dttm = datetime.utcnow()
Review Comment:
There's no need to pass in the timestamp which isn't used elsewhere.
##########
superset/reports/commands/execute.py:
##########
@@ -132,7 +136,7 @@ def create_log(
end_dttm=datetime.utcnow(),
value=self._report_schedule.last_value,
value_row_json=self._report_schedule.last_value_row_json,
- state=state,
+ state=self._report_schedule.state,
Review Comment:
There's no need to pass in the state given the report schedule record has
been updated with the new state.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
