Closed shellcromancer closed 1 year ago
overall these are solid changes! my main areas of concern are:
- switching the KV store mutexes to pointers
- not supporting ctx in the new* functions
i also noticed a couple things that may have been missed:
- Terraform support for the validation app -- we should add this as an option in the Lambda module (docs here) and update the Terraform examples in
examples/aws/pipeline/
- new* support for Operators in
condition/condition.go
-- not sure if it was missed or isn't needed
Thanks for the quick review! The first two areas of concern definitely make sense, I believe I addressed these with the changes made today (along with other code review findings). Regarding constructors for the Operators in condition/condition.go
there weren't needed at their current state of configuration but I can also add those in now to "future proof" complexity they may develop.
I'll work on adding the Terraform support, and if necessary changes to the AppConfig update script and re-mark for review.
Description
This change introduces a new AWS Lambda Substation application designed to be deployed as for AWS AppConfig validator (part of #58). This validator works by loading the Substation configuration for Transforms and Sinks, ensuring no errors and then exiting. Substation now checks as many processor, condition, operators, KV and other settings more extensively at application load time with validation in new constructor methods which allow the validation to happen without data processing to hit condition logic.
Motivation and Context
This allows teams to be more confident that as they make changes to Substation configurations that they aren't introducing errors that lead to a halt of data processing.
How Has This Been Tested?
Components to this PR have been testing in different ways:
Types of changes
Checklist: