potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375462594
##########
tests/operators/test_python.py:
##########
@@ -844,17 +848,30 @@ def f(exit_code):
assert ti.state == expected_state
+venv_cache_path = tempfile.mkdtemp(prefix="venv_cache_path")
Review Comment:
PythonVirtualenv/ExternalPython tests account for vast majority of the tests
in Operator - mainly because we have to start new Python interpreters for every
such tests but also because those tests are pretty comprehensive.
I changed the approach for those tests to get the Virtualenv tests a "bit"
(~20%) faster overall by swtiching the Venv tests to use caching by default. I
also have a few tests where caching is disabled so we are testing that case as
well, but in most cases we are not really loosing anything. Each distinct venv
(as determined by hash of requiresments and other parameters in the Caching PR
in #33355 by @jens-scheffler-bosch will be created at least at the first test
that uses it - but then that step will be effectively skipped for every other
test that uses same virtualenv specification.
The nice thing about that approach is that thanks to that I found and fixed
a very nasty edge case #35252 - so it turned out that "caching in testing by
default" is a good idea, as "caching" case is potentially more complex and has
more edge cases than non-caching one ("you know cache invalidation is the most
difficult thing in programming"). This #35252 was actually the very
manifestation of cache invalidation not being performed :)
--
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-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
