potiuk commented on a change in pull request #10956:
URL: https://github.com/apache/airflow/pull/10956#discussion_r499650488
##########
File path: airflow/models/dag.py
##########
@@ -1573,34 +1680,28 @@ def create_dagrun(self,
@classmethod
@provide_session
- def bulk_sync_to_db(cls, dags: Collection["DAG"], sync_time=None,
session=None):
+ def bulk_sync_to_db(cls, dags: Collection["DAG"], session=None):
Review comment:
Just extracting it to separate methods and changing name would make it
better. I think (i wrote about it in the other comment) some of the methods and
classes lost their original meaning and they do more or less than originally
stated. And the only reason for using separate methods and naming them is to
give the developer a chance to understand what those methods do without having
to read through all the code. So At least an indication that the methods does
more than originally intended would be nice.
I think where it is easy (such as renames/extractions which modern IDEs make
super-easy and 0-risk operation), we should try to make the names reflect a bit
more the reality. I agree introducing a new class and separating out the
responsibility elsewhere might not be a good thing to do now.
----------------------------------------------------------------
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]
