Skip to content

Conversation

@Taragolis
Copy link
Contributor


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

@boring-cyborg boring-cyborg bot added area:providers provider:amazon AWS/Amazon - related issues labels Feb 20, 2024
@Taragolis Taragolis force-pushed the allow-to-skip-validate-examples branch from 50b5bc8 to 49e1f4d Compare February 20, 2024 11:38
@Taragolis Taragolis added changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) full tests needed We need to run full set of tests for this PR to merge and removed provider:amazon AWS/Amazon - related issues area:providers labels Feb 20, 2024
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}
Copy link
Contributor

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

Copy link
Contributor Author

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?

Copy link
Contributor Author

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

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

@Taragolis
Copy link
Contributor Author

Ok, let close it for now

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

Labels

changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) full tests needed We need to run full set of tests for this PR to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants