-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Doc-only: mention minimum boto3 1.34.52 for AWS provider when using Batch ecs_properties_override
#39983
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
I suspect that's going to lead to mypy failures, no? |
|
Can you fix the static checks failures? |
|
Also see the https://github.com/apache/airflow/pull/39946/files#diff-08906ccb524962bced7b3f0c2a19d144567985f6f80077ab0e7d5bb2a6917458R95 in #39946 -> for some reason, the tests there are satisified with 1.34.0, so likely we need to add tests as well to cover the 1.34.52+ check. In #39946, apparently all tests passed with 1.34.0 |
|
To be honest, I don't have the bandwidth now to fix all these things. I originally added the Batch PR to fix a use case that my team at work needed. I'm OK with just keeping that first PR as is, and merely adding a warning to the docs that it requires boto3 1.34.52+. I will push a new commit to this PR to make the docs only change |
0cfdd2f to
f2adb9d
Compare
…properties_override'
ecs_properties_override
|
Actually. If you don't mind - bumping the version to .54 is probably way better approach - boto3 .54 has been releaed 3 months ago and we already have .106 in our constraints, so this is a bascically "no-op" change. And while it's a bit worrying that our tests do not catch it, i am not particularly picky here - we only go "up" with the minimum version, so it's quite ok if you just bump the version in provider.yaml (and pre-commit will update it in generated/provider_dependencies.json). |
So are you saying I should just bump the version, and even if mypy or other tests fail, we'll just ignore it? Because I don't have bandwidth to fix it. I'm not clear if you are volunteering to fix it, or are saying to ignore it. |
|
Why do you think my will fail ? I believe just bumping the min version in those two files will just work. But if there will be error, then yes - we expect you fix them. But it can take quite so e time so there is no hurry - you can do it at your leisure |
|
You missed the beginning of this PR, where I indeed bumped the version, all the stuff broke, and I wasn't able to fix it. Just need this to unblock releasing the new version of the Amazon provider, I need it for my team at my day job at Amazon |
|
Following an offline conversation with @o-nikolas and @eladkal, decided to follow an approach similar to the Operator in AWS Bedrock, where new features of boto3 aren't enforced, but rather if an error shows up because of it, we add more context to it. (Plus keeping the existing change in the docstring to warn that the ECS param only works with boto3 1.34.52+) |
ferruzzi
left a comment
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.
Yeah, this is how I handled it in the Bedrock operator. I'm not thrilled with it as a solution and we shouldn't make it a habit, but I think we can work on getting that boto[core | 3] min ver bumped so we can clean these out.
|
Yep. That is also okl |
|
aiobotocore has been our limiting factor for a while; they pin botocore to a pretty old version. A couple weeks ago they released a new version with much newer botocore support. I'm going to see what it will take to get this bumped properly, and we can go from there. related: #40052 |
… using Batch `ecs_properties_override` (apache#39983)" This reverts commit 99dd24b.
…atch `ecs_properties_override` (apache#39983)
* bump boto min versions * Revert "Doc-only: mention minimum boto3 1.34.52 for AWS provider when using Batch `ecs_properties_override` (apache#39983)" This reverts commit 99dd24b. * remove warnings that specific botocore version is required since min version is now higher
…atch `ecs_properties_override` (apache#39983)
…atch `ecs_properties_override` (apache#39983)
* bump boto min versions * Revert "Doc-only: mention minimum boto3 1.34.52 for AWS provider when using Batch `ecs_properties_override` (apache#39983)" This reverts commit 99dd24b. * remove warnings that specific botocore version is required since min version is now higher
The change introduced here (#39903) doesn't work unless boto3 is using version 1.34.52 or higher. In that version, they added
ecsPropertiesOverrideto the API ofsubmitJob(source).Note - I didn't update the versions of these 2 packages, even though the comment said to, because the version 1.34.52 doesn't exist for them
^ 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.