anitakar commented on pull request #8718: URL: https://github.com/apache/airflow/pull/8718#issuecomment-625364951
> > Hi, > > I will collectively answer the questions here. > > Many of the changes that I have made have been a result of setting store_serialized_dags from conf in DagBag init method in one of squashed together commits. > > Before I decided that it is better to leave it with False default I started to change it to False in all places where it broke things. > > And it broke things in CLI, dag parsing code and this migration: airflow/migrations/versions/cc1e65623dc7_add_max_tries_column_to_task_instance.py > > What error exactly? And is that error reproducible in Airflow 1.10.10, or is it only a problem in the Cloud Composer version? > No, it is not a problem in Cloud Composer. At some point I made the change in DagBag to read store_serialized_dags instead of setting it to false but it was intermediate state. It was never released like that without a fix. > > This change is in fact noop so maybe we should drop it. > > It actually is about not reading core.store_serialized_dags on server startup. I had also problems with tests because of that. But if that operation is expensive and people will not probably be turning dag serialization on and off, then maybe it is better to drop it. > > Airflow does not support changing config at runtime -- it is not possible. It is only the tests that are able to change settings. > Then I shall drop the change of removing STORE_SERIALIZED_DAGS from settings. But the change is not a no-op in the end. It had different code for accessing dag code in UI that you fixed later on. Composer for a moment in Airflow 1.10.6 used storing, deleting and saving one by one but community version uses batch operations that I saw you had to fix. But there is an actual fix for non-RBAC www for setting paused on DAG. I shall create a test for it. I can also remember that we had problems with calling run from non-RBAC UI but I am sure if it was true for open sourced version somebody would have noticed it by now. > > Here is a similar change to Airflow 1.10.10: > > #8764 > > This one actually fixes starting dag from UI. > > In this change I can also see dag async dag bag loading was dropped altogether in Airflow 1.10.10. Am I correct? > > No, async dag bag loading was _never in_ Airflow. Either dag serialization is enabled, in which case the webserver loads it form the serialized tables in the DB, or it's not and the existing mechnaism is in place. Having a _third_ method of loading dags when dag serialization exists and fixes the performance overhead seems like extra code to maintain for little benefit. > > Also: there is a reasonable chance that in Airflow 2.0 serializing dags may be the _only_ option. Oh, so it was Composer internal feature for clients with lots of DAGs. Anyways, our intention was to always contribute back to community, so I guess it is not a problem. But as there is serialized dags solution fully working this workaround can be dropped. ---------------------------------------------------------------- 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]
