go-cloud
go-cloud copied to clipboard
Docstore/awsdynamodb: Supporting the AWS V2 SDK
Resolved #3458
A somewhat quick and dirty conversion to using the AWS V2 SDK rather than the deprecated V1.
Due to the delays in my finding time to work on this I prioritised getting the functionality migrated to mitigate any risk from API drift form the deprecated V1 SDK.
The main sections that warrant further discussion:
- codec behaviour could be handled better with type switches now, but may be a follow up task
- url opener doesn't use a configuration override any more, most of this is handled by
V2ConfigFromURLParams, but is worth review - error handling using an As pattern and I've declared all the as targets in a block which is likely less memory efficient, but simple to read, should another approach be used?
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
View this failed invocation of the CLA check for more information.
For the most up to date status, view the checks section at the bottom of the pull request.
Hi Johnathan,
Last year the AWS SDK V1 was deprecated (see announcement about maintenance and end of support in project readme) with its maintenance grace period set to end on 7/31/2025, this PR was created to address that and move the package to using the new SDK as all the other modules already have.
This was discussed in the flagged issue #3458 Forms part of the larger body of work supporting #3489 And follows all the other modules being moved to use V2 as the default (#3465 and #3466)
docstore/awsdynamodb was the only module without a V2 implementation, and as seen in #3458 was discussed as being better served as a breaking replacement ahead of removing V1 from other modules, rather than the more complicated task of building a complete abstraction of the behaviour to support both, or having duplicated code for both.
I'll be adding comments on the three sections you've specifically flagged as well.
Apologies for the mentions but as its been a week and I've been seeing notification issues in other repo's this week; so just to be sure @jba and because the prior conversation about this was with you, @vangent
@Maldris, I read #3458 as concluding that a v2 fork was the right approach. I'm in favor of that.
ok, did a little history re-writing to move things into docstore/awsdynamodbv2 while preserving change history a bit better (though it forced changing all the commit dates to today to keep it simpler)
I still have the other variant on a backup branch locally, but does this pass requirements better @jba ?
Can we make the path docstore/awsdynamodb/v2? That's typically what we do. For example, math/rand and math/rand/v2.
Can you merge this with HEAD and push it forward? It's the only thing blocking removal of V1 entirely.
merged and updated with the changes to the docstore test harness added in V1 that needed to be replicated in V2.
@Maldris looks like we still have some failing checks.
@Maldris @jba Do we have any update for this PR? aws-sdk-go will be end of support in < 2months.
Fix tests, then we can submit.
Try syncing to head and re-push to see if tests pass.
I cloned this and made some fixes, see https://github.com/google/go-cloud/pull/3602.
apologies, I've been dealing with some medical and family issues and this got lost in the noise till these notifications.
Thank you for finalizing the updates, I'll hopefully have a chance to use it once I'm back at work
No worries, and thanks for the contribution! Hope things are solid enough you can get back to work soon.