Remove deprecations in airflow.models.taskreschedule#41808
Conversation
59c654c to
cf8d291
Compare
|
It's not that I disagree with the PR, but I am surprised by the comment in the newsfragment, regarding the use of sqlalchemy. Since we are not disallowing direct access to the DB, wouldn't that be problematic? |
@vikramkoka The cleanup of the deprecation was not driven by a function or demand to urgently remove it. It was in a batch of "make the house clean" rounds in search & remove all deprecations. The both mentioned functions were not used in any area in the code anyway, they just have kept for a long time as legacy interface - with the reason of semantic release and non-breaking. I did not check the reason behind it and if there is any benefit in keeping them. Somebody in the past long time ago marked them deprecated and this was just a cleaning exercise. And yes, they were not used internally a long time ago and therefore there was no replacements. Did not make a forensic analysis and I assume everybody using that from the past might just copy the traces and re-implement rather having a public API on these helpers. Also as no forensic... I don't see a problem - but this is not a rationale to keep deprecations. I assume they are a leftover trace of a re-factoring which still harm future development. |
|
Yeah. I think @vikramkoka is right - newsfragment in this one is a bit misleading. Since news fragments are intended for users - I think we should clean up the description a bit and say 'direct access to DB is discouraged - using REST API is the only way to interact with Airflow'. This is our statement for a long time and we captured it in the 'Public Interface'. |
Sorry took a moment reading it a second time. I interpreted the first remark as a question "why" we remove the deprecation and now realized that the point to sqlalchemy was the problem you wanted to highlight :-D |
Code cleanup and removal of deprecations in
airflow.models.taskreschedule.