-
Notifications
You must be signed in to change notification settings - Fork 15
Add python support in integration tests #9
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
Conversation
| AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} | ||
| # Todo : retrieve this layer version automatically from AWS | ||
| NODE_LAYER_VERSION: 55 | ||
| PYTHON_LAYER_VERSION: 41 |
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.
Do we still have a plan to automatically retrieve this from AWS?
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.
✅
| ) | ||
|
|
||
| var ( | ||
| extensionName = "recorder-extension" // extension name has to match the filename |
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.
Nit: Any reason this is a var and not a const?
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.
not at all !
| return &res, nil | ||
| } | ||
|
|
||
| // Start begins running the sidecar |
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.
What is "the sidecar" here?
| fmt.Printf("Error while JSON encoding the Log") | ||
| } | ||
| stringJsonLog := string(jsonLog) | ||
| // if we log an unwanted log, it will be availble in the next log api payload -> infinite loop |
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.
Nit: availble -> available
| http.HandleFunc("/api/v1/check_run", func(w http.ResponseWriter, r *http.Request) { | ||
| }) | ||
|
|
||
| // port 8080 is used by the Lambda Invoke API |
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.
I would move this comment to where you specify the port as 3333.
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.
Looks great! Just left a few comments.
Uh oh!
There was an error while loading. Please reload this page.