Conversation
* 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 <>
gregfurman
requested review from
dfangl,
dominikschubert and
joe4dev
as code owners
| "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.
There was a problem hiding this comment.
Go has some pretty opinionated formatting 🤷 Unsure why it never kicked in for the original changes.
cmd/localstack/custom_interop.go
Outdated
| // 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
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.
There was a problem hiding this comment.
Thank you for separating that breaking change 👍
I think we need DRG-327 to ship this safely.
| "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.
go.mod
Outdated
| 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?
| // 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?
joe4dev
left a comment
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 🙏
6 participants
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
internalpackage, and reworks our repo to reflect this.Changes
85e5302..HEADfrom upstream.internalapproach, as well as using the AWS providedInitHandlerin favor of our own incmd/awsutils.Relevant items