Skip to content

Switch to HTTP APIs by default #634

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 8 commits into from
Sep 14, 2021
Merged

Switch to HTTP APIs by default #634

merged 8 commits into from
Sep 14, 2021

Conversation

mnapoli
Copy link
Contributor

@mnapoli mnapoli commented Jul 23, 2021

The goal is to move towards HTTP APIs instead of REST APIs, at least for the most common scenarios.

This PR updates some examples, and duplicates some other examples. More specifically, it:

  • introduces new examples focused on HTTP APIs (they duplicate existing "rest-api" examples, in order to preserve examples about REST APIs)
  • update existing examples that were not focused on REST APIs

The goal was to keep examples about REST (not remove them entirely).

Some examples, like "expressjs", "simple HTTP endpoint", etc. were not specifically about REST APIs, so I updated them to use HTTP APIs.

TODO:

Copy link
Contributor

@pgrzesik pgrzesik left a comment

Choose a reason for hiding this comment

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

Looks good in general, good call 👍 I have only one concern, other than that we should be good to go here

ProvisionedThroughput:
ReadCapacityUnits: 1
WriteCapacityUnits: 1
BillingMode: PAY_PER_REQUEST
Copy link
Contributor

Choose a reason for hiding this comment

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

good call 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 some templates had this, some others didn't. I figured we might as well make it consistent over this option, it's best to get started.

@mnapoli mnapoli force-pushed the switch-to-http-api branch from c59ddb4 to 3343a4c Compare July 29, 2021 13:23
@mnapoli mnapoli marked this pull request as ready for review September 2, 2021 09:45
@mnapoli
Copy link
Contributor Author

mnapoli commented Sep 2, 2021

This PR is ready.

After merging this, the next step will be serverless/serverless#9777 (and what's in TODO in that PR). This PR here can be merged and that will unblock the other tasks (@pgrzesik let me know if I'm wrong and we need to merge something simultaneously, but I don't think this is the case, I think we can merge this now and do serverless/serverless#9777 afterwards).

@pgrzesik
Copy link
Contributor

pgrzesik commented Sep 2, 2021

Thanks @mnapoli - in general we should always merge changes here first if we're adding new examples. If we're modifying existing ones, then the onboarding will use them right after they are merge here. It's going to be the case e.g. for aws-node-express-api from your changes. Are you okay that from now on, even in previous versions, that example will use HTTP API instead of REST API?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants