search

Admiral-Piett / goaws

AWS (SQS/SNS) Clone for Development testing
MIT License
778 stars 145 forks source link

[WIP] Feature aws json #288

Closed Admiral-Piett closed 2 weeks ago

Admiral-Piett commented 8 months ago

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 x that 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:

  1. Create a model for your new Request struct a. Implement <NewStruct>.SetAttributesFromForm - even if it just returns without doing anything. b. You may also need to handle UnmarshalJSON for any nested request attributes/objects.
  2. Create a RequestFunctionV1 function
  3. Swap the reference in the routing table to the new V1 routing table
  4. Copy the code from the old function
  5. Call utils.REQUEST_TRANSFORMER with your new struct
  6. Update fetches from the request Form to look at the new struct
  7. Make the new V1 function return a status and response struct
  8. Update any references to createErrorResponse to use createErrorResponseV1
  9. Delete old/non-V1 function
  10. Update/Create/Remove unit tests
  11. Smoke Test with the SDK
  12. Add automated tests in smoke_tests/... (use the v2 SDK as much as possible)
  13. PR
Matthew-Davey commented 8 months ago

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 feature-aws-json branch).

We are using .NET AWS SDK.

Our test status right now:

AWS SDK goaws Pass/Fail
3.7.200.65 admiralpiett/goaws:v0.4.5 :heavy_check_mark:
3.7.300.37 admiralpiett/goaws:v0.4.5 :heavy_multiplication_x:
3.7.200.65 feature-aws-json docker build . :heavy_check_mark:
3.7.300.37 feature-aws-json docker build . :heavy_multiplication_x:

I'll keep a watch on this PR and contribute where I can.

Admiral-Piett commented 8 months ago

@Matthew-Davey That's awesome, thank you! The more the merrier! I would definitely appreciate any support on this, it's a hefty feature.

Admiral-Piett commented 8 months ago

@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!

https://github.com/Admiral-Piett/goaws/pull/289

Admiral-Piett commented 7 months ago

@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?

kojisaikiAtSony commented 7 months ago

@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 👍 I closed my initial PR https://github.com/Admiral-Piett/goaws/pull/285

Admiral-Piett commented 6 months ago

Thanks @kojisaikiAtSony! Apparently I had incorporated them into my last commit on the CreateQueueV1 and forgotten - sorry about that. So they're there now. 😄

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?

kojisaikiAtSony commented 6 months ago

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.

Admiral-Piett commented 6 months ago

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?

kojisaikiAtSony commented 6 months ago

Looks good! but I cannot edit the description... We may be able to use other tools (Github Project, Google SpreadSheet...) to track the progress and assignee. image

Admiral-Piett commented 6 months ago

@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!

kojisaikiAtSony commented 5 months ago

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?

Admiral-Piett commented 5 months ago

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?

  • SQS - GetQueueUrl
  • SNS - CreateTopic

Yep, you got it! @kojisaikiAtSony

andrew-womeldorf commented 4 months ago

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

kojisaikiAtSony commented 3 months ago

@Admiral-Piett Can I get assignments for below APIs?

kojisaikiAtSony commented 2 months ago

@Admiral-Piett My side have 3 engineers, so may I get assignment for remaining below APIs if you don't mind?

Admiral-Piett commented 2 weeks ago

@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!

kojisaikiAtSony commented 2 weeks ago

@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 👍