potiuk commented on code in PR #44610: URL: https://github.com/apache/airflow/pull/44610#discussion_r1867887831
########## providers/src/airflow/providers/common/compat/versions.py: ########## @@ -0,0 +1,26 @@ +# Licensed to the Apache Software Foundation (ASF) under one Review Comment: This is, unfortunately, not a complete solution. If we want to move those thing to "versions" in `common.compat` we also need to import those in all providers that use it, and add `provider.yaml` dependency to this new (not yet released) feature of `common.compat`. and after discussion with @dstandish - I am not sure we want to do it. I think we should agree what to do and how to approach it consistently (and also have it described somewhere - how we are doing it. Daniel in slack discussion raised concerns about dependencies introduced this way - for example this one introduced dependency on specific common-compat version in providers using it (which BTW. should be added in `provider.yaml` if we want to depend on this common.compat package.) And yes I agree having just import of the constant requiring to have "common.compat" dependency is excessive. So I think it would be great if we agree @dstandish @uranusjr @Lee-W @eladkal - from people involved in the discussiosn what we do - before we do it. Because it has some questions/ answers: * `tests_common` has currently the same named constants and it already caused confusion - worth renaming those ? * we miss `common.compat` dependency to the upcoming version of common.compat - each provider using those constants has to have `common.compat >= NEW_VERSION` ? * are we going to have 2.11 as well in the list of constants below for the future? * maybe it's better to copy*paste the - currently DRY - code to all providers rather than have this common code at all? Those were all the questions that need some answers and consensus, so that we are all on the same page and can guide contributors and commiters how to add this compatiblity code. -- 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]
