-
Notifications
You must be signed in to change notification settings - Fork 224
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
Feature: Wave Config for Targets #358
Conversation
Adding in tests for Target/Target Structure Updating User Guide updating tests
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.
Thank you for adding this!
Only a few minor comments to fix. I'm looking forward to merge this one.
...mit/bootstrap_repository/adf-build/shared/cdk/cdk_stacks/tests/test_default_pipeline_type.py
Outdated
Show resolved
Hide resolved
...mit/bootstrap_repository/adf-build/shared/cdk/cdk_stacks/tests/test_default_pipeline_type.py
Outdated
Show resolved
Hide resolved
...mit/bootstrap_repository/adf-build/shared/cdk/cdk_stacks/tests/test_default_pipeline_type.py
Outdated
Show resolved
Hide resolved
src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/schema_validation.py
Outdated
Show resolved
Hide resolved
src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/schema_validation.py
Outdated
Show resolved
Hide resolved
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.
Thank you! This looks great.
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.
Very minor fix to address.
...mit/bootstrap_repository/adf-build/shared/cdk/cdk_stacks/tests/test_default_pipeline_type.py
Outdated
Show resolved
Hide resolved
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.
Minor typos to fix.
...mit/bootstrap_repository/adf-build/shared/cdk/cdk_stacks/tests/test_default_pipeline_type.py
Outdated
Show resolved
Hide resolved
...mit/bootstrap_repository/adf-build/shared/cdk/cdk_stacks/tests/test_default_pipeline_type.py
Outdated
Show resolved
Hide resolved
...mit/bootstrap_repository/adf-build/shared/cdk/cdk_stacks/tests/test_default_pipeline_type.py
Outdated
Show resolved
Hide resolved
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.
LGTM
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.
LGTM!
When I launch the aws-deployment-framework-pipelines I get this error:
My deployment map is:
It seems that in the line 117 I think that the rollback of this change to: |
Hi @AngeloGelmini, thanks for reporting this. I'm sure you are aware that Wave Config is not released as part of ADF yet. Therefore we don't recommend relying on that yet, as it might be (or actually is as you noticed) unstable. We are actively debugging the code to make sure that the different features and changes introduced for the next release work together before we release ADF v3.2.0. |
Issue #, if available: #290
Description of changes:
Introduction of an optional wave configuration in the target schema.
This contains a configurable int that defines the wave size. This wave size is then used to create batches inside the target structure class.
These batches are then passed into the cdk stacks and accessed in a similar fashion to how targets were already accessed.
This PR doesn't have tests yet, as I noticed that there's no test for pipeline generation beyond Source / Build.
The default batch size is set to 50 (Max actions per stage) and in the default adf_pipeline_type, it will create one stage per wave, allowing for OUs with more than 50 accounts to be seperated over multiple stages.
Also added in an exclude property to the target for removing accounts by Id. (issue: #145 )
Also fixed an issue where if an account shared a tag with an OU and that tag was used as a Target it would break. (Issue: #296 )
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.