I feel like this is the responsibility of the provider contributor 🤔 Maybe we could check whether the PR contains common.sql usage and raise a warning?
Best, Wei > On Apr 12, 2024, at 12:31 AM, Andrey Anshin <andrey.ans...@taragol.is> wrote: > > There are some drawbacks I saw here, it would force to upgrade other > providers to the latest version. > Some scenarios from the my head: > > End users use amazon provider and google provider, and do not use > common.sql and both of them have mandatory common.sql as dependency. > if everything works fine there is no problem, but it could be a situation > that a new release of one of the providers is major or contains bugs and > there is no possible way to downgrade one of them and keep a new version of > another without breaking dependencies. > > Other case if we need to exclude common.sql provider from provider release > wave than we also need to exclude all providers which depends on common.sql > even if it problem not affected directly, e.g. it is not a sql only > provider, e.g. amazon, google, microsoft.azure > > If it is not a big deal I have no objections to adding this because I do > not have a better solution which could be implemented "Here and Now". > > It could be resolved if we might run tests against released versions of > common.sql, but after some brief investigation to run tests > against installed versions and not a main version this might require > changing quite a few things, which might take a time. > > > > > On Thu, 11 Apr 2024 at 12:41, Jarek Potiuk <ja...@potiuk.com > <mailto:ja...@potiuk.com>> wrote: > >> Hello here, >> >> I have a proposal to add a general policy that all our providers depending >> on common.sql provider always use >= LATEST_MNOR version of the provider. >> >> For example, following the change here >> https://github.com/apache/airflow/pull/38707 by David we will update all >> sql providers to have: airflow-providers-common-sql >= 1.12. We could of >> course automate that with pre-commit so that we do not have to remember >> about it. >> >> Why ? >> >> Because it's very easy by a provider to accidentally use a new feature in >> common.sql without bumping the version. Current situation is like in the >> image attached (thanks David), but we cannot be certain that the >> "min-versions" there are "good".. >> >> A bit more context: >> >> Generally speaking - common.sql **should** be backwards compatible - like >> - always. We should not make any changes in it's API (which is for-now >> captured here: >> https://github.com/apache/airflow/blob/main/airflow/providers/common/sql/hooks/sql.pyi >> ). And from time to time we add new things to common.sql that providers >> might start using. >> >> Example: >> >> From the https://lists.apache.org/thread/lzcpllwcgo7pc8g18l3b905kn8f9k4w8 >> thread the new "suppress_and_warn" method is going to be added. >> >> Currently we have no good mechanism to verify if the min version in >> provider dependencies is really "good". For example when we add >> `suppress_and_warn` today and someone starts using `suppress_and_warn` in >> the "presto" (for example) provider tomorrow, without bumping common-sql to >>> = 1.12 - it will fail with 1.11 or earlier installed. >> >> On the other hand - all the tests we run in `main` use the "latest" >> version of the `common.sql` and all the constraints we produce also use the >> latest version released, so we can be rather sure that the new providers >> actually work well with the latest `common.sql` version. If there will be a >> bug about compatibility (happened in the past), then it should be fixed by >> fixing it in a new patchlevel of the `common.sql`. >> >> So in-general it should be "safe" to add >= LATEST MINOR of common.sql in >> all providers. >> >> Should we do (and automate) it ? Any other comments / proposals maybe ? >> >> Here is the current state of dependencies: >> >> >> [image: image.png]
