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 2020/05/18 14:44:28 UTC

[GitHub] [airflow] realradical opened a new pull request #8902: [AIRFLOW-8875] fix Dag Run UI execution date with timezone cannot be saved issue

realradical opened a new pull request #8902:
URL: https://github.com/apache/airflow/pull/8902


   This PR is to address the following bug: https://github.com/apache/airflow/issues/8842
   Airflow 1.10.10 displays datetime with timezone on the web UI. Currently, when user tries to create a dag run on the UI, it gives invalid DateTime error which prevents the dag run being created.
   ![image](https://user-images.githubusercontent.com/41271167/82059578-36fb8180-9694-11ea-8a60-74d1afa889ed.png)
   
   
   ---
   Make sure to mark the boxes below before creating PR: [x]
   
   - [x] Description above provides context of the change
   - [x] Unit tests coverage for changes (not needed for documentation changes)
   - [x] Target Github ISSUE in description if exists
   - [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
   - [x] Relevant documentation is updated including usage instructions.
   - [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
   
   ---
   In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
   Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] realradical commented on pull request #8902: [AIRFLOW-8902] fix Dag Run UI execution date with timezone cannot be saved issue

Posted by GitBox <gi...@apache.org>.
realradical commented on pull request #8902:
URL: https://github.com/apache/airflow/pull/8902#issuecomment-630230910


   @ashb sorry, accidentally closed https://github.com/apache/airflow/pull/8875
   
   added the logic to parse datetime without TZ, and more test cases, please check if it is good


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] ashb merged pull request #8902: [AIRFLOW-8902] fix Dag Run UI execution date with timezone cannot be saved issue

Posted by GitBox <gi...@apache.org>.
ashb merged pull request #8902:
URL: https://github.com/apache/airflow/pull/8902


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] ashb commented on a change in pull request #8902: [AIRFLOW-8902] fix Dag Run UI execution date with timezone cannot be saved issue

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #8902:
URL: https://github.com/apache/airflow/pull/8902#discussion_r427276243



##########
File path: dev/airflow-jira
##########
@@ -20,11 +20,11 @@
 # This tool is based on the Spark merge_spark_pr script:
 # https://github.com/apache/spark/blob/master/dev/merge_spark_pr.py
 
-from collections import defaultdict, Counter
-
-import jira
 import re
 import sys
+from collections import Counter, defaultdict
+
+import jira

Review comment:
       Could you revert the change to this file please as it's unrelated




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] realradical commented on pull request #8902: [AIRFLOW-8902] fix Dag Run UI execution date with timezone cannot be saved issue

Posted by GitBox <gi...@apache.org>.
realradical commented on pull request #8902:
URL: https://github.com/apache/airflow/pull/8902#issuecomment-630828287


   > At the end of airflow/www/utils.py we have
   > 
   > ```
   > 
   > # This class is used directly (i.e. we cant tell Fab to use a different
   > # subclass) so we have no other option than to edit the converstion table in
   > # place
   > FieldConverter.conversion_table = (
   >     (('is_utcdatetime', DateTimeField, AirflowDateTimePickerWidget),) +
   >     FieldConverter.conversion_table
   > )
   > ```
   > 
   > That will be the field that SQLA uses for DT fields -- we should update that to use this new class too.
   
   good catch!


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] boring-cyborg[bot] commented on pull request #8902: [AIRFLOW-8902] fix Dag Run UI execution date with timezone cannot be saved issue

Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #8902:
URL: https://github.com/apache/airflow/pull/8902#issuecomment-633564443


   Awesome work, congrats on your first merged pull request!
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] realradical commented on a change in pull request #8902: [AIRFLOW-8902] fix Dag Run UI execution date with timezone cannot be saved issue

Posted by GitBox <gi...@apache.org>.
realradical commented on a change in pull request #8902:
URL: https://github.com/apache/airflow/pull/8902#discussion_r427995254



##########
File path: airflow/www/forms.py
##########
@@ -17,25 +17,66 @@
 # under the License.
 
 import json
+from datetime import datetime as dt
 from operator import itemgetter
 
+import pendulum
 from flask_appbuilder.fieldwidgets import (
     BS3PasswordFieldWidget, BS3TextAreaFieldWidget, BS3TextFieldWidget, Select2Widget,
 )
 from flask_appbuilder.forms import DynamicForm
 from flask_babel import lazy_gettext
 from flask_wtf import FlaskForm
-from wtforms import validators
+from wtforms import validators, widgets
 from wtforms.fields import (
-    BooleanField, DateTimeField, IntegerField, PasswordField, SelectField, StringField, TextAreaField,
+    BooleanField, DateTimeField, Field, IntegerField, PasswordField, SelectField, StringField, TextAreaField,
 )
 
+from airflow.configuration import conf
 from airflow.models import Connection
 from airflow.utils import timezone
 from airflow.www.validators import ValidJson
 from airflow.www.widgets import AirflowDateTimePickerWidget
 
 
+class DateTimeWithTimezoneField(Field):
+    """
+    A text field which stores a `datetime.datetime` matching a format.
+    """
+    widget = widgets.TextInput()
+
+    def __init__(self, label=None, validators=None, format='%Y-%m-%d %H:%M:%S%Z', **kwargs):
+        super(DateTimeWithTimezoneField, self).__init__(label, validators, **kwargs)
+        self.format = format
+
+    def _value(self):
+        if self.raw_data:
+            return ' '.join(self.raw_data)
+        else:
+            return self.data and self.data.strftime(self.format) or ''
+
+    def process_formdata(self, valuelist):
+        if valuelist:
+            date_str = ' '.join(valuelist)
+            try:
+                # Check if the datetime string is in the format without timezone, if so convert it to the
+                # default timezone
+                if len(date_str) == 19:
+                    parsed_datetime = dt.strptime(date_str, '%Y-%m-%d %H:%M:%S')
+                    defualt_timezone = pendulum.timezone('UTC')
+                    tz = conf.get("core", "default_timezone")
+                    if tz == "system":
+                        defualt_timezone = pendulum.local_timezone()
+                    else:
+                        defualt_timezone = pendulum.timezone(tz)
+                    self.data = defualt_timezone.convert(parsed_datetime)
+                else:
+                    self.data = pendulum.parse(date_str)
+            except ValueError:
+                self.data = None
+                raise ValueError(self.gettext('Not a valid datetime value'))
+
+
 class DateTimeForm(FlaskForm):
     # Date filter form needed for task views
     execution_date = DateTimeField(

Review comment:
       good catch, do you think it is good now? @ashb 




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] ashb commented on a change in pull request #8902: [AIRFLOW-8902] fix Dag Run UI execution date with timezone cannot be saved issue

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #8902:
URL: https://github.com/apache/airflow/pull/8902#discussion_r427276653



##########
File path: airflow/www/forms.py
##########
@@ -17,25 +17,66 @@
 # under the License.
 
 import json
+from datetime import datetime as dt
 from operator import itemgetter
 
+import pendulum
 from flask_appbuilder.fieldwidgets import (
     BS3PasswordFieldWidget, BS3TextAreaFieldWidget, BS3TextFieldWidget, Select2Widget,
 )
 from flask_appbuilder.forms import DynamicForm
 from flask_babel import lazy_gettext
 from flask_wtf import FlaskForm
-from wtforms import validators
+from wtforms import validators, widgets
 from wtforms.fields import (
-    BooleanField, DateTimeField, IntegerField, PasswordField, SelectField, StringField, TextAreaField,
+    BooleanField, DateTimeField, Field, IntegerField, PasswordField, SelectField, StringField, TextAreaField,
 )
 
+from airflow.configuration import conf
 from airflow.models import Connection
 from airflow.utils import timezone
 from airflow.www.validators import ValidJson
 from airflow.www.widgets import AirflowDateTimePickerWidget
 
 
+class DateTimeWithTimezoneField(Field):
+    """
+    A text field which stores a `datetime.datetime` matching a format.
+    """
+    widget = widgets.TextInput()
+
+    def __init__(self, label=None, validators=None, format='%Y-%m-%d %H:%M:%S%Z', **kwargs):
+        super(DateTimeWithTimezoneField, self).__init__(label, validators, **kwargs)
+        self.format = format
+
+    def _value(self):
+        if self.raw_data:
+            return ' '.join(self.raw_data)
+        else:
+            return self.data and self.data.strftime(self.format) or ''
+
+    def process_formdata(self, valuelist):
+        if valuelist:
+            date_str = ' '.join(valuelist)
+            try:
+                # Check if the datetime string is in the format without timezone, if so convert it to the
+                # default timezone
+                if len(date_str) == 19:
+                    parsed_datetime = dt.strptime(date_str, '%Y-%m-%d %H:%M:%S')
+                    defualt_timezone = pendulum.timezone('UTC')
+                    tz = conf.get("core", "default_timezone")
+                    if tz == "system":
+                        defualt_timezone = pendulum.local_timezone()
+                    else:
+                        defualt_timezone = pendulum.timezone(tz)
+                    self.data = defualt_timezone.convert(parsed_datetime)
+                else:
+                    self.data = pendulum.parse(date_str)
+            except ValueError:
+                self.data = None
+                raise ValueError(self.gettext('Not a valid datetime value'))
+
+
 class DateTimeForm(FlaskForm):
     # Date filter form needed for task views
     execution_date = DateTimeField(

Review comment:
       ```suggestion
       execution_date = DateTimeWithTimezoneField(
   ```
   
   And a few other places in this file too, no?




----------------------------------------------------------------
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:
users@infra.apache.org