You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "potiuk (via GitHub)" <gi...@apache.org> on 2023/02/12 16:19:09 UTC
[GitHub] [airflow] potiuk opened a new pull request, #29493: Require explicit flag to remove MyPy cache when running breeze stop
potiuk opened a new pull request, #29493:
URL: https://github.com/apache/airflow/pull/29493
The change #29184 added cleanup of MyPy cache when `breeze stop` was called, but this is a bit too agressive. It will remove the cache always when breeze stop is run and it will prolong (on MacOS by 10s of seconds) the MyPy static checks when it happens.
Since cleaning the cache is only really needed when MyPy cache gets broken (for example when we upgrade MyPy), cleaning it explicitly when you encounter such problem is a better idea.
This PR:
* adds flag to breeze stop to clean the mypy cache (disabled by default)
* adds documentation about that feature in STATIC_CHECKS
* adds hint displayed to the user when mypy check fails
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+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 a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #29493: Require explicit flag to remove MyPy cache when running breeze stop
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #29493:
URL: https://github.com/apache/airflow/pull/29493#issuecomment-1427078265
Well. It's not 'Always' when my version changes. In vast majority of cases cache worked fine when we upgraded mypy. Usually mypy can clean it up all right. It was just one case when some cache was broken few weeks ago that affected literally few people i am aware of. And also the cache could be broken for other reasons (for example when you ctrl-c while mypy doing it's job if you are unlucky/. So this is really 'easy recovery' when things get really, really wrong
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #29493: Require explicit flag to remove MyPy cache when running breeze stop
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #29493:
URL: https://github.com/apache/airflow/pull/29493
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #29493: Require explicit flag to remove MyPy cache when running breeze stop
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #29493:
URL: https://github.com/apache/airflow/pull/29493#issuecomment-1427081269
I am also not even sure if the root cause in this case was related to MyPy upgrade actually - it looked like it could be (because that was around the time when we upgraded), but I cannot be 100% sure 🤷 . It could also be because - for example some typeshed changed or mypy information for our dependencies (and some stored information in cache could be wrong and "missed" by MyPy (and that one would be next to impossible to know where such error gets triggered).
So we do not really have a clear "trigger" for cleaning the cache that we are sure of. We could potentially catch the right exception and clean it up, but this kind of error might result in an exception that we won't be able to detect as "invalid cache" being the root cause.
Best we can do (and I did) is revert to human-in-the-loop and in case we see an error tell the human "look - if you see strange errors, maybe this one will help".
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org