Skip to content

Conversation

@vincbeck
Copy link
Contributor

Resolves #40684


^ 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

potiuk commented Jul 10, 2024

I think we also need to add the 2_9_PLUS skipif for the failing test

@vincbeck
Copy link
Contributor Author

vincbeck commented Jul 10, 2024

I think we also need to add the 2_9_PLUS skipif for the failing test

Do we? Should not it be automatic? If airflow < 2.9 is used (which is the case in the compatibility tests), then AwsAuthManager raises AirflowOptionalProviderFeatureException, then the test is skipped?

Let's see 👀

@vincbeck
Copy link
Contributor Author

You're right

@potiuk
Copy link
Member

potiuk commented Jul 10, 2024

Do we? Should not it be automatic? If airflow < 2.9 is used (which is the case in the compatibility tests), then AwsAuthManager raises AirflowOptionalProviderFeatureException, then the test is skipped?

Those are two different mechanisms. Yes. It used to be like you described when we only did "import check". But when we started to run real tests, the tests that are not supposed to be run should be excluded by skipif

@vincbeck
Copy link
Contributor Author

Do we? Should not it be automatic? If airflow < 2.9 is used (which is the case in the compatibility tests), then AwsAuthManager raises AirflowOptionalProviderFeatureException, then the test is skipped?

Those are two different mechanisms. Yes. It used to be like you described when we only did "import check". But when we started to run real tests, the tests that are not supposed to be run should be excluded by skipif

I see. It is reassuring to know where this misunderstanding comes from :) Thanks!

@vincbeck vincbeck merged commit 224cb75 into apache:main Jul 10, 2024
@vincbeck vincbeck deleted the vincbeck/aws_auth_manager_2_9 branch July 10, 2024 16:01
@potiuk
Copy link
Member

potiuk commented Jul 10, 2024

I see. It is reassuring to know where this misunderstanding comes from :) Thanks!

We are living on a bleeding edge. What was valid yesterday, might not be today ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers provider:amazon AWS/Amazon - related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AwsAuthManager seems to be incompatible with Airflow 2.8

2 participants