-
Notifications
You must be signed in to change notification settings - Fork 812
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
docstore/awsdynamodb: Support AWS SDK V2 #3458
Comments
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 Thoughts on this roadmap? |
Doesn't this exist already? E.g., for |
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.
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. 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? |
Let's use this bug to cover migrating 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 |
See #3459 for the other bug, I couldn't figure out how to cc you. |
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. |
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., |
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. |
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. |
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.
The text was updated successfully, but these errors were encountered: