Skip to content

Conversation

@kahlstrm
Copy link
Contributor

Relates to #39496 and the aftermath of it.
In that PR tests for served logs were added for other instance states than TaskInstanceState.RUNNING, including TaskInstanceState.UP_FOR_RETRY (added in the PR) and TaskInstanceState.DEFERRED (not added in the PR).

However, as it was briefly discussed in the PR and was later discovered, the current testing setup does not seem to guarantee consistent behavior for TaskInstanceState.DEFERRED, as the tests succeed in local, but fail in the CI.

This PR removes the testing of the state TaskInstanceState.DEFERRED, as it was not the main purpose of #39496 but was rather an attempt to add better testing coverage for old behavior cases as well.


^ 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.

kahlstrm added 2 commits June 15, 2024 13:11
This test works fine when running it on local, but fails in CI. Setting up a correct DEFERRED task instance that behaves consistently seems to require more work, so removing this parameterized case for now.
@ferruzzi
Copy link
Contributor

In addition to the comment in the code, could you open an Issue with details so someone might pick it up?

@potiuk
Copy link
Member

potiuk commented Jun 18, 2024

In addition to the comment in the code, could you open an Issue with details so someone might pick it up?

Yeah. that's what we usually do. Create an issue and link to it from the TODO comment.

@uranusjr uranusjr merged commit ddee71f into apache:main Jun 20, 2024
romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants