botocore 1.28.0 compat #261
Conversation
Job submissions which triggered the auto-registration of a new job
definition (i.e. because a new image was needed) were failing with
botocore ≥1.28.0 (released 24 Oct 2022), because a new field that is
invalid for registering new job definitions started being returned with
the existing job definition used as a base. This resulted in errors
from `nextstrain build` like:
Submitting job
Parameter validation failed:
Unknown parameter in input: "containerOrchestrationType", must be one of: jobDefinitionName, type, parameters, schedulingPriority, containerProperties, nodeProperties, retryStrategy, propagateTags, timeout, tags, platformCapabilities, eksProperties
Job submission failed!
So far this bug only manifests when installing Nextstrain CLI from the
Bioconda package (and potentially only with Mamba's dependency solver)
because of interactions between s3fs, botocore, and aiobotocore. With
the Bioconda package (and Mamba), the dependency solution uses a new
botocore and old s3fs, which doesn't use aiobotocore and thus doesn't pin
an older botocore.¹ An old s3fs is allowed because the Bioconda package
doesn't specify a lower bound.
In contrast, our Python package installed by Pip _does_ specify a lower
bound for s3fs², which then requires aiobotocore, which then requires an
older botocore where we don't encounter this bug.
A better approach than this hardcoded list of fields will come in the
following commit.
¹ aiobotocore is a giant monkeypatch of botocore, and thus has very
narrow version bounds on botocore.
² See <https://github.com/nextstrain/cli/blob/e9f11e60/setup.py#L100-L123>
for why.
Related-to: <#108>
Related-to: <#134>
| # aren't supported as keyword arguments by register_job_definition(). | ||
| for key in {'jobDefinitionArn', 'revision', 'status'}: | ||
| del derived_definition[key] | ||
| register_inputs = set(batch.meta.service_model.operation_model("RegisterJobDefinition").input_shape.members) |
There was a problem hiding this comment.
This looks like an awesome direction! Are all of the attributes/methods access through botocore's here part of their public API? I don't know anything about botocore and how it wraps AWS APIs, but I didn't immediately find operation_model in the Batch API docs, for example.
There was a problem hiding this comment.
☝️ I have the same question. How did you find such a deeply nested attribute?
There was a problem hiding this comment.
The batch client object here is part of botocore, not boto3, even though we construct it via boto3. The botocore docs, which are almost entirely generated, don't cover the service meta and model stuff, so I'm not sure what the public vs. private nature is. The docs don't explicitly address public vs. private API surfaces. However, the attributes accessed here seem at least semi-public to me, as there are many private-by-convention attributes starting with a leading underscores.
I found the attribute chain thru inspection, knowing that they'd likely be there as botocore had data-defined models of all the services and API endpoints and used those to generate and drive its client code. (If the attributes weren't there, I was prepared to load and traverse the JSON files defining the models directly, but that would be more brittle, I think.)
There was a problem hiding this comment.
Ohh, I see. botocore uses these attributes to generate their own docs.
There was a problem hiding this comment.
Yeah. My guess is that AWS would disclaim support for these attributes and consider them only public within botocore's own internals, but I think they're likely to be stable enough to be ok for us to use. (That is, at least as stable as hardcoding a list of fields.)
Adds the same minimum version bound used by our Python package on PyPI. This lower bound is not strictly necessary for functionality, but it ensures that this Conda package ends up with an s3fs version that uses aiobotocore instead of much older s3fs versions that don't. The increased parity between the Python and Conda packages is useful to avoid compatibility bugs only observed in one packaging or the other. Related-to: <nextstrain/cli#261>
Adds the same minimum version bound used by our Python package on PyPI. This lower bound is not strictly necessary for functionality, but it ensures that this Conda package ends up with an s3fs version that uses aiobotocore instead of much older s3fs versions that don't. The increased parity between the Python and Conda packages is useful to avoid compatibility bugs only observed in one packaging or the other. Related-to: <nextstrain/cli#261>
…t don't roundtrip Use boto3/botocore's models to discover the valid fields for job definition registration and pare down the derived input object to those. This approach avoids breakage when the API changes, e.g. like the one fixed by "runner.aws_batch: Fix error on job submission with botocore ≥1.28.0" (d1fd5f5).
tsibley
force-pushed
the
trs/botocore-1.28.0-compat
branch
from
08fd343 to
0b23ebf
Compare
|
Repushed with a small but I think useful code simplification. Testing locally it still works as expected. |
Adds the same minimum version bound used by our Python package on PyPI. This lower bound is not strictly necessary for functionality, but it ensures that this Conda package ends up with an s3fs version that uses aiobotocore instead of much older s3fs versions that don't. The increased parity between the Python and Conda packages is useful to avoid compatibility bugs only observed in one packaging or the other. Related-to: <nextstrain/cli#261>
Merge PR #39711, commits were: * Update nextstrain-cli's dependency on s3fs Adds the same minimum version bound used by our Python package on PyPI. This lower bound is not strictly necessary for functionality, but it ensures that this Conda package ends up with an s3fs version that uses aiobotocore instead of much older s3fs versions that don't. The increased parity between the Python and Conda packages is useful to avoid compatibility bugs only observed in one packaging or the other. Related-to: <nextstrain/cli#261>
Merge PR bioconda#39711, commits were: * Update nextstrain-cli's dependency on s3fs Adds the same minimum version bound used by our Python package on PyPI. This lower bound is not strictly necessary for functionality, but it ensures that this Conda package ends up with an s3fs version that uses aiobotocore instead of much older s3fs versions that don't. The increased parity between the Python and Conda packages is useful to avoid compatibility bugs only observed in one packaging or the other. Related-to: <nextstrain/cli#261>
See commit messages for details.
Testing