potiuk commented on PR #50269:
URL: https://github.com/apache/airflow/pull/50269#issuecomment-2871374182
Yes. I think such changes in Airflow should be **minor** version upgrades ->
3.1.0 should be minimum version
Also - we have to be quite careful when making such change. We should at
least have a mental model and work out scenarios where various versions and
combos of airlfow + fab would be installed (or prevent them from being
installed together if those combos are not good).
I don't think we have a realy good guide for what happens when you move
config from Airflow to provider like thiat. The ConfigChange we have currently
implemented does not serve well case where configurations are moved between two
distributions, It only works if ConfigChange is moved within Airflow or within
distribution.
Assume we add this in 2.1.0 FAB (not 2.0.2) - moving configuration like that
is for me sign of "minor release" at least.
Two easy cases are :
* FAB 2.0.1 + Airflow 3.0.1 (only config in Airlfow works, no-one suggests
to move it)
* FAB 2.1.0 + Airflow 3.1.0 - config should be in FAB, if someone still has
it in airflow it is going to work wth deprecation.
* More interesting is: 2.0.1 FAB and 3.1.0 Airflow: Will it work? FAB will
read "airflow" one and it will raise deprecation warning, and no way to remove
the deprecation warning. Changing the entry to "fab" will not really work (?)
because FAB will continue reading "airflow" one. I guess that can be solved by
adding FAB >= 2.1.0 in Airfflow 3.1.0
* Even more interesting is 2.1.0 FAB and 3.0.1 Airflow. FAB will read "fab"
entry but airflow wil not have config change inforamtion - so it will only work
when you move configuraiton to "fab' but you have no warning about that - as
airflow is not aware about that config change. Effect - you might silently
loose configuration set in "airflow" config. Again , setting Airlfow >3.1.0 for
FAB 2.1.0 makes sense, but then we should make that change **just** before we
relase 3.1.0, because otherwise nobody will be able to use any 2.1.0 FAB after
we release it. -until we release Airflow 3.1.0.
In shortl - we have two options:
a) I think we should only make that change just before we release 3.1.0 and
bump both Airlfow 3.1.0 and Fab 2.1.0 and make them both depend on the new
version.
b) or maybe we shoudl also figure out handling of config change in provider
separately so that 2.1.0 FAB will handle config change as well and work also
with Airlfow 3.0.*
I think option b) is better if we can pull it off, because then FAB 2.1.
will be able to work with Airflow 3.0* and we will be able to relase security
fixes independently for it. But then some compatibilty code needs to be
implemented in FAB even as simple as manually checking if "airflow" is set and
falling back to it - no matter what is implemented in Airflow.
Actually probably best option would be to add possiblity of providers
contributing dynamically config changes to Airflow, but that might be a bit too
much as we would have to also require some Airflow providers's manager changes
and min version set for Airflow in such providers - so likely "hand-written"
check for config in FAB provider is a better option:
if "airflow" config exists -> read from there and raise deprecation, else
read from "fab". That's probably simples solutin.
--
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]
