feat: Integrate with upstream - #62
Conversation
Sorry, something went wrong.
* update to 1.24 * fix changes
* chore(deps): Update go-chi to v5.2.2
* Add support for multi-tenancy --------- Co-authored-by: Chengjun Li <>
joe4dev
left a comment
There was a problem hiding this comment.
Sorry, something went wrong.
| "github.com/aws/aws-lambda-runtime-interface-emulator/internal/lambda/fatalerror" | ||
| "github.com/aws/aws-lambda-runtime-interface-emulator/internal/lambda/interop" | ||
| "github.com/aws/aws-lambda-runtime-interface-emulator/internal/lambda/testdata" | ||
| "github.com/stretchr/testify/assert" |
There was a problem hiding this comment.
nit: Is this change necessary? I guess it follows alphabetical import sorting.
Sorry, something went wrong.
There was a problem hiding this comment.
Go has some pretty opinionated formatting 🤷 Unsure why it never kicked in for the original changes.
Sorry, something went wrong.
| // The InvokeRequest is sent by LocalStack to trigger an invocation | ||
| type InvokeRequest struct { | ||
| InvokeId string `json:"invoke-id"` | ||
| InvokeId string `json:"request-id"` |
There was a problem hiding this comment.
I like that we fixed this inconsistent naming.
❓ Are we safe to ship this without breaking K8 images, which are not directly tied to LocalStack versions?
/cc @dfangl
Sorry, something went wrong.
There was a problem hiding this comment.
I'll make a corresponding change in LocalStack for this once the latest RIE changes are shipped. Then, in a single PR, we switch over RIE in addition to changing the param on invoke request.
Sorry, something went wrong.
There was a problem hiding this comment.
Thank you for separating that breaking change 👍
I think we need DRG-327 to ship this safely.
Sorry, something went wrong.
| "strings" | ||
| "time" | ||
|
|
||
| "github.com/aws/aws-lambda-runtime-interface-emulator/internal/lambda/interop" |
There was a problem hiding this comment.
Good catch 👍
So we basically need to change the imports to our fork instead of using upstream via go.amzn.com. I guess these imports might be a frequent area of conflict, so it's worth documenting the rationale in our LocalStack readme.
Sorry, something went wrong.
| github.com/shirou/gopsutil v2.19.10+incompatible | ||
| github.com/aws/aws-lambda-go v1.46.0 | ||
| github.com/go-chi/chi v1.5.5 | ||
| github.com/go-chi/chi/v5 v5.2.2 |
There was a problem hiding this comment.
❓ Why are both chi and chi/v5 gone here?
Sorry, something went wrong.
| // start runtime init. It is important to start `InitHandler` synchronously because we need to ensure the | ||
| // notification channels and status fields are properly initialized before `AwaitInitialized` | ||
| log.Debugln("Starting runtime init.") | ||
| InitHandler(sandbox.LambdaInvokeAPI(), GetEnvOrDie("AWS_LAMBDA_FUNCTION_VERSION"), int64(invokeTimeoutSeconds), bootstrap, lsOpts.AccountId) // TODO: replace this with a custom init |
There was a problem hiding this comment.
❓ Why is lsOpts.AccountId not necessary anymore?
Sorry, something went wrong.
joe4dev
left a comment
There was a problem hiding this comment.
Thank you for integrating these big upstream changes and separating our minimal changes nicely 🚀
Please trigger all relevant pipelines (LS & LS Pro without test selection) before shipping this change in LocalStack to catch potential regressions early 🙏
Sorry, something went wrong.
Motivation
There are several upstream changes that our fork had not yet pulled in. This PR cherry picks the relevant upstream changes, namely the introduction of the internal package, and reworks our repo to reflect this.
Changes
Relevant items