Feature/Eventbridge v2: Support extending available targets #11407
Conversation
| } | ||
|
|
||
| # Remove any keys with a value of None | ||
| kwargs = {k: v for k, v in kwargs.items() if v is not None} |
There was a problem hiding this comment.
Hierarchical configs can still contain None values. Example: NetworkConfiguration.awsvpcConfiguration.subnets
There was a problem hiding this comment.
they should not be passed as input arguments to overwrite potential defaults if they are empty, also if they are empty lists, especially with required values like subnets.
|
|
||
| def _validate_input(self, target: Target): | ||
| super()._validate_input(target) | ||
| if not collections.get_safe(target, "$.EcsParameters.TaskDefinitionArn"): | ||
| raise ValueError("EcsParameters.TaskDefinitionArn is required for ECS target") | ||
| ecs_parameters = target.get("EcsParameters", {}) | ||
| if ecs_parameters.get("LaunchType", {}) == "FARGATE": |
There was a problem hiding this comment.
Are these validations based on service knowledge or manual testing?
Mostly a notice because the ext tests cover the happy path and don't snapshot any validation errors.
There was a problem hiding this comment.
provider knowledge, no testing of errors is added for now
b5d4c0f to
f923913
Compare
f578238 to
cabb5fb
Compare
7ce2910 to
b19d0b7
Compare
|
Currently, only patch changes are allowed on master. Your PR labels (aws:events, semver: minor) indicate that it cannot be merged into the master at this time. |
a52d2c5 to
e0bab5e
Compare
f7f0d39 to
c516d51
Compare
c516d51 to
9f281ea
Compare
93b2df7 to
e2c37cd
Compare
Update: Looking at the Lambda example and how service dependencies are merged, I think this dependency should be added in ext here |
Motivation
This PR adds the ability to extend the TargetSenderFactory by targets defined in localstack-pro.
Furthermore, it makes testing fixtures available to localstack-pro
Testing
Since ECS is a pro feature, the aws validated test can be found in the companion branch.