[GitHub] [superset] eschutho commented on a diff in pull request #19941: fix(reports): Clear last value when state is WORKING

[GitBox <gi...@apache.org>]

eschutho commented on code in PR #19941:
URL: https://github.com/apache/superset/pull/19941#discussion_r868586449


##########
superset/migrations/versions/cbe71abde154_fix_report_schedule_and_log.py:
##########
@@ -0,0 +1,82 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""fix report schedule and execution log
+
+Revision ID: cbe71abde154
+Revises: a9422eeaae74
+Create Date: 2022-05-03 19:39:32.074608
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "cbe71abde154"
+down_revision = "a9422eeaae74"
+
+from alembic import op
+from sqlalchemy import Column, Float, Integer, String, Text
+from sqlalchemy.ext.declarative import declarative_base
+
+from superset import db
+from superset.models.reports import ReportState
+
+Base = declarative_base()
+
+
+class ReportExecutionLog(Base):
+    __tablename__ = "report_execution_log"
+
+    id = Column(Integer, primary_key=True)
+    state = Column(String(50), nullable=False)
+    value = Column(Float)
+    value_row_json = Column(Text)
+
+
+class ReportSchedule(Base):
+    __tablename__ = "report_schedule"
+
+    id = Column(Integer, primary_key=True)
+    last_state = Column(String(50))
+    last_value = Column(Float)
+    last_value_row_json = Column(Text)
+
+
+def upgrade():
+    bind = op.get_bind()
+    session = db.Session(bind=bind)
+
+    for schedule in (
+        session.query(ReportSchedule)
+        .filter(ReportSchedule.last_state == ReportState.WORKING)
+        .all()
+    ):
+        schedule.last_value = None
+        schedule.last_value_row_json = None

Review Comment:
   @john-bodley do you think there's a chance that if there is a report that's in a working state, that we could potentially wind up altering the value after the report has completed? Just based off your migration timing of around a minute and most of these reports take under a minute to run, we may wind up with some reports that wind up with a completed state and no value, i.e., the commit happens here after the state has changed on the report side. Just wondering if it's safer not to change reports that are in progress. 



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org