+1 to this idea overall. A bit torn on naming it "common_test_code" -- no strong reason for it but names like: `airflow_test_utils` or `airflow_test_shared` sound better to me. No strong objection though.
Thanks & Regards, Amogh Desai On Mon, Feb 17, 2025 at 11:16 PM Jarek Potiuk <ja...@potiuk.com> wrote: > Main reason is that this might avoid duplication and remove ambiguity of > what is being imported. If we keep the same name, we will have to have > something like that: > > a) folder where project is > b) python package we import > > > So ...if we do tests_common, we will have to do: > > tests_common <- folder > \pyproject.toml > \src > \tests_common <- Python package > > > And when you do: > > import tests_common > > Depending on the tooling, PYTHONPATH, whether implicit python packages are > enabled, WEIRD things might happen. We've seen it when we had "smtp" > provider name and it clashed with built-in package and that's why we have > now "unit.smtp", "integrations.smtp" and "system.smtp" as "canonical > imports" to avoid this problem. > > This is similar - importing tests_common in this case might behave > differently if we keep the same name. That was my main motivation to have a > "different" name - whether it's "common_test_code" or something else, does > not matter, it just has to be significantly different. > > Another approach could be what we did in providers, add extra package > inside the project, and import "tests_common" with a package prefix (for > example `airflow_tests`): > > > tests_common <- folder > \pyproject.toml > \src > \airflow_tests <- Python package > \tests_common <- Python package > > But then we would have to change all the tests that import tests_common to: > > from `airflow_tests.tests_common` > > Easy to do, but I wanted to avoid another 1000+ files PR for our poor > reviewers :) > > So basically, I think we have two options: > > a) different "folder" name where we keep the project > b) adding parent package to "tests_common" > > We could do both, I am happy with either approach but maybe we should do a > small unscientific poll, and have others propose better names (you know, > naming is hard :D). > > J > > > pon., 17 lut 2025, 16:43 użytkownik Vincent Beck <vincb...@apache.org> > napisał: > > > Overall +1 on this one. Regarding the naming, why not keeping > > "tests_common" instead of "common_test_code"? I am not a big fan of > > "common_test_code" but it is obviously just a personal opinion (as it is > > always with naming :)) > > > > On 2025/02/16 13:30:09 Jarek Potiuk wrote: > > > > Just wondernig... would an optional dependency not be the right place > > to > > > describe that apache-airflow-providers-google[tests] would have an > > > dependency to the common_tests subproject? > > > > Would mean you would need to install via > > > > pip install -e . -e ./task_sdk[tests] -e. ./providers/google[tests] > > > > > > Something like that. This is more of a details of workspace but we are > > > going to use "dev" dependency group for that - rather than extra. > Details > > > to be worked out how pip interaction will look like (pip does not have > > > support for dependency groups yet - they are coming in 25.1 - they are > > > already merged in main) > > > > > > With `uv` dependency groups can be used today and "dev" dependency > group > > is > > > installed automatically when you run uv sync or `uv pip install -e.` -> > > so > > > we will follow this along > > > > > > See details about "development dependencies" in uv here > > > > > > https://docs.astral.sh/uv/concepts/projects/dependencies/#development-dependencies > > > > > > Dependency groups are already approved via PEP-735 > > > https://peps.python.org/pep-0735/ and as mentioned - we are a release > > away > > > from having them released in `pip`, so for now we will have to emulate > it > > > with extras for pip case I think but we will be able to remove it when > > pip > > > 25.1 is released and matures a bit. > > > > > > J. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sun, Feb 16, 2025 at 2:23 PM Jarek Potiuk <ja...@potiuk.com> wrote: > > > > > > > > I would love to see some airflow_testing package which will be > > useful for > > > > testing airflow-related projects and involve independently. > > > > > > > > > Certainly, it's not a good thing to have tests import something > from > > > > tests. > > > > New packages as projects are cheap and provide more flexibility and > are > > > > useful from outside of the project. > > > > > > > > I already explained in the PR, but let me repeat my opinion here: > > > > > > > > IMHO It's extremely unlikely we are going to release and publish the > > > > common test code / fixtures in any way. They will continue to be in > > > > development-only-distribution and they will be treated as "internal > > detail". > > > > > > > > If we decide to release and publish them, we will have to maintain > > > > backwards compatibility and account for our users (like you) using > > them for > > > > their own purpose. That would block us or make it very difficult to > > make > > > > breaking changes in them. > > > > > > > > So while you will be free to continue copying the whole distribution > > and > > > > use it in your tests as you want (our licence allows that) - I > > seriously > > > > doubt we will ever release and publish it in "reusable form" with > > > > "compatibility guarantees". It's far more efficient if people like > you > > just > > > > copy them and are aware that they can change any time. > > > > > > > > J. > > > > > > > > On Sun, Feb 16, 2025 at 2:21 PM Alexander Shorin <kxe...@apache.org> > > > > wrote: > > > > > > > >> I would love to see some airflow_testing package which will be > useful > > for > > > >> testing airflow-related projects and involve independently. > > > >> > > > >> Certainly, it's not a good thing to have tests import something from > > > >> tests. > > > >> New packages as projects are cheap and provide more flexibility and > > are > > > >> useful from outside of the project. > > > >> > > > >> Also this project could have a future for testing, compatibility, > > quality > > > >> and rest of measuring. > > > >> > > > >> -- > > > >> ,,,^..^,,, > > > >> > > > >> > > > >> On Sun, Feb 16, 2025 at 4:12 PM Jarek Potiuk <ja...@potiuk.com> > > wrote: > > > >> > > > >> > Hello here, > > > >> > > > > >> > Next phase of the cleanup - it's been sped up by the comment from > > > >> @kxepal > > > >> > - > > https://github.com/apache/airflow/pull/46801#issuecomment-2661415731 > > > >> - > > > >> > I > > > >> > have planned to do it a bit later this week, but maybe indeed > it's a > > > >> good > > > >> > idea to start a discussion now so that people are not confused. > > > >> > > > > >> > Currently we are using "from tests_common" to reuse code in > various > > > >> > providers and airflow, and this is fine, with the exception that > > > >> > "tests_common" is currently just a package in "airflow" main > > project. > > > >> But > > > >> > it does not have to be (or rather it should not be). > > > >> > > > > >> > We should have it in a separate sub-project - similarly as > > providers/* > > > >> and > > > >> > task_sdk are now separate projects - and we should add dependency > > to the > > > >> > common test code distribution from within all the projects that > use > > it > > > >> (via > > > >> > workspace feature). > > > >> > > > > >> > My proposal is to name it "common_test_code" (but I am open to any > > > >> > suggestions): It will look like this > > > >> > > > > >> > . > > > >> > |- airflow > > > >> > |- common_test_code > > > >> > | pyproject.toml > > > >> > | src > > > >> > | tests_common > > > >> > > > > >> > The code in airflow and providers will not change, they will > > continue to > > > >> > use "from tests_common import ...". > > > >> > > > > >> > * uv will work as it used to work, no changes will be needed - uv > > sync > > > >> will > > > >> > automatically install the common_test_code as editable project > > > >> > > > > >> > * additionally - (and that's a big plus) after this change you > > should be > > > >> > able to run `uv sync` in a provider folder and have > > provider-specific > > > >> > separate venv, and basically you should be able to run tests for a > > > >> provider > > > >> > using that .venv and generally treat provider project as a > > "standalone" > > > >> one > > > >> > - this is what I hinted at in the previous mail. > > > >> > > > > >> > * people who do not use uv but pip for example will have to > > manually add > > > >> > the `common_test_code` as an editable install in their venv. For > > > >> example: > > > >> > > > > >> > pip install -e . -e ./task_sdk -e. ./providers/google -e > > > >> ./common_test_code > > > >> > > > > >> > This is the next step in standardizing the layout of the Airflow > > project > > > >> > using workspaces. > > > >> > > > > >> > J. > > > >> > > > > >> > > > > > > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org > > For additional commands, e-mail: dev-h...@airflow.apache.org > > > > >
