-
Notifications
You must be signed in to change notification settings - Fork 30
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
Set explicit defaults w/ placeholders in the cdk.context.json #1021
Set explicit defaults w/ placeholders in the cdk.context.json #1021
Conversation
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1021 +/- ##
=========================================
Coverage 80.20% 80.20%
Complexity 2722 2722
=========================================
Files 365 365
Lines 13603 13603
Branches 941 941
=========================================
Hits 10910 10910
Misses 2118 2118
Partials 575 575
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -1,17 +1,17 @@ | |||
{ | |||
"demo-deploy": { |
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.
Replace demo-deploy name with <stage>
or something else.
"reindexFromSnapshotServiceEnabled": true, | ||
"trafficReplayerServiceEnabled": true, | ||
"otelCollectorEnabled": true |
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 don't think this should be changed right now but this is an implementation detail. A user shouldn't have to know what OTEL is to use our solution.
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.
Oh, yeah, I actually meant to pull this one out. It's set to true in the defaults, so they get it anyways
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
}, | ||
"sourceCluster": { | ||
"endpoint": "<SOURCE_CLUSTER_ENDPOINT>", | ||
"auth": { "type": "none" } |
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.
What about having options in here, even if not valid json? like
"auth": {
"type": "none|basic|sigv4"
// if basic
"username"
"password_from_secret_arn"
}
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 like this! Incorporating it now
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
daaf9b0
into
opensearch-project:main
Description
[Describe what this change achieves]
Issues Resolved
[List any issues this PR will resolve]
Is this a backport? If so, please add backport PR # and/or commits #
Testing
[Please provide details of testing done: unit testing, integration testing and manual testing]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.