Skip to content

Conversation

@kzosabe
Copy link

@kzosabe kzosabe commented Mar 15, 2024

closes: #34493

The previous PR (#34771) is stuck due to the complexity of the problem.
This incorrect warning is causing inconvenience to users, so we will make a change to stop it first.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@gilt-cl
Copy link

gilt-cl commented Mar 25, 2024

@kzosabe Is there a chance this will be merged in the next release?

@kzosabe
Copy link
Author

kzosabe commented Mar 25, 2024

@gil-tober I hope it will, but I don't have the authority to decide that.
For some reason this PR and related issues have been forgotten by the maintainers of this OSS, so I am not sure if it will happen.

@gilt-cl
Copy link

gilt-cl commented Mar 25, 2024

@kaxil @XD-DENG @ashb
As @kzosabe mentioned, is there a chance for this to be merged in the upcoming release?

@potiuk potiuk added this to the Airflow 2.9.0 milestone Mar 26, 2024
@potiuk
Copy link
Member

potiuk commented Mar 26, 2024

Yes. It's very bad to keep you in a limbo like that especially after all the attempt you've made @kzosabe . Apologies for that in the name of all other maintainers. You seem to have a valid annoying issue that we generally avoided to handle.

I have no idea if it is right or wrong. This all seems from the implementation in #30612 and it seem to be somewhat broken anyway as indicated in #30612 (comment) and PR to likely fix some of that in #37936. Maybe @vandonr as the original author can chime in an comment on those. Seem that none of the other maintainers involved in the past discussions have picked up the discussion, but It's lkely better to avoid harm even if the discussion is not finished (cc: @hussein-awala @uranusjr).

I will ping them to see if they can be fixed in the original PRs, but for now I see no harm at all to change that warning into a debug mode. Happy to accept the change for 2.9.0, but:

a) please change it to debug.
b) please leave a comment explaining that this is temporary measure until the PR/issue is fixed.

Please ping me when it's done and I will merge it.

@kaxil
Copy link
Member

kaxil commented Mar 27, 2024

Yup agreed with changing it to Debug in the meantime

@gilt-cl
Copy link

gilt-cl commented Mar 27, 2024

@potiuk @kaxil @Lee-W @kzosabe
Thanks for picking this up.
@HadarSha and I have been waiting for this for quite a long time.
We have about 1.3K DAG runs/70K tasks daily and this issue has prevented us from upgrading Airflow since 2.5.3

@phanikumv phanikumv requested review from dstandish and uranusjr March 27, 2024 07:00
Co-authored-by: Wei Lee <weilee.rx@gmail.com>
@pankajkoti pankajkoti changed the title Remove incorrect warning about scheduled_duration metric Update log level to debug from warning about scheduled_duration metric Mar 27, 2024
@potiuk
Copy link
Member

potiuk commented Mar 27, 2024

Random failure. Merging.

@potiuk potiuk merged commit b9e96df into apache:main Mar 27, 2024
@boring-cyborg
Copy link

boring-cyborg bot commented Mar 27, 2024

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

@eladkal eladkal added the type:misc/internal Changelog: Misc changes that should appear in change log label Mar 28, 2024
ephraimbuddy pushed a commit that referenced this pull request Mar 31, 2024
…ic (#38180)

---------

Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
Co-authored-by: Wei Lee <weilee.rx@gmail.com>
(cherry picked from commit b9e96df)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:misc/internal Changelog: Misc changes that should appear in change log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

emit_state_change_metric produces many warnings even in standard use cases

9 participants