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

Issue-41243 Unpin the moto dependency #41256

Merged

Conversation

vikramaditya91
Copy link
Contributor

The exception should be ExportTaskNotFound (and not ExportTaskNotFoundFault) as recorded here:
https://docs.aws.amazon.com/AmazonRDS/latest/APIReference/API_DescribeExportTasks.html

closes: #41243


^ 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 AWS/Amazon - related issues labels Aug 4, 2024
@vikramaditya91 vikramaditya91 force-pushed the bugfix/issue-41243-fix-moto-version-pin branch from fd90d5a to 1893dcd Compare August 4, 2024 21:10
@potiuk
Copy link
Member

potiuk commented Aug 5, 2024

The tests should also be made compatible with older versions of Airflow / moto constraints - as we are running the same tests with older versions of Airflow as well, so I guess checking for both variants should solve it.

@vikramaditya91 vikramaditya91 force-pushed the bugfix/issue-41243-fix-moto-version-pin branch from 1893dcd to e55e078 Compare August 5, 2024 12:56
@vikramaditya91
Copy link
Contributor Author

The tests should also be made compatible with older versions of Airflow / moto constraints - as we are running the same tests with older versions of Airflow as well, so I guess checking for both variants should solve it.

I see. Fixed it accordingly

@potiuk
Copy link
Member

potiuk commented Aug 5, 2024

You need to rebase and:

breeze static-checks --type update-providers-dependencies --all-files 

@vikramaditya91 vikramaditya91 force-pushed the bugfix/issue-41243-fix-moto-version-pin branch 2 times, most recently from 19647d2 to 000dbdb Compare August 6, 2024 06:25
@vikramaditya91 vikramaditya91 marked this pull request as draft August 6, 2024 07:03
@vikramaditya91 vikramaditya91 force-pushed the bugfix/issue-41243-fix-moto-version-pin branch 2 times, most recently from cbe6ba2 to 42bfbb1 Compare August 7, 2024 19:49
@vincbeck
Copy link
Contributor

vincbeck commented Aug 7, 2024

Tests are still failing, did you run breeze static-checks --type update-providers-dependencies --all-files?

@vikramaditya91
Copy link
Contributor Author

vikramaditya91 commented Aug 7, 2024

I ran them, but they formatted the files as

@@ -46,7 +46,7 @@
     "devel-deps": [
       "aiobotocore>=2.13.0",
       "aws_xray_sdk>=2.12.0",
-      "moto[cloudformation,glue]>=5.0.0",
+      "moto[cloudformation,glue]>=5.0.0,",
       "mypy-boto3-appflow>=1.34.0",

Not really sure, why it puts that comma after the 5.0.0.

Checking why that actually happens

EDIT: Nevermind. There was a stupid error there. Fixed it

@vikramaditya91 vikramaditya91 force-pushed the bugfix/issue-41243-fix-moto-version-pin branch from 42bfbb1 to 8df3c68 Compare August 7, 2024 20:05
@vikramaditya91 vikramaditya91 marked this pull request as ready for review August 7, 2024 20:06
@vikramaditya91 vikramaditya91 force-pushed the bugfix/issue-41243-fix-moto-version-pin branch from 8df3c68 to d3f9861 Compare August 7, 2024 20:46
@vikramaditya91
Copy link
Contributor Author

vikramaditya91 commented Aug 7, 2024

Looks like some unrelated tests are failing...:(

@vikramaditya91 vikramaditya91 force-pushed the bugfix/issue-41243-fix-moto-version-pin branch from 09009ba to 4828dd5 Compare August 8, 2024 21:07
@potiuk potiuk force-pushed the bugfix/issue-41243-fix-moto-version-pin branch from 4828dd5 to 5a91b36 Compare August 11, 2024 21:27
The exception should be `ExportTaskNotFound` (and not
`ExportTaskNotFoundFault`) as recorded here:
https://docs.aws.amazon.com/AmazonRDS/latest/APIReference/API_DescribeExportTasks.html

The moto version dependency pin is updated to reflect this.
Older versions of moto will not succeed this test case, hence the
version pin is bumped to 5.0.12
@potiuk potiuk force-pushed the bugfix/issue-41243-fix-moto-version-pin branch from 5a91b36 to f486be0 Compare August 12, 2024 22:34
@potiuk potiuk merged commit fb378bd into apache:main Aug 13, 2024
106 of 107 checks passed
Copy link

boring-cyborg bot commented Aug 13, 2024

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

Artuz37 pushed a commit to Artuz37/airflow that referenced this pull request Aug 19, 2024
The exception should be `ExportTaskNotFound` (and not
`ExportTaskNotFoundFault`) as recorded here:
https://docs.aws.amazon.com/AmazonRDS/latest/APIReference/API_DescribeExportTasks.html

The moto version dependency pin is updated to reflect this.
Older versions of moto will not succeed this test case, hence the
version pin is bumped to 5.0.12

Co-authored-by: Vikram Gaonkar <[email protected]>
@vikramaditya91 vikramaditya91 deleted the bugfix/issue-41243-fix-moto-version-pin branch August 19, 2024 14:19
romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Aug 20, 2024
The exception should be `ExportTaskNotFound` (and not
`ExportTaskNotFoundFault`) as recorded here:
https://docs.aws.amazon.com/AmazonRDS/latest/APIReference/API_DescribeExportTasks.html

The moto version dependency pin is updated to reflect this.
Older versions of moto will not succeed this test case, hence the
version pin is bumped to 5.0.12

Co-authored-by: Vikram Gaonkar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:amazon-aws AWS/Amazon - related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Moto 5.0.12 breaks amazon provider's RDS tests
3 participants