You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/08/17 02:29:48 UTC

[GitHub] [airflow] LionelZhao28 commented on a change in pull request #17502: localize the run_id in dagrun

LionelZhao28 commented on a change in pull request #17502:
URL: https://github.com/apache/airflow/pull/17502#discussion_r689988346



##########
File path: airflow/models/dagrun.py
##########
@@ -286,7 +287,17 @@ def find(
     @staticmethod
     def generate_run_id(run_type: DagRunType, execution_date: datetime) -> str:
         """Generate Run ID based on Run Type and Execution Date"""
-        return f"{run_type}__{execution_date.isoformat()}"
+        from airflow.configuration import conf
+        local_run_id = conf.getboolean("core", "localize_dag_run_id", fallback=False)
+        if local_run_id:
+            local_time = pendulum.instance(execution_date).astimezone(tz=settings.TIMEZONE)
+            is_dst = local_time.is_dst()
+            if is_dst:
+                return f"{run_type}__{local_time.isoformat()}__DST"
+            else:
+                return f"{run_type}__{local_time.isoformat()}"
+        else:
+            return f"{run_type}__{execution_date.isoformat()}"

Review comment:
       Sure. Post the checks before I determine changing these
   
   As I checked the source codes, there may be two risks after changing these codes:
   1. As there is a UNIQUE_KEY(dag_id,run_id) in the database, we must make sure that the two values are unique. 
   1. If the user changes the server config `localize_dag_run_id` from `True` to `False` or they change the `default_timezone` frequently, the dag `run_id` may be conflict. 
   
   For the first risk issue, I use `pendulum` to make sure the local_times are unique during the DST changing. And I tried to did the full test and add the test codes into PR.
   For the second issue, I think it is only a low-rate case. Even the user changes the `default_timezone`, the dag `run_id` is accurate to six decimal places, it is really hard to make the `run_id` conflict.




-- 
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: commits-unsubscribe@airflow.apache.org

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