potiuk commented on PR #23286: URL: https://github.com/apache/airflow/pull/23286#issuecomment-1122639779
There are two types of 2.1 compatibiliy tests: 1) Import tests https://github.com/apache/airflow/blob/60a1d9d191fb8fc01893024c897df9632ad5fbf4/.github/workflows/ci.yml#L807 It does this: * removes airflow from Breeze image * builds all provider packages from sources * installs airlfow 2.1 from PyPI * installs all (compatible with 2.1) provider packages from the files * checks if all dependencies match * walks through all "airflow.providers" classes and tries to import them * fails if there is an import error or unknown warning (it keeps list of known warnings) It's not perfect (and some of the problems in the past slipped through) because this test only checks if the top-level imports, so if you change method's signature, or perform local import in your provider it would not be caught. But it already saved us from accidental incompatibilities more than few times. Because of those possibilities to "slip-through" we also have a separate pre-commit test where we add "known" ways you could break compatibility. Those are usually new fields or signature changes that we know people could use accidentally without even realising they might be using it wrongly. https://github.com/apache/airflow/blob/main/scripts/ci/pre_commit/pre_commit_check_2_1_compatibility.py Again not perfect (simple regexp matches) but "good enough" to capture accidental usages. We could potentially add your method as a "known" case, but this one would be tricky - simple regexp would not be enough, we wouod have to literally parse the AST tree of the code which is way slower and much more complex than simple regexp. That's why adding a new method seems like a better solution (the use of this method would have been caught in the "import" check). And also - we have two providers, cncf.kubernetes and celery, that we have "good reasons" to break the 2.1 compatibilty - they are both 2.3.0+ only, so your new method would be useful in those providers even now (and those providers are removed from the compatibility check) - I am planning to add a new compatibility check for the two as soon as we get to 2.3.1 release BTW. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
