Skip to content
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

Add Serverless API Template for Ruby Runtimes (3.2 and 3.3) #511

Merged
merged 6 commits into from
Oct 31, 2024

Conversation

toydestroyer
Copy link
Contributor

Description of changes:

Adding Serverless API template for Ruby runtimes (both 3.2 and 3.3).

This template implements the following CRUD functions:

  • CreateItemFunction with DynamoDBCrudPolicy, exposed through the POST / endpoint.
  • GetAllItemsFunction and GetItemByIdFunction with DynamoDBReadPolicy, exposed through the GET / and GET /{id} endpoints, respectively.
  • UpdateItemFunction with DynamoDBCrudPolicy, exposed through the PUT /{id} endpoint.
  • DeleteItemFunction with DynamoDBCrudPolicy, exposed through the DELETE /{id} endpoint.

Each function is tested with minitest and includes corresponding JSON events in the events/ directory for local invocation.

P.S. I took the liberty of moving the Runtime, Architectures, MemorySize, and Timeout directives to the Globals section. Please let me know if you prefer these to be explicitly defined for each function, as they are in the nodejs20.x/web template.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@toydestroyer toydestroyer requested a review from a team as a code owner September 18, 2024 16:02
@toydestroyer toydestroyer requested review from dkphm and jysheng123 and removed request for a team September 18, 2024 16:02
@github-actions github-actions bot added pr/external stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. labels Sep 18, 2024
Copy link
Contributor

@lucashuy lucashuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for creating this PR! It looks fine in general, no issues from me with using the Globals section for all those properties.

Could you add the templates into our testing framework? The logic itself won't be tested since this requires AWS credentials, but we just want to make sure that its included in the CI checks.

There are two locations:

These two files are pretty much identical. Basically, these two test files will run the Ruby test files inside of the new template file.

While that is happening, I can validate the behaviour of the template itself.

@toydestroyer
Copy link
Contributor Author

Could you add the templates into our testing framework?

Done in 1e362fd. I wasn’t sure which name to include in the TODO comment, so I left it blank.

@toydestroyer
Copy link
Contributor Author

Actually, does this mean that ruby3.3 is now supported by cfn-lint?

https://github.com/aws-cloudformation/cfn-lint/blob/4a57bc9f626992e3d9a67fc68f995d669279eac8/src/cfnlint/data/schemas/providers/us_east_1/aws-lambda-function.json#L429

Should I remove should_test_lint = False altogether?

@lucashuy lucashuy removed the stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. label Sep 25, 2024
@lucashuy
Copy link
Contributor

Yup, it looks like sam validate --lint works fine with Ruby 3.3 functions, so we should remove that line in the tests now.

@toydestroyer
Copy link
Contributor Author

Removed 👍

@toydestroyer
Copy link
Contributor Author

Hm, I see that test failed because it couldn't find aws-sdk-dynamodb, but I'm not sure if b026283 will help :suspect:

@toydestroyer
Copy link
Contributor Author

I’ve checked the CI on my repo, and yes, b026283 does help! @lucashuy, could you please take another look at this PR?

Copy link
Contributor

@hawflau hawflau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@toydestroyer
Copy link
Contributor Author

@hawflau thanks for the review!

Added build_invoke tests in 6b2b4c0. I had to introduce a new BuildInvoke class because I have more JSON events than node/dotnet versions.

Copy link
Contributor

@hawflau hawflau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for your contribution!

@mndeveci mndeveci added this pull request to the merge queue Oct 21, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 21, 2024
@super132 super132 added this pull request to the merge queue Oct 28, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 28, 2024
@hnnasit hnnasit added this pull request to the merge queue Oct 31, 2024
Merged via the queue into aws:master with commit f10c876 Oct 31, 2024
46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants