> Yes we will have to wait some time before all providers can use such a helper, but time flies, and there's no emergency here.
I think that's the key. This is not a question of emergency but having confusion, mistakes for the next at least 8 months. I like the https://github.com/apache/airflow/pull/44607 better because it is simpler. And maybe this is a good one as long as we work out strategy how to get there. One could argue that adding such a solution in Airflow will add **MORE** possibilities to make mistakes. What happens if someone starts using this method in their provider between now and 8 months from now? That will - accidentally - make such a provider not work on Airflow 2.8, 2.9, 2.10. Only 2.11 will work (as long as we back-port it to v2-10-test/ Luckily, we have the back compatibility tests that **should** catch it, but I can imagine the case that someone adds this if and it will not be covered by tests. Then - having it in airflow will add more possibilities of introducing issues. So ... ideally if we introduce this one, we should also somehow (yet another pre-commit?) that it is **not used** till 8 months pass (or generally till min-airflow-version will be 2.11). If we do it, I am all for it as additional solution - then we will be able to remove whatever we come up with for the next 8 months. J. On Fri, Dec 6, 2024 at 8:48 PM Daniel Standish <daniel.stand...@astronomer.io.invalid> wrote: > There are multiple different issues implicated here but the only one I am > interested in is these version constants. And that's what I'll address > here. > > My take is that adding constants like this in ... all? ... providers is > unnecessary and overkill and very bloaty. > > PROVIDERS_{{ PROVIDER_CONSTANT_PREFIX }}_AIRFLOW_IS_2_9_PLUS > > All we're really after is, "what is the airflow version?" > > The context here is when providers are implementing conditional logic based > on airflow core version. Let's also remember that there will soon be a > SDK... shall we add constants for that too? > > I think we're better off if we simply provide a simple way to answer the > question, "what is the core version?" > > We can do this in one place instead of many. > > I have offered two ways we could do that. > > One is #44607 <https://github.com/apache/airflow/pull/44607> which adds a > get version function that gives the base version, which is what all of > these comparisons use. > > Usage example: > > if get_airflow_version() > Version("3.0.0"): > do_something() > > Another, which I think I like better is, #44743 > <https://github.com/apache/airflow/pull/44743>, which returns an > AIRFLOW_BASE_VERSION constant that can be compared both with Version > objects and with strings. > > Usage example: > > if AIRFLOW_BASE_VERSION >= "2.10.0": > do_something() > > This way we don't need to add such constants in every provider, for every > version, for both "greater equals" and "greater than". > > You just give the info and the call site does the evaluation. > > Yes we will have to wait some time before all providers can use such a > helper, but time flies, and there's no emergency here. > > The other issue, overwriting the autogen file has now already been fixed, > thanks to Jarek. And the cross provider import thing, I will leave that > for others to debate. >
