[GitHub] [airflow] zhongjiajie commented on a change in pull request #6900: [AIRFLOW-6346] Enhance dag default_view and orientation
zhongjiajie commented on a change in pull request #6900: [AIRFLOW-6346] Enhance dag default_view and orientation URL: https://github.com/apache/airflow/pull/6900#discussion_r378914969 ## File path: airflow/serialization/schema.json ## @@ -93,7 +93,7 @@ "end_date": { "$ref": "#/definitions/datetime" }, "dagrun_timeout": { "$ref": "#/definitions/timedelta" }, "doc_md": { "type" : "string"}, -"_default_view": { "type" : "string"}, +"default_view": { "type" : "string"}, Review comment: > **keep in the old name _default_view** for now Ok, I get it. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #6900: [AIRFLOW-6346] Enhance dag default_view and orientation
zhongjiajie commented on a change in pull request #6900: [AIRFLOW-6346] Enhance dag default_view and orientation URL: https://github.com/apache/airflow/pull/6900#discussion_r374637765 ## File path: airflow/serialization/schema.json ## @@ -93,7 +93,7 @@ "end_date": { "$ref": "#/definitions/datetime" }, "dagrun_timeout": { "$ref": "#/definitions/timedelta" }, "doc_md": { "type" : "string"}, -"_default_view": { "type" : "string"}, +"default_view": { "type" : "string"}, Review comment: I remark here, so we have two ways: * change schema name to `default_view` and make compatible to existing dags * keep in the old name `_default_view` 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #6900: [AIRFLOW-6346] Enhance dag default_view and orientation
zhongjiajie commented on a change in pull request #6900: [AIRFLOW-6346] Enhance dag default_view and orientation URL: https://github.com/apache/airflow/pull/6900#discussion_r374021290 ## File path: airflow/serialization/schema.json ## @@ -93,7 +93,7 @@ "end_date": { "$ref": "#/definitions/datetime" }, "dagrun_timeout": { "$ref": "#/definitions/timedelta" }, "doc_md": { "type" : "string"}, -"_default_view": { "type" : "string"}, +"default_view": { "type" : "string"}, Review comment: Do you mean we should update user's exists schema in databases? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #6900: [AIRFLOW-6346] Enhance dag default_view and orientation
zhongjiajie commented on a change in pull request #6900: [AIRFLOW-6346] Enhance dag default_view and orientation URL: https://github.com/apache/airflow/pull/6900#discussion_r374020451 ## File path: airflow/models/dag.py ## @@ -300,8 +302,16 @@ def __init__( self.max_active_runs = max_active_runs self.dagrun_timeout = dagrun_timeout self.sla_miss_callback = sla_miss_callback -self._default_view = default_view -self.orientation = orientation +if default_view in DEFAULT_VIEW_PRESETS: +self.default_view = default_view +else: +raise AirflowException(f'Invalid values of dag.default_view: only support ' + f'{DEFAULT_VIEW_PRESETS}, but get {default_view}') Review comment: Yeah, I think it work in py3.6 https://www.python.org/dev/peps/pep-0498/ 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #6900: [AIRFLOW-6346] Enhance dag default_view and orientation
zhongjiajie commented on a change in pull request #6900: [AIRFLOW-6346] Enhance dag default_view and orientation URL: https://github.com/apache/airflow/pull/6900#discussion_r37093 ## File path: airflow/models/dag.py ## @@ -296,8 +298,16 @@ def __init__( self.max_active_runs = max_active_runs self.dagrun_timeout = dagrun_timeout self.sla_miss_callback = sla_miss_callback -self._default_view = default_view Review comment: So I think is ok to change to `default_view ` 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #6900: [AIRFLOW-6346] Enhance dag default_view and orientation
zhongjiajie commented on a change in pull request #6900: [AIRFLOW-6346] Enhance dag default_view and orientation URL: https://github.com/apache/airflow/pull/6900#discussion_r363106891 ## File path: airflow/models/dag.py ## @@ -296,8 +298,16 @@ def __init__( self.max_active_runs = max_active_runs self.dagrun_timeout = dagrun_timeout self.sla_miss_callback = sla_miss_callback -self._default_view = default_view Review comment: I change name from `_default_view` -> `default_view` I'm not really sure why we use `self._default_view` as property name before? Prevent cover the Flask `default_view` variable ? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #6900: [AIRFLOW-6346] Enhance dag default_view and orientation
zhongjiajie commented on a change in pull request #6900: [AIRFLOW-6346] Enhance dag default_view and orientation URL: https://github.com/apache/airflow/pull/6900#discussion_r363106891 ## File path: airflow/models/dag.py ## @@ -296,8 +298,16 @@ def __init__( self.max_active_runs = max_active_runs self.dagrun_timeout = dagrun_timeout self.sla_miss_callback = sla_miss_callback -self._default_view = default_view Review comment: I'm not really sure why we use `self._default_view` as property name before? Prevent cover the Flask `default_view` variable ? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services