-
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
fix(opensearchservice): imported domain's domainendpoint
is a url not an endpoint
#18027
Conversation
Title does not follow the guidelines of Conventional Commits. Please adjust title before merge. |
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.
Looks good @joejegan! Just a minor nit that I added in
domainendpoint
is a url not an endpoint
Pull request has been modified.
Whoops. I'm glad this didn't get merged because this is a breaking change to a stable module, which is not allowed. Users may be using This may be an allowed breaking change, but at the very least we'd have to document it. Give me a second to figure it out. |
domainendpoint
is a url not an endpointdomainendpoint
is a url not an endpoint
Pull request has been modified.
Now this PR relies on #18102. It's a little controversial; we may have to find some other workaround. |
…18102) This PR introduces a proposed new label, `pr-linter/exempt-breaking-change` that, when added, circumvents the check that asserts stable modules do not have breaking changes. Motivation: A situation like #18027 where we have are willing to accept a functional breaking change to a stable module. The regular `allowed-breaking-changes.txt` file does not work here, since there is no breaking change to the API. We want to be able to document the breaking change, but by documenting we alert `prlint` that we are breaking a stable module. Counterargument: Functional breaking changes were explicitly banned in #14861. From the PR description: "The CDK must be more strict on preventing such changes and the impact due to their perception." I also added some "manual linting" to the file myself since it was bothering me, and now it muddies the diff. Sorry! ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…ws#18102) This PR introduces a proposed new label, `pr-linter/exempt-breaking-change` that, when added, circumvents the check that asserts stable modules do not have breaking changes. Motivation: A situation like aws#18027 where we have are willing to accept a functional breaking change to a stable module. The regular `allowed-breaking-changes.txt` file does not work here, since there is no breaking change to the API. We want to be able to document the breaking change, but by documenting we alert `prlint` that we are breaking a stable module. Counterargument: Functional breaking changes were explicitly banned in aws#14861. From the PR description: "The CDK must be more strict on preventing such changes and the impact due to their perception." I also added some "manual linting" to the file myself since it was bothering me, and now it muddies the diff. Sorry! ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…ot an endpoint (aws#18027) imported Domain's domainEndpoint should be a endpoint and not a url. Fixes [aws#18017](aws#18017) BREAKING CHANGE: imported domain property `domainEndpoint` used to contain `https://` prefix, now the prefix is dropped and it returns the same value as a `domainEndpoint` on a created domain ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
imported Domain's domainEndpoint should be a endpoint and not a url.
Fixes #18017
BREAKING CHANGE: imported domain property
domainEndpoint
used to containhttps://
prefix, now the prefix is dropped and it returns the same value as adomainEndpoint
on a created domainBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license