Thanks to everyone who spoke. Yeah. There are certain dangers of that approach. I think for now we have not seen much of a problem with the current approach, but we might want to address it differently - probably not by running a complete set of tests for older versions (that's not really feasible), most likely by just running import checks as we do currently in the compatibility checks by providers.
Let me park this one for a while, and possibly we can come back to it when/if we do the `--lowest ` resolution check for dependencies with UV - which is something that should be addressed separately - but fits the general area to address - what should be the right lower binding set for the dependencies. J. On Mon, Apr 15, 2024 at 8:41 AM Aritra Basu <aritrabasu1...@gmail.com> wrote: > I tend to agree with Wei here, if process can fix the issue maybe it > shouldn't go into code. > > -- > Regards, > Aritra Basu > > On Mon, Apr 15, 2024, 9:42 AM Amogh Desai <amoghdesai....@gmail.com> > wrote: > > > I agree with the points made by Andrey here. > > > > > End users use amazon provider and google provider, and do not use > > common.sql and both of them have mandatory common.sql as dependency. > > > > On the contrary, would it be better to remove common.sql as mandatory > > dependencies for > > providers that do not use it? That way we would avoid one of the bigger > > problems which is > > maintaining the providers that have common.sql as dependency but do not > > need it as and when we > > someday deprecate common.sql? > > > > Thanks & Regards, > > Amogh Desai > > > > > > On Sun, Apr 14, 2024 at 2:20 PM Wei Lee <weilee...@gmail.com> wrote: > > > > > 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] > > > > > > > > >
