-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
fix(stepfunctions-tasks): jobQueueArn support JsonPath or JSONata #33670
Conversation
…Nata
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.
(This review is outdated)
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #33670 +/- ##
=======================================
Coverage 82.38% 82.38%
=======================================
Files 120 120
Lines 6937 6937
Branches 1170 1170
=======================================
Hits 5715 5715
Misses 1119 1119
Partials 103 103
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
FROM public.ecr.aws/lambda/python:3.6 | ||
FROM python:3.13-alpine |
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.
Can you clarify the motivation for this change?
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.
Yes sure. The context is:
- Back in the day, Lambda image behaviour was like executing a job: run and exit
- Now, all Lambda image act like a backend server: keep running forever
When I tried the old Dockerfile, StateMachine execution never escape RUNNING state as ECS serve the image as a server.
So I decided to use the plain python image.
One weird thing is although there is a test using the changed Dockerfile, no snapshot change was detected. So I only ran my new test file.
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.
One weird thing is although there is a test using the changed Dockerfile, no snapshot change was detected. So I only ran my new test file.
After checking the implementation of DockerImageAsset I don't see any logic which use the docker file's content to compute asset's hash.
Hi @phuhung273, thank you for this PR. I am good with the changes, and I can approve after you answer my comment. |
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.
(This review is outdated)
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Comments on closed issues and PRs are hard for our team to see. |
Issue # (if applicable)
Closes #33580
Reason for this change
Incorrect IAM Policy for jobQueueArn when using JsonPath or JSONata
Description of changes
For JsonPath or JSONata
jobQueueArn
, IAM Policy use wildcard (*)Describe any new or updated permissions being added
For JsonPath or JSONata
jobQueueArn
, IAM Policy use wildcard (*)Description of how you validated changes
Unit test
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license