ashb commented on a change in pull request #4863: [AIRFLOW-3841] Remove DagBag
from /tree
URL: https://github.com/apache/airflow/pull/4863#discussion_r263724702
##########
File path: airflow/models/__init__.py
##########
@@ -2574,19 +2574,21 @@ def clear(self,
return count
- def get_task_instances(self, session, start_date=None, end_date=None):
+ def get_task_instances(self, start_date=None, end_date=None):
Review comment:
Peter and I have been chatting about this, and what the right approach is.
I'd like us to move torward less granular commits, particularly in the
webserver, following the flow form
https://docs.sqlalchemy.org/en/latest/orm/contextual.html?highlight=contextual#using-thread-local-scope-with-web-applications
which basically says "start a transaction at the start of a request, run
everything in that one transaction, and commit once at the end".
This change takes us further from that.
I would vote for leaving this as is -- it _allows_ us to have control over
what transaction this FN runs in, but by default (if it is not passed) it will
run in it's own transaction anyway.
In short I don't think this code change is necessary: it removes control,
and doesn't change the default behaviour
----------------------------------------------------------------
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
