[WIP] Feature aws json #288
Conversation
Admiral-Piett
force-pushed
the
feature-aws-json
branch
from
eb9d898
to
39895d4
Compare
|
Hi folks, I'd like to offer some testing support on this issue. Like others, we have been pinned to an old version of the AWS SDK to mitigate this, but we are starting to feel some pressure to resolve. Some of my colleagues suggest switching to LocalStack, but I am fighting to stick with goaws, as it has been flawless for us until now (and I have a painful history with LocalStack). I have a suite of tests I can run in a vagrant environment with a goaws container, and I can quickly switch between AWS SDK versions and goaws versions (including image built from We are using .NET AWS SDK. Our test status right now:
I'll keep a watch on this PR and contribute where I can. |
Admiral-Piett
force-pushed
the
feature-aws-json
branch
from
7893a9a
to
39895d4
Compare
|
@Matthew-Davey That's awesome, thank you! The more the merrier! I would definitely appreciate any support on this, it's a hefty feature. |
|
@Matthew-Davey @kojisaiki @kojisaiki @mancej I think I've got something here for the pattern we were talking about before. I think it's workable and, to me anyway, simplifies things for once we get into the actual Action level logic. Let me know what you think when you have time! |
Admiral-Piett
force-pushed
the
feature-aws-json
branch
2 times, most recently
from
6733100
to
44e22ae
Compare
|
@kojisaiki I just squashed some commits in here that I just now noticed you committed from a while ago - just to make things a little easier to manage. After looking closer though - are those still valid? They look like they're tests/pseudo code to explore the pattern early on? Anyway, they conflict with my PR now. Do you have any complaints if I get rid of them so I can rebase onto this cleanly? |
No problem to squash completely my old commit 👍 |
Admiral-Piett
force-pushed
the
feature-aws-json
branch
from
44e22ae
to
d6568e7
Compare
|
Thanks @kojisaikiAtSony! Apparently I had incorporated them into my last commit on the Regardless, I just merged so I think we're ready. Did you wind up having time to take a look at this? If so, do you know what API you were looking at? I'll mark you down on the list so I can grab something new? |
|
I tried to migrate SendMessage in an old PR, but I can look into it again with latest V1 flow 👍 While I'm in charge of SendMessage, I'd like to mark the name of the person in charge so that other people don't have to duplicate their efforts. |
|
Sounds great @kojisaikiAtSony - can you edit the description above? I added a list - and put my name against the first one. Want to follow that pattern? |
|
Looks good! but I cannot edit the description... |
|
@kojisaikiAtSony Boo GitHub 🙂. I guess because it's "my PR", I'll see if I can change that. I put you on there for now though. Thanks for hacking on SendMessage! |
|
Hi @Admiral-Piett , sorry it's too late, but I can start again. I may be able to get other team members to help us out, so can I get assignments for other APIs as well?
|
Yep, you got it! @kojisaikiAtSony |
|
Hello! I submitted a PR for sqs.ReceiveMessage, sqs.ChangeMessageVisibility, and sqs.DeleteMessage. I had started on sqs.ReceiveMessage just to see if I could make any progress before committing to the refactor here, but then ended up getting a working implementation. ChangeMessageVisibility and DeleteMessage were also needed for what I'm working on, and they both built nicely on top of ReceiveMessage, so I added them to the same PR |
Squashed commits: [7342cdd] Update [f1de347] Add tests [28011fb] fix test [a2d30b4] Add SetAttributesFromForm test [8464892] Add xml smoke test [fe667e4] Update UT [14c8798] Update by reviews [29dc396] Add a test about message size exceeding [9083ad0] Add a test [4efb39a] Fix MessageAttributes propagation [85b0f0b] separate unit test [29b45a3] move model into same model files + update a test [06db0ee] add 1 smoke test [031e67e] test commit [f3062f3] Migrate SQS SendMessage w/o smoke test test commit add 1 smoke test move model into same model files + update a test separate unit test Fix MessageAttributes propagation Add a test Add a test about message size exceeding Update by reviews Update UT Add xml smoke test Add SetAttributesFromForm test fix test Add tests Update add TODO comment
|
@Admiral-Piett
|
|
@Admiral-Piett
|
fixes comment
added ut fix review ref review ref resolve conflicts rm sqs_messages.go
minor fixes add not implemented comment to sns model reset tests affecting new tests add debug message when listing sns topics remove merge remnants, minor fixups update listtopics test to handle new mock config
add request transform test to delete topic revise comments - change deletetopicv1 println to log.Info - remove unnecessary deletetopicv1 request setAttributesFromForm update deletetopics test to handle new mock config
add bad request ut case to listsubscriptionsv1 rename listSubscriptions test case review ref
rename review ref review ref
review ref
update udpate fixed update
|
@kojisaikiAtSony I want to thank you so much for your hard work on all this! You beared with me through this whole overhaul and it's down to you on how much I think we've improved the tool while incorporating this whole new feature. If you ever have interest in carrying on with GoAWS let me know! I'd be honored to work with you again, and I'd be happy to make you a contributor on the project. Thanks again for everything! Here's to version 0.5.0! |
|
@Admiral-Piett thank you for accepting our contribution! As goaws users, we will try the new version soon! We will also continue to contribute with issues and PRs 👍 |
WIP for the AWS json full feature.
NOTE
If you plan to contribute to this feature, please PR against this branch. We designed the first supported endpoint to (hopefully) provide a foundation/pattern on which to build the subsequent changes. Once we get them all, we'll release this.
To make sure we don't trip on each other, please update the list below by putting your name next to the API you intend to work on. Once it gets into PR we can
xthat API and move on. Thank you in advance for any support/tests/reviews/code work!JSON Supported APIs
SQS
SNS
SNS Internal
Steps For PR:
a. Implement
<NewStruct>.SetAttributesFromForm- even if it just returns without doing anything.b. You may also need to handle
UnmarshalJSONfor any nested request attributes/objects.RequestFunctionV1functionV1routing tableutils.REQUEST_TRANSFORMERwith your new structcreateErrorResponseto usecreateErrorResponseV1smoke_tests/...(use the v2 SDK as much as possible)