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/01 15:12:28 UTC

[GitHub] [airflow] yuqian90 opened a new pull request #7324: [AIRFLOW-6704] Set TaskInstance.operator in constructor

yuqian90 opened a new pull request #7324: [AIRFLOW-6704] Set TaskInstance.operator in constructor
URL: https://github.com/apache/airflow/pull/7324
 
 
   ``TaskInstance.operator`` is currently only set when task is executed. But if a task is marked success or failed, the ``operator`` field is left as None.
   
   This causes bugs when some other code tries to use the ``operator`` field to find the name of the class.
   
   The fix is trivial, just set ``TaskInstance.operator`` in its constructor.
   
   An assertion is also added in ``test_mark_tasks.py`` to test this.
   
   
   ---
   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] nuclearpinguin commented on a change in pull request #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task

Posted by GitBox <gi...@apache.org>.
nuclearpinguin commented on a change in pull request #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task
URL: https://github.com/apache/airflow/pull/7324#discussion_r379829017
 
 

 ##########
 File path: airflow/models/taskinstance.py
 ##########
 @@ -888,13 +899,11 @@ def _run_raw_task(
         from airflow.sensors.base_sensor_operator import BaseSensorOperator
 
         task = self.task
-        self.pool = pool or task.pool
-        self.pool_slots = task.pool_slots
         self.test_mode = test_mode
+        self.refresh_from_task(task, pool_override=pool)
 
 Review comment:
   Is it needed here if we call `self.refresh_from_db` immediately? 

----------------------------------------------------------------
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] kaxil merged pull request #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task

Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task
URL: https://github.com/apache/airflow/pull/7324
 
 
   

----------------------------------------------------------------
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] yuqian90 commented on a change in pull request #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task

Posted by GitBox <gi...@apache.org>.
yuqian90 commented on a change in pull request #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task
URL: https://github.com/apache/airflow/pull/7324#discussion_r379829531
 
 

 ##########
 File path: airflow/models/taskinstance.py
 ##########
 @@ -888,13 +899,11 @@ def _run_raw_task(
         from airflow.sensors.base_sensor_operator import BaseSensorOperator
 
         task = self.task
-        self.pool = pool or task.pool
-        self.pool_slots = task.pool_slots
         self.test_mode = test_mode
+        self.refresh_from_task(task, pool_override=pool)
 
 Review comment:
   This is mostly trying to preserve the existing behavior and also move some duplicated code into `refresh_from_task()`. However, you are right that this part is not perfect:
   
   Ideally we should first call `refresh_from_db()` and then call `refresh_from_task()`. The call to `refresh_from_db()` is to load those **cumulative** values such as `self.try_number` and `self.max_tries` from db so that individual runs of the task can increment these numbers. The call to `refresh_from_task()` is to get those configurable values from the latest DAG definition. However at the moment `refresh_from_db()` is loading both cumulative values and configurable attributes. So it also sets configurable values such as `self.queue` and `self.operator` which are most likely more useful to be read from DAG definition via `refresh_task()`. 
   
   This PR is not trying to fix everything. It only consolidate some duplicated code and make attributes such as `self.queue` and `self.pool` update-able when tasks are cleared in `clear_task_instances()`. It's probably worth a separate and bigger PR to make sure `refresh_from_db()` is only reading those attributes that really should come from db and leave other attributes to `refresh_from_task()`.

----------------------------------------------------------------
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] yuqian90 commented on issue #7324: [AIRFLOW-6704] Set TaskInstance.operator in constructor

Posted by GitBox <gi...@apache.org>.
yuqian90 commented on issue #7324: [AIRFLOW-6704] Set TaskInstance.operator in constructor
URL: https://github.com/apache/airflow/pull/7324#issuecomment-581137326
 
 
   > Should we remove the other place where it was set?
   
   I think we should keep them unless it's obvious some of them are redundant. From what I can tell, the constructor `__init__()` is not the only way a `TaskInstance` can be created. E.g. sometimes `TaskInstance.task` is not set when the `TaskInstance` is created from deserialization or be read from database via sqlalchemy.
   
   One place I think it should be kept is in `_run_raw_task`. If the DAG author changes the type of an existing task that has been executed before to another class, without changing the task_id, we probably want the next execution of the task to update the `TaskInstance.operator` to the new type.
   
   @ashb  what do you think?

----------------------------------------------------------------
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 #7324: [AIRFLOW-6704] Set TaskInstance.operator in constructor

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #7324: [AIRFLOW-6704] Set TaskInstance.operator in constructor
URL: https://github.com/apache/airflow/pull/7324#issuecomment-581094712
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7324?src=pr&el=h1) Report
   > Merging [#7324](https://codecov.io/gh/apache/airflow/pull/7324?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/39375c01ced008a715bb2a540df31fe2cf908bbb?src=pr&el=desc) will **increase** coverage by `0.23%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7324/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/7324?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #7324      +/-   ##
   ==========================================
   + Coverage   85.59%   85.83%   +0.23%     
   ==========================================
     Files         863      863              
     Lines       40520    40521       +1     
   ==========================================
   + Hits        34685    34780      +95     
   + Misses       5835     5741      -94
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7324?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/models/taskinstance.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvdGFza2luc3RhbmNlLnB5) | `94.72% <100%> (ø)` | :arrow_up: |
   | [airflow/jobs/scheduler\_job.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL3NjaGVkdWxlcl9qb2IucHk=) | `89.34% <0%> (+0.14%)` | :arrow_up: |
   | [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.88% <0%> (+0.28%)` | :arrow_up: |
   | [airflow/hooks/dbapi\_hook.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9kYmFwaV9ob29rLnB5) | `91.73% <0%> (+0.82%)` | :arrow_up: |
   | [airflow/models/connection.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvY29ubmVjdGlvbi5weQ==) | `77.4% <0%> (+0.96%)` | :arrow_up: |
   | [airflow/providers/apache/hive/hooks/hive.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvaG9va3MvaGl2ZS5weQ==) | `77.55% <0%> (+1.53%)` | :arrow_up: |
   | [airflow/providers/mysql/operators/mysql.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbXlzcWwvb3BlcmF0b3JzL215c3FsLnB5) | `100% <0%> (+45%)` | :arrow_up: |
   | [airflow/security/kerberos.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9zZWN1cml0eS9rZXJiZXJvcy5weQ==) | `76.08% <0%> (+45.65%)` | :arrow_up: |
   | [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `100% <0%> (+100%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7324?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/7324?src=pr&el=footer). Last update [39375c0...273e79b](https://codecov.io/gh/apache/airflow/pull/7324?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] kaxil commented on issue #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task

Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task
URL: https://github.com/apache/airflow/pull/7324#issuecomment-589646401
 
 
   Thanks @yuqian90 🎉 

----------------------------------------------------------------
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 #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task
URL: https://github.com/apache/airflow/pull/7324#issuecomment-581094712
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7324?src=pr&el=h1) Report
   > Merging [#7324](https://codecov.io/gh/apache/airflow/pull/7324?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/288a50a0c95e14a81763cee214e55dde2feccf42?src=pr&el=desc) will **decrease** coverage by `0.39%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7324/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/7324?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##           master    #7324     +/-   ##
   =========================================
   - Coverage   86.59%   86.19%   -0.4%     
   =========================================
     Files         871      871             
     Lines       40660    40660             
   =========================================
   - Hits        35209    35047    -162     
   - Misses       5451     5613    +162
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7324?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/models/taskinstance.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvdGFza2luc3RhbmNlLnB5) | `94.28% <100%> (ø)` | :arrow_up: |
   | [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `100% <0%> (ø)` | :arrow_up: |
   | [airflow/operators/generic\_transfer.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvZ2VuZXJpY190cmFuc2Zlci5weQ==) | `100% <0%> (ø)` | :arrow_up: |
   | [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
   | [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `50% <0%> (-50%)` | :arrow_down: |
   | [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
   | [airflow/security/kerberos.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9zZWN1cml0eS9rZXJiZXJvcy5weQ==) | `76.08% <0%> (ø)` | :arrow_up: |
   | [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0%> (-45.08%)` | :arrow_down: |
   | [airflow/providers/mysql/operators/mysql.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbXlzcWwvb3BlcmF0b3JzL215c3FsLnB5) | `100% <0%> (ø)` | :arrow_up: |
   | [...roviders/google/cloud/operators/postgres\_to\_gcs.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL29wZXJhdG9ycy9wb3N0Z3Jlc190b19nY3MucHk=) | `52.94% <0%> (-32.36%)` | :arrow_down: |
   | ... and [9 more](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7324?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/7324?src=pr&el=footer). Last update [288a50a...278638d](https://codecov.io/gh/apache/airflow/pull/7324?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] ashb commented on a change in pull request #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task
URL: https://github.com/apache/airflow/pull/7324#discussion_r376704377
 
 

 ##########
 File path: airflow/models/taskinstance.py
 ##########
 @@ -61,6 +61,21 @@
 from airflow.utils.timeout import timeout
 
 
+def refresh_from_task(ti, task, pool_override=None):
+    """
+    Copy the necessary attributes of a TaskInstance using its task.
+    """
+    ti.queue = task.queue
+    ti.pool = pool_override or task.pool
+    ti.pool_slots = task.pool_slots
+    ti.priority_weight = task.priority_weight_total
+    ti.run_as_user = task.run_as_user
+    ti.max_tries = task.retries
+    ti.run_as_user = task.run_as_user
+    ti.executor_config = task.executor_config
 
 Review comment:
   Ping @dimberman - I remember there was a previous issue about this field

----------------------------------------------------------------
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] yuqian90 commented on issue #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task

Posted by GitBox <gi...@apache.org>.
yuqian90 commented on issue #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task
URL: https://github.com/apache/airflow/pull/7324#issuecomment-583727410
 
 
   @ashb  I actually noticed some similar issues for other attributes too. For example, if the `pool` of an existing task is changed in the DAG code, there's no way to clear existing `TaskInstance` and re-run them in the new pool. So I extracted the repeated code that sets `TaskInstance` attributes from task into a function `refresh_from_task()` and used it everywhere.
   
   Please take another look.

----------------------------------------------------------------
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 #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task
URL: https://github.com/apache/airflow/pull/7324#issuecomment-581094712
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7324?src=pr&el=h1) Report
   > Merging [#7324](https://codecov.io/gh/apache/airflow/pull/7324?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/288a50a0c95e14a81763cee214e55dde2feccf42?src=pr&el=desc) will **decrease** coverage by `0.47%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7324/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/7324?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #7324      +/-   ##
   ==========================================
   - Coverage   86.59%   86.11%   -0.48%     
   ==========================================
     Files         871      871              
     Lines       40660    40660              
   ==========================================
   - Hits        35209    35014     -195     
   - Misses       5451     5646     +195
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7324?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/models/taskinstance.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvdGFza2luc3RhbmNlLnB5) | `94.28% <100%> (ø)` | :arrow_up: |
   | [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0%> (-64.16%)` | :arrow_down: |
   | [airflow/operators/generic\_transfer.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvZ2VuZXJpY190cmFuc2Zlci5weQ==) | `100% <0%> (ø)` | :arrow_up: |
   | [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
   | [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `100% <0%> (ø)` | :arrow_up: |
   | [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
   | [airflow/security/kerberos.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9zZWN1cml0eS9rZXJiZXJvcy5weQ==) | `30.43% <0%> (-45.66%)` | :arrow_down: |
   | [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0%> (-45.08%)` | :arrow_down: |
   | [airflow/providers/mysql/operators/mysql.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbXlzcWwvb3BlcmF0b3JzL215c3FsLnB5) | `55% <0%> (-45%)` | :arrow_down: |
   | [...roviders/google/cloud/operators/postgres\_to\_gcs.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL29wZXJhdG9ycy9wb3N0Z3Jlc190b19nY3MucHk=) | `85.29% <0%> (ø)` | :arrow_up: |
   | ... and [9 more](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7324?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/7324?src=pr&el=footer). Last update [288a50a...278638d](https://codecov.io/gh/apache/airflow/pull/7324?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 edited a comment on issue #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task
URL: https://github.com/apache/airflow/pull/7324#issuecomment-581094712
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7324?src=pr&el=h1) Report
   > Merging [#7324](https://codecov.io/gh/apache/airflow/pull/7324?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/288a50a0c95e14a81763cee214e55dde2feccf42?src=pr&el=desc) will **decrease** coverage by `0.38%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7324/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/7324?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##           master   #7324      +/-   ##
   =========================================
   - Coverage   86.59%   86.2%   -0.39%     
   =========================================
     Files         871     871              
     Lines       40660   40661       +1     
   =========================================
   - Hits        35209   35052     -157     
   - Misses       5451    5609     +158
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7324?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/models/taskinstance.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvdGFza2luc3RhbmNlLnB5) | `94.29% <100%> (ø)` | :arrow_up: |
   | [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
   | [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `50% <0%> (-50%)` | :arrow_down: |
   | [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
   | [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0%> (-45.08%)` | :arrow_down: |
   | [...roviders/google/cloud/operators/postgres\_to\_gcs.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL29wZXJhdG9ycy9wb3N0Z3Jlc190b19nY3MucHk=) | `52.94% <0%> (-32.36%)` | :arrow_down: |
   | [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
   | [...viders/cncf/kubernetes/operators/kubernetes\_pod.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvY25jZi9rdWJlcm5ldGVzL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZC5weQ==) | `70.21% <0%> (-23.41%)` | :arrow_down: |
   | [airflow/providers/postgres/hooks/postgres.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvaG9va3MvcG9zdGdyZXMucHk=) | `77.46% <0%> (-16.91%)` | :arrow_down: |
   | [airflow/hooks/dbapi\_hook.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9kYmFwaV9ob29rLnB5) | `90.08% <0%> (-1.66%)` | :arrow_down: |
   | ... and [2 more](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7324?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/7324?src=pr&el=footer). Last update [288a50a...cb355ca](https://codecov.io/gh/apache/airflow/pull/7324?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] yuqian90 commented on a change in pull request #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task

Posted by GitBox <gi...@apache.org>.
yuqian90 commented on a change in pull request #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task
URL: https://github.com/apache/airflow/pull/7324#discussion_r376715983
 
 

 ##########
 File path: airflow/models/taskinstance.py
 ##########
 @@ -61,6 +61,21 @@
 from airflow.utils.timeout import timeout
 
 
+def refresh_from_task(ti, task, pool_override=None):
+    """
+    Copy the necessary attributes of a TaskInstance using its task.
+    """
+    ti.queue = task.queue
+    ti.pool = pool_override or task.pool
+    ti.pool_slots = task.pool_slots
+    ti.priority_weight = task.priority_weight_total
+    ti.run_as_user = task.run_as_user
+    ti.max_tries = task.retries
+    ti.run_as_user = task.run_as_user
+    ti.executor_config = task.executor_config
 
 Review comment:
   I found the PR by @dimberman . https://github.com/apache/airflow/pull/5926/files
   
   I think the issue was that @dimberman  wants ``TaskInstance`` to use the latest ``executor_config`` from the `Task`, instead of reading it from db. Since no where else we set `refresh_executor_config` to True, so what I'm doing here will not affect the current behavior for `executor_config`.
   
   However, that said, it's a little strange there's a `refresh_executor_config` default to `False` but not being used anywhere. Should we just remove the `refresh_executor_config` flag since that's always what we are doing? But that's not related to what I'm doing here. Just a suggestion.

----------------------------------------------------------------
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 #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task
URL: https://github.com/apache/airflow/pull/7324#issuecomment-581094712
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7324?src=pr&el=h1) Report
   > Merging [#7324](https://codecov.io/gh/apache/airflow/pull/7324?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/288a50a0c95e14a81763cee214e55dde2feccf42?src=pr&el=desc) will **decrease** coverage by `0.47%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7324/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/7324?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #7324      +/-   ##
   ==========================================
   - Coverage   86.59%   86.11%   -0.48%     
   ==========================================
     Files         871      871              
     Lines       40660    40661       +1     
   ==========================================
   - Hits        35209    35017     -192     
   - Misses       5451     5644     +193
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7324?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/models/taskinstance.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvdGFza2luc3RhbmNlLnB5) | `94.29% <100%> (ø)` | :arrow_up: |
   | [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0%> (-64.16%)` | :arrow_down: |
   | [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
   | [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
   | [airflow/security/kerberos.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9zZWN1cml0eS9rZXJiZXJvcy5weQ==) | `30.43% <0%> (-45.66%)` | :arrow_down: |
   | [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0%> (-45.08%)` | :arrow_down: |
   | [airflow/providers/mysql/operators/mysql.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbXlzcWwvb3BlcmF0b3JzL215c3FsLnB5) | `55% <0%> (-45%)` | :arrow_down: |
   | [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
   | [...viders/cncf/kubernetes/operators/kubernetes\_pod.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvY25jZi9rdWJlcm5ldGVzL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZC5weQ==) | `70.21% <0%> (-23.41%)` | :arrow_down: |
   | [airflow/providers/apache/hive/hooks/hive.py](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvaG9va3MvaGl2ZS5weQ==) | `76.02% <0%> (-1.54%)` | :arrow_down: |
   | ... and [4 more](https://codecov.io/gh/apache/airflow/pull/7324/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7324?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/7324?src=pr&el=footer). Last update [288a50a...cb355ca](https://codecov.io/gh/apache/airflow/pull/7324?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] ashb commented on a change in pull request #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #7324: [AIRFLOW-6704] Copy common TaskInstance attributes from Task
URL: https://github.com/apache/airflow/pull/7324#discussion_r376704395
 
 

 ##########
 File path: airflow/models/taskinstance.py
 ##########
 @@ -61,6 +61,21 @@
 from airflow.utils.timeout import timeout
 
 
+def refresh_from_task(ti, task, pool_override=None):
 
 Review comment:
   This should be a method on TaskInstsnce, not a standalone function

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