o-nikolas commented on code in PR #27067:
URL: https://github.com/apache/airflow/pull/27067#discussion_r1017199736
##########
airflow/dag_processing/manager.py:
##########
@@ -1129,7 +1129,6 @@ def _kill_timed_out_processors(self):
)
Stats.decr('dag_processing.processes')
Stats.incr('dag_processing.processor_timeouts')
Review Comment:
Great discussion everyone! Two followups:
1. Related to @dstandish feedback:
> If we don't know when it will be removed, then there's not much value in
saying anything about when (i.e. "when we decide to" or "if we ever have
another major release".
The value I'm trying to provided with my proposals isn't to forecast _when_
it will be removed, rather to provide some helpful context, namely some
examples of conditions that would trigger it's removal. Simply saying
"Deprecated; may be removed in a future Airflow release." is a tautology (as
anything deprecated is subject to removal at some point in a future release)
and doesn't provide the reader any further context beyond the first word
"Deprecated". But as always, if I'm out voted, I'm more than happy to go with
your suggestion :relaxed:
2. Related to @potiuk feedback:
I'm struggling to find where to land here. I've been part of some
discussions where I've suggested deprecating early ("slightly more liberal" as
you put it) and I've received push back, and in this thread we're going the
other way (at all costs don't deprecate, just keep the behaviour and document
it's existence as deprecated). This quantum approach means that no one ever
quite knows if and when anything can be removed, so it leads to long
discussions like this one. Whereas, if you take SemVer very literally then at
least everyone knows, more clearly, how to proceed.
If we're choosing to be more flexible and liberal (which I mostly agree
with), then I actually re-propose to remove this metric entirely. It was
publicly messaged to be deprecated along with a batch of other metrics, all of
which were removed but this one was forgotten; you can really think of this
change as a bug-fix of sorts (so no major release needed). If we do the math of
how many users will really be affected, versus the code quality improvements of
not having to maintain and document this behaviour, I think we could easily
land on the removal side. Thoughts?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
