Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Remove remaining Airflow 2.5 backcompat code from GCS Task Ha… #36453

Merged
merged 1 commit into from
Dec 27, 2023

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Dec 27, 2023

…ndler (#36443)"

This reverts commit 75faf11.


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

@potiuk
Copy link
Member Author

potiuk commented Dec 27, 2023

Seems that after merging that limited PR - the error started to appear again. Let's see what happens in the PR.

@potiuk potiuk added the full tests needed We need to run full set of tests for this PR to merge label Dec 27, 2023
@potiuk potiuk closed this Dec 27, 2023
@potiuk potiuk reopened this Dec 27, 2023
@hussein-awala
Copy link
Member

There is nothing related to asyncio in this change 🤔

@potiuk
Copy link
Member Author

potiuk commented Dec 27, 2023

I know :(

@potiuk
Copy link
Member Author

potiuk commented Dec 27, 2023

So far It somehow looks liks this failure only happens in main (I have not seen it in any PR) - so maybe there is a difference that I will be able to track if I will get enough evidences ...

@Taragolis
Copy link
Contributor

Taragolis commented Dec 27, 2023

I think this all errors refers to the old issue with this test, previous attempt before we migrate everything to the pytest:

@potiuk
Copy link
Member Author

potiuk commented Dec 27, 2023

Also reverting it and running few times main merge without seeing the error after seeing it in few other builds before merge, might give more clues. I plan to merge a few more PRs and see if main builds will have consistent failures before/after merging this one. It's a bit slow and "try/error" but for now I am a bit lost to what causes it and have to revert to the merge-> revert see what happens :D

@potiuk
Copy link
Member Author

potiuk commented Dec 27, 2023

I think this all errors refers to the old issue with this test, previous attempt before we migrate everything to the pytest:

🤔

@Taragolis
Copy link
Contributor

Maybe we could return to previous solution, when we manually filtered log records by a logger name instead of provide it into the caplog.at_level.

More interesting why this error appear again 🤔

@potiuk
Copy link
Member Author

potiuk commented Dec 27, 2023

More interesting why this error appear again 🤔

Indeed .. that's very interesting.

@potiuk
Copy link
Member Author

potiuk commented Dec 27, 2023

Let's see https://github.com/apache/airflow/actions/runs/7339685558

Update: Failed again with the same error on Postgres 14/Python 3.10

@potiuk
Copy link
Member Author

potiuk commented Dec 27, 2023

Let's see the next one https://github.com/apache/airflow/actions/runs/7340134340

@potiuk
Copy link
Member Author

potiuk commented Dec 27, 2023

Let's merge it to see if it helps. We had two subsequent main failures after merging PRs

@potiuk
Copy link
Member Author

potiuk commented Dec 27, 2023

Merged the https://github.com/apache/airflow/actions/runs/7340642180 to see the third failure.

@potiuk potiuk merged commit 127c072 into apache:main Dec 27, 2023
89 of 127 checks passed
@potiuk potiuk deleted the revert-gcp-backcompat branch December 27, 2023 18:50
@potiuk
Copy link
Member Author

potiuk commented Dec 27, 2023

Merged. It failed 3 times in a row now -let's see if that one will fix main ... Would be super strange but with this kind of side effect, it could be something very, very unrelated

@potiuk
Copy link
Member Author

potiuk commented Dec 27, 2023

Aaaand surprise surprise ... Merging the revert fixed the problem ?!?! .... this is a very strange issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:logging area:providers full tests needed We need to run full set of tests for this PR to merge provider:google Google (including GCP) related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants