potiuk commented on a change in pull request #6950: [AIRFLOW-6392] Remove
cyclic dependency baseoperator <-> helpers
URL: https://github.com/apache/airflow/pull/6950#discussion_r361961106
##########
File path: airflow/utils/helpers.py
##########
@@ -142,83 +135,6 @@ def as_flattened_list(iterable):
return [e for i in iterable for e in i]
-def chain(*tasks):
Review comment:
> The existence of the idea of creating an automatic tool does not affect
whether we should or should not make changes.
I don't agree. By having automated tools we might decide that we
deliberately not keep backwards compatibility in some cases more easily -
especially if those changes can easily and automatically be migrated in the
existing DAGs.
"Backwards compatibility" is simply not as important for 2.0 as it was in
1.10. This decision has already been made. We have a number of backwards
incompatible changes, there is no going back.
> Others look much less used, although there may be someone who finds these
functions. But in my opinion, this is the choice of the lesser evil.
I am not sure which one is lesser. If we automate migration for chain method
it's not evil at all for migration. Especially that finally it makes sense for
the chain method to be in baseoperator package (or in some form of
operator_helpers but I prefer not to use helpers/managers/etc. if not
neccessary). It makes much more sense for the other helpers method to be there
- they were there originally, simply adding chains and the other method here
later was a mistake that we want to correct now. For me more evil is to have
bad naming in future version of airflow.
>> It is not possible to import a single static method into a file. You must
always import the whole class. A similar opinion was expressed in "Google
Python Style Guide":
Agree - will make it module function in baseoperator. it makes more sense.
It comes from the old Java background where I prefer to have functions in
classes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
