-
Notifications
You must be signed in to change notification settings - Fork 4k
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(batch): add default AWS_ACCOUNT and AWS_REGION to Batch container, if they are not explicitly set #21041
Merged
Changes from 31 commits
Commits
Show all changes
41 commits
Select commit
Hold shift + click to select a range
fb6de3d
Merge remote-tracking branch 'remote/master'
Dzhuneyt c038921
fix(ec2): `UserData.addSignalOnExitCommand` does not work in combinat…
corymhall 6a35cbd
Merge branch 'aws:master' into master
Dzhuneyt f2958c6
Merge branch 'aws:master' into master
Dzhuneyt 5531ac1
Merge branch 'aws:master' into master
Dzhuneyt 846ecd3
Merge branch 'aws:master' into master
Dzhuneyt ee14ff1
Merge branch 'aws:master' into master
Dzhuneyt 2720ad1
Merge branch 'aws:main' into master
Dzhuneyt 7fef2a2
feat: Pass default AWS_REGION and AWS_ACCOUNT
Dzhuneyt e726152
Merge branch 'main' into dzhuneyt/issue-10952-aws-batch-env-name
Dzhuneyt 4fbcfcb
Merge branch 'main' into dzhuneyt/issue-10952-aws-batch-env-name
Dzhuneyt cb8e781
Merge branch 'main' into dzhuneyt/issue-10952-aws-batch-env-name
Dzhuneyt cf07efd
Merge branch 'main' into dzhuneyt/issue-10952-aws-batch-env-name
Dzhuneyt 248319c
chore: Update README.md
Dzhuneyt 364c74a
feat: Pass default environment variables
Dzhuneyt d488b86
feat: Pass default environment variables
Dzhuneyt c8bd318
Merge branch 'main' into dzhuneyt/issue-10952-aws-batch-env-name
Dzhuneyt 52a9b4c
chore: Fix tests
Dzhuneyt 66d92aa
chore: Restore original formatting
Dzhuneyt ccd5dc8
chore: Update README.md
Dzhuneyt f8f765e
chore: Update README.md
Dzhuneyt 658bfaf
Merge branch 'main' into dzhuneyt/issue-10952-aws-batch-env-name
Dzhuneyt c5b0106
test: Try creating an integration test
Dzhuneyt 6c7d0e4
Merge branch 'main' into dzhuneyt/issue-10952-aws-batch-env-name
Dzhuneyt 285e3de
Merge branch 'main' into dzhuneyt/issue-10952-aws-batch-env-name
TheRealAmazonKendra 9dd3c6b
Merge branch 'main' into dzhuneyt/issue-10952-aws-batch-env-name
Dzhuneyt 552e393
Merge branch 'main' into dzhuneyt/issue-10952-aws-batch-env-name
Dzhuneyt aa4478e
chore: Update integration test snapshots
Dzhuneyt c2f1ac2
chore: Update integration test snapshots
Dzhuneyt 66dc657
chore: Update integration test snapshots
Dzhuneyt 6968942
Merge branch 'main' into dzhuneyt/issue-10952-aws-batch-env-name
Dzhuneyt 420eba8
Merge branch 'main' into dzhuneyt/issue-10952-aws-batch-env-name
Dzhuneyt 85fd70a
chore: Update integration test snapshots
Dzhuneyt d3d3323
Merge branch 'main' into dzhuneyt/issue-10952-aws-batch-env-name
Dzhuneyt 752defa
chore: Update integration test snapshot of deps
Dzhuneyt d828c92
Merge branch 'main' into dzhuneyt/issue-10952-aws-batch-env-name
Dzhuneyt 963aa6e
chore: Update integration test snapshots
Dzhuneyt 2a025dd
chore: Update integration test snapshots
Dzhuneyt 2366c3f
Merge branch 'main' into dzhuneyt/issue-10952-aws-batch-env-name
Dzhuneyt 0eab1f6
Merge branch 'main' into dzhuneyt/issue-10952-aws-batch-env-name
Dzhuneyt be06bff
Merge branch 'main' into dzhuneyt/issue-10952-aws-batch-env-name
mergify[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 changes: 2 additions & 2 deletions
4
packages/@aws-cdk/aws-batch/test/batch.integ.snapshot/batch-stack.assets.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
So, I'm wondering if always having values here is a breaking change for customers currently using this. Breaking changes are OK in an experimental module, but I'm wondering if we should make setting these defaults an option in the props instead of just doing it 100% of the time. I'm also wondering if there are use cases where a user would want this undefined. I'm not super familiar with AWS Batch so I think I need to consult with others on this.
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'm not 100% sure about this specific scenario, but generally speaking, from my experience - when we talk about breaking change in the code of some library - it's usually the removal of parameters, functions, classes, or a modification of their existing behavior, whereas additions of fields, parameters, methods are not considered a breaking change.
But again, that's just my perspective, looking at other libraries I've consumed in my projects as a developer. With CDK it's slightly different since we are considering not just the code but also things that users might have deployed using that code, and I realize it's a slightly more flavored and opinionated discussion.