-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Allow to skip test_should_be_importable if optional dependency not installed
#37558
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
Conversation
50b5bc8 to
49e1f4d
Compare
| OPTIONAL_PROVIDERS_DEPENDENCIES: dict[str, dict[str, str | None]] = { | ||
| # Regression of https://github.com/apache/airflow/pull/37524 | ||
| # It loads the module now eagerly instead of lazily | ||
| "tests/system/providers/common/io/example_file_transfer_local_to_s3.py": {"s3fs": None} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's no do this. This one hides the issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm think it depend on how does it long takes to fix it.
If we think that it could be fixed soon, than we fix it first and after that made appropriate changes
If do not know how long does it take we have a to option
Option 1: Revert #37524, otherwise we would have an error in the other tests
Option 2: Hide issue (merge this PR as is), create a task and remove it during the fixing
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like it might be a quick fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But yeah. This fix either not work well. It fix on one case, but not in the others
|
Ok, let close it for now |
^ 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.rstor{issue_number}.significant.rst, in newsfragments.