docstore/awsdynamodb: Support AWS SDK V2

[Maldris]

Is your feature request related to a problem? Please describe.

The V1 AWS SDK has officially entered maintenance mode (31st July 2024). In addition, the vendor specific constructors for AWS are inconsistent between using the V1 and V2 SDK.

Describe the solution you'd like

I propose migrating all modules to use the V2 SDK both for consistency when using the vendor specific constructors, but also to address future maintenance considerations as the V1 SDK continues to be deprecated and diverges from V2.

Describe alternatives you've considered

The V1 support could be retained for some time, as the V1 SDK is flagged as receiving maintenance for another year. But this seems the kind of edit likely to accumulate in the amount of effort needed to make the migration over time.

Additional context

I've created a fork and will be investigating the necessary changes there. My intent is to try and keep any necessary changes as minimal as feasible, and created this issue for discussion of anything I find along the way needing input, and to provide advance warning for a future PR.

[Maldris]

I realised in retrospect that in trying to set this up between calls I omitted some critical details.

My initial plan is to ensure a consistent V2 constructor for each package as seen in s3blob. Then (once the above is demonstrable and testable) I'd consider inverting the "UseV2" behaviour to instead be "UseV1" so the new SDK is the default behaviour. Then as V1 gets sunset it would be worth removing it's support entirely, simplifying the packages again.

Thoughts on this roadmap?

[vangent]

Doesn't this exist already? E.g., for s3blob, see https://pkg.go.dev/gocloud.dev/blob/s3blob#OpenBucketV2, pubsub https://pkg.go.dev/gocloud.dev@v0.38.0/pubsub/awssnssqs#OpenSNSTopicV2, etc.

[Maldris]

Sorry, I'm being far too reductive with a few things this morning, I'll blame it on a case of the Mondays sorry.

Most of the packages do, and some of the recent PRs have fixed issues my team flagged.

docstore is the main one without V2 support, which is my immediate focus (and going to be a non-trivial change to keep the "usev2" strategy looking at the package structure)

The secondary goal, which I completely failed to state explicitly because the docstore element was front of mind for an upcoming project was to discuss a strategy for what happens when V1 is completely deprecated and may need to be removed from the package. This was mainly a consideration around the general attempt to keep Go packages as backwards compatible as possible, but persisting its support, particularly as the default behaviour could cause the package to cease working in some aspects over time.

In hindsight I should have split these into two issues, one discussing the docstore change and the other regarding the V1 sunset plan. Which I can do now if you'd prefer?

[vangent]

Let's use this bug to cover migrating docstore to support v2.

I will file a separate one for v1->v2 plan and cc you.

I don't remember why I didn't add v2 support to docstore/awsdynamodb when I did all of the others, but I'm guessing I tried and it was hard :-|. If you want to give it a shot, go for it.

[vangent]

See https://github.com/google/go-cloud/issues/3459 for the other bug, I couldn't figure out how to cc you.

[Maldris]

thanks for renaming and splitting, the start of the week was a lot more manic than planned.

I can confirm docstore has a lot more intersecting considerations, with the main one being that most of the functions pass around AWS concrete types that have been replaced by interfaces in V2 (which the V1 types don't satisfy)

I've got the codec behaviors updated (though I dislike the way I did decoder and may re-do it), and I've spent some time making sure I understand the query constructor and have started experimenting with some of the different strategies I could think of to assess suitability (my intent was for this to be a focus this week and that has not panned out so a lot less than I'd hoped has been accomplished)

One stylistic question I do have, what is the general thinking on making simple types within the package (package private) that wrap the used parts of both? The thought was something similar to the codec's encoder/decoder types, but without the feature level interface they satisfy to be used by driver. Some of these features definitely need the level of access thats easiest with passing around a concrete struct, but I very much don't want to have functions that start, then immediately fork into to near identical functions for v1 and v2 that just use the different types

[vangent]

TBH I'm not sure what the best path forward is. I only vaguely recall trying to do the port and realizing it was a lot more complicated than the others.

If it is too intrusive to support both types at once, another option is to just copy the whole directory and make a new module (e.g., docstore/awsdynamodbv2). That didn't make sense for the other modules like s3blob because the ratio of duplicated code to version-specific code was very high, but that ratio might be different here. Presumably there would still be a significant amount of copied/duplicated code, but this code isn't changing much nowadays, and we could probably announce that the v1 one is deprecated and will be dropped in 6 months so it wouldn't last long.

[Maldris]

Sorry for going quiet, we've had a number of things come up for clients that has pushed the requirement for this down our pipeline a bit, but I'm still chipping away at it periodically.

I've consolidated most of the codec behaviour and several other parts, but large portions of things like query planner would need to be either re-built with an internal representation, or be largely duplicated with different types, so in that scenario I'm wondering if I should revert what I've done and just make a v2 variant as you've suggested.

In the meantime, I'll experiment with that on another branch, and I or my team members may open some other issues and PRs around other smaller things we've found that are client facing in our system that could benefit the community more broadly, once we finish verifying them.

[vangent]

just make a v2 variant as you've suggested

Let's go ahead and do that. It sounds like it might be less work now overall, and it will also be less work in 6 months or whatever when we remove support for V1.