fix response structure for S3 ListMultipartUploads#7983
Conversation
aec8162 to
72afb68
Compare
72afb68 to
338ca3c
Compare
thrau
left a comment
There was a problem hiding this comment.
LGTM! another nice catch :-) just wondering about whether we need moto here
localstack/services/s3/provider.py
Outdated
| try: | ||
| response: ListMultipartUploadsResult = call_moto(context) | ||
| except NotImplementedError: |
There was a problem hiding this comment.
just a question: do we need this try/except? is this method implemented in moto, and are we only using it partially? or can we just replace call_moto with our implementation all together?
There was a problem hiding this comment.
It's partially implemented in moto, and raises only if some parameters are passed:
https://github.com/localstack/moto/blob/58830863f04661fd32d72a79c0046cb667068a34/moto/s3/responses.py#L428-L432
We could replace it I suppose because I think if there's an exception, we're basically doing what it would do if there wasn't an exception. Good idea!
There was a problem hiding this comment.
I guess it was in the case they would add more things, because right now our implementation is incomplete, and if moto adds feature we wouldn't get them.
This is a follow up from #7545, a user reported an issue in our Community Slack. The test is AWS validated, and once again the specs were not compliant with what S3 really returns.
We also reapply the logic that was in a patch previously set, moto would have raised an error if using some request parameters that were not implemented. It would raise a
NotImplementedexception. The previous provider was swallowing the exception and letting it pass anyway without applying the parameters. We can keep the same behaviour for now and come back later and actually implement the right behaviour.related: #7981, localstack/docs#539
\cc @steffyP