-
Notifications
You must be signed in to change notification settings - Fork 14.5k
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
Use methodtools.lru_cache
instead of functools.lru_cache
in class methods
#37757
Conversation
77be0ea
to
35a38d8
Compare
217eaa5
to
bab6130
Compare
@potiuk I've also found that airflow/airflow/models/abstractoperator.py Lines 494 to 497 in 8c44bcb
airflow/airflow/models/mappedoperator.py Lines 336 to 341 in 8c44bcb
airflow/airflow/models/taskinstance.py Lines 641 to 643 in fce3a58
airflow/airflow/models/taskinstance.py Lines 674 to 678 in fce3a58
airflow/airflow/models/taskinstance.py Lines 681 to 685 in fce3a58
airflow/airflow/models/taskinstance.py Lines 715 to 718 in fce3a58
airflow/airflow/models/taskinstance.py Lines 727 to 730 in fce3a58
airflow/airflow/utils/task_group.py Lines 588 to 592 in 0a74928
airflow/airflow/utils/weight_rule.py Lines 37 to 41 in f60d458
Lines 336 to 339 in 8c44bcb
|
WeightRule is enum, and members of enum are singletons, so it is pretty safe to use it there, but in the other cases it still looks like a problem |
The problem here with tests that Prod images build do not reflect changes into requirements in |
That's already fixed in #38678 -> I rebased your PR @Taragolis |
(and even better - extraction of the common code added then is in #38682 - ready for review. |
Hehe, I've also rebased it. DOUBLE REBASE 🤣 |
But this failure also show that docker compose tests can't show useful information in case if API failed because it just fail before show logs from the container. I will add workaround for that later on in the separate PR |
Yep we could dump and upload logs as artifacts from all conainers as we do in case of failure in k8s/regular tests |
This is attempt to prevent use
functools.lru_cache
orfunctools.cache
on methods which lead to memory leak, e.g. object can't be garbage collected if it use methods decorated by this decorators.Simple reproducible example
Instead of that better to use use
functools.cached_property
(if it could be a property), ruffB018
rule suggestion or use some third-party library, e.g.methodtools
compat
decoratorairflow.compat.functools.cache
for avoid use it in methods use simple pre-commit check. It is only required until we have Python 3.9 as min version, and do not useairflow.compat.functools.cache
in code base.methodtools.lru_cache
- this one would not breaks public interfaces or behaviour and do no required to rewrite. In another word we have bug fix "Here and Now" instead of think how to rewrite without additional dependency.^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named
{pr_number}.significant.rst
or{issue_number}.significant.rst
, in newsfragments.