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/02/24 22:47:20 UTC

[GitHub] [airflow] mik-laj opened a new pull request #7527: [AIRFLOW-6907][WIP] Simplify SchedulerJob

mik-laj opened a new pull request #7527: [AIRFLOW-6907][WIP] Simplify SchedulerJob
URL: https://github.com/apache/airflow/pull/7527
 
 
   If we realize that some methods always get one constant value, then their logic can be simplified.
   
   ---
   Issue link: WILL BE INSERTED BY [boring-cyborg](https://github.com/kaxil/boring-cyborg)
   
   Make sure to mark the boxes below before creating PR: [x]
   
   - [ ] Description above provides context of the change
   - [ ] Commit message/PR title starts with `[AIRFLOW-NNNN]`. AIRFLOW-NNNN = JIRA ID<sup>*</sup>
   - [ ] Unit tests coverage for changes (not needed for documentation changes)
   - [ ] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
   - [ ] Relevant documentation is updated including usage instructions.
   - [ ] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
   
   <sup>*</sup> For document-only changes commit message can start with `[AIRFLOW-XXXX]`.
   
   ---
   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


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #7527: [AIRFLOW-6907] Simplify SchedulerJob

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7527: [AIRFLOW-6907] Simplify SchedulerJob
URL: https://github.com/apache/airflow/pull/7527#issuecomment-590717059
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7527?src=pr&el=h1) Report
   > :exclamation: No coverage uploaded for pull request base (`master@d1a3424`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7527/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/7527?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##             master    #7527   +/-   ##
   =========================================
     Coverage          ?   86.57%           
   =========================================
     Files             ?      896           
     Lines             ?    42634           
     Branches          ?        0           
   =========================================
     Hits              ?    36910           
     Misses            ?     5724           
     Partials          ?        0
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7527?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/jobs/scheduler\_job.py](https://codecov.io/gh/apache/airflow/pull/7527/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL3NjaGVkdWxlcl9qb2IucHk=) | `90.28% <100%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7527?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7527?src=pr&el=footer). Last update [d1a3424...37c630d](https://codecov.io/gh/apache/airflow/pull/7527?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io commented on issue #7527: [AIRFLOW-6907] Simplify SchedulerJob

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #7527: [AIRFLOW-6907] Simplify SchedulerJob
URL: https://github.com/apache/airflow/pull/7527#issuecomment-590717059
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7527?src=pr&el=h1) Report
   > :exclamation: No coverage uploaded for pull request base (`master@d1a3424`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7527/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/7527?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##             master    #7527   +/-   ##
   =========================================
     Coverage          ?   86.57%           
   =========================================
     Files             ?      896           
     Lines             ?    42621           
     Branches          ?        0           
   =========================================
     Hits              ?    36900           
     Misses            ?     5721           
     Partials          ?        0
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7527?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/jobs/scheduler\_job.py](https://codecov.io/gh/apache/airflow/pull/7527/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL3NjaGVkdWxlcl9qb2IucHk=) | `90.28% <100%> (ø)` | |
   | [...irflow/providers/google/cloud/hooks/stackdriver.py](https://codecov.io/gh/apache/airflow/pull/7527/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL2hvb2tzL3N0YWNrZHJpdmVyLnB5) | `81.94% <100%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7527?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7527?src=pr&el=footer). Last update [d1a3424...f226ff5](https://codecov.io/gh/apache/airflow/pull/7527?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [airflow] mik-laj commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob
URL: https://github.com/apache/airflow/pull/7527#discussion_r383821345
 
 

 ##########
 File path: airflow/jobs/scheduler_job.py
 ##########
 @@ -1098,37 +1094,20 @@ def _find_executable_task_instances(self, simple_dag_bag, states, session=None):
         TI = models.TaskInstance
         DR = models.DagRun
         DM = models.DagModel
-        ti_query = (
+        task_instances_to_examine = (
             session
             .query(TI)
             .filter(TI.dag_id.in_(simple_dag_bag.dag_ids))
             .outerjoin(
-                DR,
-                and_(DR.dag_id == TI.dag_id, DR.execution_date == TI.execution_date)
+                DR, and_(DR.dag_id == TI.dag_id, DR.execution_date == TI.execution_date)
             )
-            .filter(or_(DR.run_id == None,  # noqa: E711 pylint: disable=singleton-comparison
-                    not_(DR.run_id.like(BackfillJob.ID_PREFIX + '%'))))
+            .filter(or_(DR.run_id.is_(None), not_(DR.run_id.like(BackfillJob.ID_PREFIX + '%'))))
             .outerjoin(DM, DM.dag_id == TI.dag_id)
-            .filter(or_(DM.dag_id == None,  # noqa: E711 pylint: disable=singleton-comparison
-                    not_(DM.is_paused)))
+            .filter(or_(DM.dag_id.is_(None), not_(DM.is_paused)))
 
 Review comment:
   `DM.dag_id == None` => disable=singleton-comparison
   'DM.dag_id.is_(None)` is correct syntax.

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


With regards,
Apache Git Services

[GitHub] [airflow] mik-laj merged pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob

Posted by GitBox <gi...@apache.org>.
mik-laj merged pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob
URL: https://github.com/apache/airflow/pull/7527
 
 
   

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


With regards,
Apache Git Services

[GitHub] [airflow] ashb commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob
URL: https://github.com/apache/airflow/pull/7527#discussion_r383874965
 
 

 ##########
 File path: airflow/jobs/scheduler_job.py
 ##########
 @@ -1098,37 +1094,20 @@ def _find_executable_task_instances(self, simple_dag_bag, states, session=None):
         TI = models.TaskInstance
         DR = models.DagRun
         DM = models.DagModel
-        ti_query = (
+        task_instances_to_examine = (
             session
             .query(TI)
             .filter(TI.dag_id.in_(simple_dag_bag.dag_ids))
             .outerjoin(
-                DR,
-                and_(DR.dag_id == TI.dag_id, DR.execution_date == TI.execution_date)
+                DR, and_(DR.dag_id == TI.dag_id, DR.execution_date == TI.execution_date)
             )
-            .filter(or_(DR.run_id == None,  # noqa: E711 pylint: disable=singleton-comparison
-                    not_(DR.run_id.like(BackfillJob.ID_PREFIX + '%'))))
+            .filter(or_(DR.run_id.is_(None), not_(DR.run_id.like(BackfillJob.ID_PREFIX + '%'))))
             .outerjoin(DM, DM.dag_id == TI.dag_id)
-            .filter(or_(DM.dag_id == None,  # noqa: E711 pylint: disable=singleton-comparison
-                    not_(DM.is_paused)))
+            .filter(or_(DM.dag_id.is_(None), not_(DM.is_paused)))
 
 Review comment:
   (Sorry for not being clear. This comment was not about this PR, but about the other one)

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


With regards,
Apache Git Services

[GitHub] [airflow] ashb commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob
URL: https://github.com/apache/airflow/pull/7527#discussion_r383812108
 
 

 ##########
 File path: airflow/jobs/scheduler_job.py
 ##########
 @@ -1098,37 +1094,20 @@ def _find_executable_task_instances(self, simple_dag_bag, states, session=None):
         TI = models.TaskInstance
         DR = models.DagRun
         DM = models.DagModel
-        ti_query = (
+        task_instances_to_examine = (
             session
             .query(TI)
             .filter(TI.dag_id.in_(simple_dag_bag.dag_ids))
             .outerjoin(
-                DR,
-                and_(DR.dag_id == TI.dag_id, DR.execution_date == TI.execution_date)
+                DR, and_(DR.dag_id == TI.dag_id, DR.execution_date == TI.execution_date)
             )
-            .filter(or_(DR.run_id == None,  # noqa: E711 pylint: disable=singleton-comparison
-                    not_(DR.run_id.like(BackfillJob.ID_PREFIX + '%'))))
+            .filter(or_(DR.run_id.is_(None), not_(DR.run_id.like(BackfillJob.ID_PREFIX + '%'))))
             .outerjoin(DM, DM.dag_id == TI.dag_id)
-            .filter(or_(DM.dag_id == None,  # noqa: E711 pylint: disable=singleton-comparison
-                    not_(DM.is_paused)))
+            .filter(or_(DM.dag_id.is_(None), not_(DM.is_paused)))
 
 Review comment:
   @nuclearpinguin This was an example of where we don't need a noqa anymore -- do you know why you did on your pylint fixes to jobs? Is it because we are ignoring this whole file?

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


With regards,
Apache Git Services

[GitHub] [airflow] nuclearpinguin commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob

Posted by GitBox <gi...@apache.org>.
nuclearpinguin commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob
URL: https://github.com/apache/airflow/pull/7527#discussion_r383920198
 
 

 ##########
 File path: airflow/jobs/scheduler_job.py
 ##########
 @@ -1098,37 +1094,20 @@ def _find_executable_task_instances(self, simple_dag_bag, states, session=None):
         TI = models.TaskInstance
         DR = models.DagRun
         DM = models.DagModel
-        ti_query = (
+        task_instances_to_examine = (
             session
             .query(TI)
             .filter(TI.dag_id.in_(simple_dag_bag.dag_ids))
             .outerjoin(
-                DR,
-                and_(DR.dag_id == TI.dag_id, DR.execution_date == TI.execution_date)
+                DR, and_(DR.dag_id == TI.dag_id, DR.execution_date == TI.execution_date)
             )
-            .filter(or_(DR.run_id == None,  # noqa: E711 pylint: disable=singleton-comparison
-                    not_(DR.run_id.like(BackfillJob.ID_PREFIX + '%'))))
+            .filter(or_(DR.run_id.is_(None), not_(DR.run_id.like(BackfillJob.ID_PREFIX + '%'))))
             .outerjoin(DM, DM.dag_id == TI.dag_id)
-            .filter(or_(DM.dag_id == None,  # noqa: E711 pylint: disable=singleton-comparison
-                    not_(DM.is_paused)))
+            .filter(or_(DM.dag_id.is_(None), not_(DM.is_paused)))
 
 Review comment:
   I don't know, but here we do not run pylint over this file, do we? @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


With regards,
Apache Git Services

[GitHub] [airflow] ashb commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob
URL: https://github.com/apache/airflow/pull/7527#discussion_r383813377
 
 

 ##########
 File path: airflow/providers/google/cloud/hooks/stackdriver.py
 ##########
 @@ -29,7 +29,7 @@
 from google.protobuf.json_format import MessageToDict, MessageToJson, Parse
 from googleapiclient.errors import HttpError
 
-from airflow import AirflowException
+from airflow.exceptions import AirflowException
 
 Review comment:
   Unrelated change here?

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


With regards,
Apache Git Services

[GitHub] [airflow] mik-laj commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob
URL: https://github.com/apache/airflow/pull/7527#discussion_r383830531
 
 

 ##########
 File path: airflow/providers/google/cloud/hooks/stackdriver.py
 ##########
 @@ -29,7 +29,7 @@
 from google.protobuf.json_format import MessageToDict, MessageToJson, Parse
 from googleapiclient.errors import HttpError
 
-from airflow import AirflowException
+from airflow.exceptions import AirflowException
 
 Review comment:
   This change was already in the master, but the PR preview has not been updated.

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


With regards,
Apache Git Services

[GitHub] [airflow] nuclearpinguin commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob

Posted by GitBox <gi...@apache.org>.
nuclearpinguin commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob
URL: https://github.com/apache/airflow/pull/7527#discussion_r383921545
 
 

 ##########
 File path: airflow/jobs/scheduler_job.py
 ##########
 @@ -1098,37 +1094,20 @@ def _find_executable_task_instances(self, simple_dag_bag, states, session=None):
         TI = models.TaskInstance
         DR = models.DagRun
         DM = models.DagModel
-        ti_query = (
+        task_instances_to_examine = (
             session
             .query(TI)
             .filter(TI.dag_id.in_(simple_dag_bag.dag_ids))
             .outerjoin(
-                DR,
-                and_(DR.dag_id == TI.dag_id, DR.execution_date == TI.execution_date)
+                DR, and_(DR.dag_id == TI.dag_id, DR.execution_date == TI.execution_date)
             )
-            .filter(or_(DR.run_id == None,  # noqa: E711 pylint: disable=singleton-comparison
-                    not_(DR.run_id.like(BackfillJob.ID_PREFIX + '%'))))
+            .filter(or_(DR.run_id.is_(None), not_(DR.run_id.like(BackfillJob.ID_PREFIX + '%'))))
             .outerjoin(DM, DM.dag_id == TI.dag_id)
-            .filter(or_(DM.dag_id == None,  # noqa: E711 pylint: disable=singleton-comparison
-                    not_(DM.is_paused)))
+            .filter(or_(DM.dag_id.is_(None), not_(DM.is_paused)))
 
 Review comment:
   It's still in TODO list:
   https://github.com/PolideaInternal/airflow/blob/37c630da636d1ef352273e33fbdb417727914386/scripts/ci/pylint_todo.txt#L11
   
   And I suppose that once I rebase onto new master I will have to fix it in more places...

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


With regards,
Apache Git Services

[GitHub] [airflow] nuclearpinguin commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob

Posted by GitBox <gi...@apache.org>.
nuclearpinguin commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob
URL: https://github.com/apache/airflow/pull/7527#discussion_r383920198
 
 

 ##########
 File path: airflow/jobs/scheduler_job.py
 ##########
 @@ -1098,37 +1094,20 @@ def _find_executable_task_instances(self, simple_dag_bag, states, session=None):
         TI = models.TaskInstance
         DR = models.DagRun
         DM = models.DagModel
-        ti_query = (
+        task_instances_to_examine = (
             session
             .query(TI)
             .filter(TI.dag_id.in_(simple_dag_bag.dag_ids))
             .outerjoin(
-                DR,
-                and_(DR.dag_id == TI.dag_id, DR.execution_date == TI.execution_date)
+                DR, and_(DR.dag_id == TI.dag_id, DR.execution_date == TI.execution_date)
             )
-            .filter(or_(DR.run_id == None,  # noqa: E711 pylint: disable=singleton-comparison
-                    not_(DR.run_id.like(BackfillJob.ID_PREFIX + '%'))))
+            .filter(or_(DR.run_id.is_(None), not_(DR.run_id.like(BackfillJob.ID_PREFIX + '%'))))
             .outerjoin(DM, DM.dag_id == TI.dag_id)
-            .filter(or_(DM.dag_id == None,  # noqa: E711 pylint: disable=singleton-comparison
-                    not_(DM.is_paused)))
+            .filter(or_(DM.dag_id.is_(None), not_(DM.is_paused)))
 
 Review comment:
   I don't know, but here we do not run pylint over this file, do we?

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


With regards,
Apache Git Services

[GitHub] [airflow] ashb commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob
URL: https://github.com/apache/airflow/pull/7527#discussion_r383838670
 
 

 ##########
 File path: airflow/jobs/scheduler_job.py
 ##########
 @@ -1098,37 +1094,20 @@ def _find_executable_task_instances(self, simple_dag_bag, states, session=None):
         TI = models.TaskInstance
         DR = models.DagRun
         DM = models.DagModel
-        ti_query = (
+        task_instances_to_examine = (
             session
             .query(TI)
             .filter(TI.dag_id.in_(simple_dag_bag.dag_ids))
             .outerjoin(
-                DR,
-                and_(DR.dag_id == TI.dag_id, DR.execution_date == TI.execution_date)
+                DR, and_(DR.dag_id == TI.dag_id, DR.execution_date == TI.execution_date)
             )
-            .filter(or_(DR.run_id == None,  # noqa: E711 pylint: disable=singleton-comparison
-                    not_(DR.run_id.like(BackfillJob.ID_PREFIX + '%'))))
+            .filter(or_(DR.run_id.is_(None), not_(DR.run_id.like(BackfillJob.ID_PREFIX + '%'))))
             .outerjoin(DM, DM.dag_id == TI.dag_id)
-            .filter(or_(DM.dag_id == None,  # noqa: E711 pylint: disable=singleton-comparison
-                    not_(DM.is_paused)))
+            .filter(or_(DM.dag_id.is_(None), not_(DM.is_paused)))
 
 Review comment:
   Yeah I'm okay with this. My question is why in #7484 does he have
   
   ```
   models.DagRun.state.is_(None)))  # pylint: disable=no-member
   ```
   
   but we don't need that here?

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


With regards,
Apache Git Services

[GitHub] [airflow] ashb commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob
URL: https://github.com/apache/airflow/pull/7527#discussion_r383813867
 
 

 ##########
 File path: airflow/providers/google/cloud/hooks/stackdriver.py
 ##########
 @@ -29,7 +29,7 @@
 from google.protobuf.json_format import MessageToDict, MessageToJson, Parse
 from googleapiclient.errors import HttpError
 
-from airflow import AirflowException
+from airflow.exceptions import AirflowException
 
 Review comment:
   Other than this, :+1: to the change.

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


With regards,
Apache Git Services

[GitHub] [airflow] mik-laj commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7527: [AIRFLOW-6907] Simplify SchedulerJob
URL: https://github.com/apache/airflow/pull/7527#discussion_r383821345
 
 

 ##########
 File path: airflow/jobs/scheduler_job.py
 ##########
 @@ -1098,37 +1094,20 @@ def _find_executable_task_instances(self, simple_dag_bag, states, session=None):
         TI = models.TaskInstance
         DR = models.DagRun
         DM = models.DagModel
-        ti_query = (
+        task_instances_to_examine = (
             session
             .query(TI)
             .filter(TI.dag_id.in_(simple_dag_bag.dag_ids))
             .outerjoin(
-                DR,
-                and_(DR.dag_id == TI.dag_id, DR.execution_date == TI.execution_date)
+                DR, and_(DR.dag_id == TI.dag_id, DR.execution_date == TI.execution_date)
             )
-            .filter(or_(DR.run_id == None,  # noqa: E711 pylint: disable=singleton-comparison
-                    not_(DR.run_id.like(BackfillJob.ID_PREFIX + '%'))))
+            .filter(or_(DR.run_id.is_(None), not_(DR.run_id.like(BackfillJob.ID_PREFIX + '%'))))
             .outerjoin(DM, DM.dag_id == TI.dag_id)
-            .filter(or_(DM.dag_id == None,  # noqa: E711 pylint: disable=singleton-comparison
-                    not_(DM.is_paused)))
+            .filter(or_(DM.dag_id.is_(None), not_(DM.is_paused)))
 
 Review comment:
   `DM.dag_id == None` => disable=singleton-comparison
   `DM.dag_id.is_(None)` is correct syntax.

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


With regards,
Apache Git Services