potiuk commented on PR #41327: URL: https://github.com/apache/airflow/pull/41327#issuecomment-2435250032
That's a big one. There are two comments here: 1) We are rethinking packaging/ providers/plugins APIs and very likely we are going to have several different "types" of Airflow extensions - separate secret backends, separate "operators/hooks providers", separate log handlers, separate UI plugins. And while this one looks like classic "operator/hooks type provider" I think we should decide what to do before we merge that one (we are going to discuss it today at the dev call and discussion will start shortly in devlist I think) cc: @kaxil @ashb @vikramkoka 2) I think that one is good as a "base" PR, but once this one is green and we more or less know that the common part is stable, you should split that one into first adding the functionality to provider's manager and other common parts (and maybe even split that one), followed by separate change for every provider. That would make it WAY easier to review and is relatively easy to do - this is the favourite way for me to do such changes - becaue once you get the common part merged, you rebase the "big" PR and it gest smaller - and it gets smaller and smaller with every provider added later until it disappears. This helps with fast and effective reviews of such PRs. -- 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]
