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

(WIP) Update templates/examples for SDK v3 #319

Merged

Conversation

karthik2804
Copy link
Collaborator

@karthik2804 karthik2804 commented Jan 27, 2025

This PR updates the HTTP template to use the new V3 SDK. It currently points at 3.0.0-rc1.

To currently try out the templates, one can install it using

$ spin templates install --update --git https://github.com/karthik2804/spin-js-sdk --branch update_templates_samples_v3

TODOS:

  • Swap to point at v3.0.0 once we have a stable release
  • Update the examples to use the new template.
  • Once there is a stable v3.0.0 update all examples and generate a package-lock.json

@karthik2804 karthik2804 force-pushed the update_templates_samples_v3 branch from 0efb6dd to e181b8e Compare January 27, 2025 12:29
@karthik2804 karthik2804 marked this pull request as ready for review January 27, 2025 21:39
@@ -30,18 +22,15 @@ npm install

```typescript
const client = new S3Client({
region: "<>",
Copy link
Member

Choose a reason for hiding this comment

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

Is this intended?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah good catch - this changed in the example because I rewrote the example to use the router to both be able to list objects and also stream objects for which I hardcoded the region. I will update the readme to use the hardcoded version for now if that is acceptable.

secretAccessKey: '<>',
sessionToken: '<>',
},
region: 'us-west-2',
Copy link
Member

Choose a reason for hiding this comment

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

same as above

@@ -3,17 +3,17 @@ spin_manifest_version = 2
[application]
authors = ["karthik2804 <[email protected]>"]
description = ""
name = "s3"
name = "sqs"
Copy link
Member

Choose a reason for hiding this comment

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

Good catch.

@tschneidereit tschneidereit changed the title (WIP) Update templaes/examples for SDK v3 (WIP) Update templates/examples for SDK v3 Jan 28, 2025
Copy link
Contributor

@tschneidereit tschneidereit left a comment

Choose a reason for hiding this comment

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

This looks great! Thank you for being extremely diligent in applying changes uniformly.

I left a bunch of suggestions, but none of them are really blocking—except for updating to 3.0 of the SDK: I think we should land this once we've published that. Alternatively we can land it now without a new release, and plan for a very quick follow-up

examples/aws/s3/README.md Outdated Show resolved Hide resolved
examples/aws/s3/package.json Outdated Show resolved Hide resolved
examples/aws/s3/spin.toml Show resolved Hide resolved
examples/aws/s3/webpack.config.js Outdated Show resolved Hide resolved
// Any unmatched route will return a 404
router
.get("/", async () => {
let command = new SendMessageCommand(params);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this dynamically make use of request state somehow? If not, we should move it outside the request handler, so it happens at wizening time.

examples/spin-host-apis/spin-kv/webpack.config.js Outdated Show resolved Hide resolved
examples/spin-host-apis/spin-llm/webpack.config.js Outdated Show resolved Hide resolved
examples/spin-host-apis/spin-mqtt/webpack.config.js Outdated Show resolved Hide resolved
examples/upstash/qstash/src/index.ts Outdated Show resolved Hide resolved
examples/upstash/vector/src/index.ts Outdated Show resolved Hide resolved
Copy link

@itowlson itowlson left a comment

Choose a reason for hiding this comment

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

I love the way you've got rid of the spin.ts file, and re-established a distinction between Spin stuff and third-party stuff like the router. I left notes on a couple of things I was puzzled by, but they're nits: this is storming stuff.

templates/http-js/content/src/index.js Outdated Show resolved Hide resolved
templates/http-js/content/src/index.js Outdated Show resolved Hide resolved
templates/http-ts/content/spin.toml Show resolved Hide resolved
@karthik2804 karthik2804 force-pushed the update_templates_samples_v3 branch from d951c50 to 045f9b3 Compare January 28, 2025 22:02
@karthik2804 karthik2804 merged commit cb6a784 into spinframework:main Jan 28, 2025
5 checks passed
@karthik2804 karthik2804 deleted the update_templates_samples_v3 branch January 28, 2025 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants