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/08/24 01:03:41 UTC

[GitHub] [airflow] potiuk commented on a change in pull request #10498: Make www/views.py pylint compatible

potiuk commented on a change in pull request #10498:
URL: https://github.com/apache/airflow/pull/10498#discussion_r475293510



##########
File path: airflow/www/views.py
##########
@@ -94,54 +95,53 @@ def get_safe_url(url):
     return url_for('Airflow.index')
 
 
-def get_date_time_num_runs_dag_runs_form_data(request, session, dag):
+def get_date_time_num_runs_dag_runs_form_data(the_request, session, dag):
     """Get Execution Data, Base Date & Number of runs from a Request """
-    dttm = request.args.get('execution_date')
-    if dttm:
-        dttm = timezone.parse(dttm)
+    date_time = the_request.args.get('execution_date')
+    if date_time:
+        date_time = timezone.parse(date_time)
     else:
-        dttm = dag.get_latest_execution_date(session=session) or timezone.utcnow()
+        date_time = dag.get_latest_execution_date(session=session) or timezone.utcnow()
 
-    base_date = request.args.get('base_date')
+    base_date = the_request.args.get('base_date')
     if base_date:
         base_date = timezone.parse(base_date)
     else:
         # The DateTimeField widget truncates milliseconds and would loose
         # the first dag run. Round to next second.
-        base_date = (dttm + timedelta(seconds=1)).replace(microsecond=0)
+        base_date = (date_time + timedelta(seconds=1)).replace(microsecond=0)
 
     default_dag_run = conf.getint('webserver', 'default_dag_run_display_number')
-    num_runs = request.args.get('num_runs')
+    num_runs = the_request.args.get('num_runs')
     num_runs = int(num_runs) if num_runs else default_dag_run
 
-    DR = models.DagRun
     drs = (
-        session.query(DR)
+        session.query(DagRun)

Review comment:
       It was not strictly necessarym but it increases the readability IMHO. It shortens the name but it also increases the mental effort to look up TI and DR elsewhere. I think the code is much more readable when you explicitly spell the class that you use rather than use shorter, two letter names. This is the same reason why pylint disallows short variable names and prefers longer alternatives that spell the actual variable name (df/dtm/fn etc.). That's why I put it as the same change.
   
   In general the code IMHO should be optimised for being more readable explicitely then shorter-writeable.




----------------------------------------------------------------
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