-
Notifications
You must be signed in to change notification settings - Fork 14.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix task_instance_mutation_hook #9910
fix task_instance_mutation_hook #9910
Conversation
(cherry picked from commit 021bb88bb82adf54b2299f723db0dc4d88fb6ab9)
Python 2.7 is no longer supported in Pygments 2.6.x
* add git sync sidecars * add a helm test * add more tests * allow users to provide git username and pass via a k8s secrets * set default values for airflow worker repository & tag * change ci timeout * fix link * add credentials_secret to airflow.cfg configmap * set GIT_SYNC_ADD_USER on kubernetes worker pods, set uid * add fsGroup to webserver and kubernete workers * move gitSync to dags.gitSync * rename valueFields * turn off git sync and dag persistence by default * provide option to specify known_hosts * add git-sync details into the chart documentation * Update .gitignore Co-authored-by: Ash Berlin-Taylor <[email protected]> * make git sync max failures configurable * Apply suggestions from code review Co-authored-by: Jarek Potiuk <[email protected]> * add back requirements.lock Co-authored-by: Ash Berlin-Taylor <[email protected]> Co-authored-by: Jarek Potiuk <[email protected]> (cherry picked from commit d93555b)
…9816) Rather than only allowing specific pre-determined config settings, this change allows the user to place _any_ config setting they like in the generated airflow.cfg, including overwriting the "generated defaults". This providers a nicer interface for the users of the chart (even if the could already set these via the env vars). (cherry picked from commit e4790d5)
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst)
|
cc @milton0825 @ashb |
Looks like static check is failing:
|
that is odd, I don't think it makes any difference. The module is always fully imported into the sys.modules mapping |
@kaxil In # airflow/__init__.py > airflow/models/__init__.py > airflow/models/dagrun.py
from airflow.settings import task_instance_mutation_hook will load |
This is good finding! Do you mind look into the test failures? I assume they are related to the change. |
@milton0825 I'm checking them now (Static checks is passing now) |
when you run
Line 41 in 4eddce2
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running any airflow
cli command calls
Line 41 in 4eddce2
settings.initialize() |
so I don't see the change in this PR making it any different.
This issue is happening with 1.10.11 - not related to master branch. from airflow import utils
from airflow import settings
from airflow.configuration import conf
from airflow.models import DAG
from flask_admin import BaseView
from importlib import import_module
from airflow.exceptions import AirflowException
settings.initialize() it will do a bunch of imports before calling initialize. These import will reach |
Should we target the change with |
Ok, I'll create a new PR for v1-10-test. |
That is a very good point, in that case the correct solution for v1-10-test would be to move the WDYT @waleedsamy @milton0825 ? |
+1 |
@kaxil Ok, I'll move |
Thanks, really appreciate the fix. Can you also please add a comment in that file on why the import line is below Also pylint or isort might complain, so you might have to add some |
I'll, thanks guys :) |
Awesome work, congrats on your first merged pull request! |
Fixes #9902.
Importing task_instance_mutation_hook will keep the default impl. To fix this I updated Dagrun model to import settings and then later call
settings.task_instance_mutation_hook
; The same way used while callingpolicy
function.Make sure to mark the boxes below before creating PR: [x]
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.
Read the Pull Request Guidelines for more information.