-
Notifications
You must be signed in to change notification settings - Fork 83
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
feat: upload go binaries as al2-compatible zip #490
Conversation
✅ Deploy Preview for open-api ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how are we making sure that binaries we get are ready for this runtime? they need to use a certain version of the Lambda Go SDK right?
The binary needs to be built with |
hmm, i was under the impression that you needed to run a minimum version of the Go SDK for the provided runtime to be supported, but i can't find any docs on this. i guess that must have been possible for a long time. |
|
||
assert.Equal(t, "js", jsFunction.Runtime) | ||
assert.Equal(t, "py", pyFunction.Runtime) | ||
assert.Equal(t, "rs", rsFunction.Runtime) | ||
assert.Equal(t, "go", goFunction.Runtime) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there's no manifest file, there's no change. We're still uploading stuff as go
, for now.
assert.Equal(t, "a-runtime", functions.Files["hello-js-function-test"].Runtime) | ||
assert.Empty(t, functions.Files["hello-js-function-test"].FunctionMetadata.InvocationMode) | ||
assert.Equal(t, "some-other-runtime", functions.Files["hello-py-function-test"].Runtime) | ||
assert.Equal(t, "stream", functions.Files["hello-py-function-test"].FunctionMetadata.InvocationMode) | ||
assert.Equal(t, "provided.al2", functions.Files["hello-go-binary-function"].Runtime) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there's a runtimeVersion: "provided.al2"
in the manifest, we now pick that up and send it to the API as the runtime.
I've made some updates to go along with netlify/zip-it-and-ship-it#1609, and would love another review. |
Uh-oh, the tests are failing on windows, probably because open-api/go/porcelain/deploy.go Lines 919 to 944 in b2b8227
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. CI seems to be complaining but I don't know how relevant it is since we've changed the porcelain
part and nothing on the auto generated bit - https://github.com/netlify/open-api/actions/runs/6494408840/job/17671324967?pr=490#step:5:10
Ensures that Go binaries that have
provided.al2
as theirruntimeVersion
are uploaded in aprovided.al2
-compatible ZIP. This means that the name of the binary inside the created ZIP needs to bebootstrap
, and that we're sendingruntime=provided.al2
to the Netlify API.Part of https://github.com/netlify/pod-dev-foundations/issues/581.