I understand that having multiple ways of doing the same thing is not needed and we need to slowly and eventually clean up these things to simplify the codebase and also make it maintainable.
Aligning with one of the principles of Airflow3, we need to simplify things, nuke unnecessary stuff, remove deprecated code etc. This will have multiple benefits for the ecosystem: 1. Maintainability is not a headache. We will know for sure that users are using certain things in a certain way. 2. It will make it easier for us to make some decisions like this one without major discussions and edge case considerations because we know how users are using certain patterns (aligns with point 1) 3. Easier for new contributors to contribute to :) I am all up for cleaning up the codebase if we have something of this nature. My 2c. Thanks & Regards, Amogh Desai On Tue, Jul 2, 2024 at 4:29 AM Daniel Standish <daniel.stand...@astronomer.io.invalid> wrote: > Sometimes in software, we try to be helpful by doing a bunch of work for > the user, but it ends up causing a bunch of confusion and maintenance > headaches. I think these decorators are an example of that. > > What `unify_bucket_name_and_key` does is, it tries to make it so you could > pass the full S3 URI to `key`, or you can provide them separately as bucket > and key, and the decorator will "figure it out". > > Then this decorator can also be combined with another one, > @provide_bucket_name, which, if you don't provide the bucket name, it will > grab it from an airflow connection. > > Then, sometimes the params are again passed through another function > `get_s3_bucket_key`. > > This requires us to add tests to ensure that the decorators are applied in > the correct order, and now, to handle the various async cases (the > decorated fn may be either coroutine or async iterator). > > It's a bunch of code and complexity that, really does not need to exist. > I.e. what if bucket were simply bucket, and key were simply key, always? > > If it were up to me, I'd just remove all of it. Just force the user to > supply a bucket and key separately. > > We can of course do this since semver allows it. But some portion of users > will be depending on this change and will have to change their code. > > Curious what others think. >
