-
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
feat(ec2): add missing endpoints to InterfaceVpcEndpointAwsService #21401
Conversation
duplicate of my PR #21220 but you have the missing tests. |
Sure thing. I can add S3 and test cases for it todat
…On Mon, Aug 1, 2022, 12:48 AM Thorsten Hoeger ***@***.***> wrote:
duplicate of my PR #21220 <#21220> but
you have the missing tests.
Can you add S3 also? Then my PR would be superseded.
—
Reply to this email directly, view it on GitHub
<#21401 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADIMVN2KWJW46DNHLAKX4TTVW56NPANCNFSM55GNZ3AQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
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.
Just two quick notes:
- This is missing the chore/feat/fix designation at the beginning of the PR title.
- Looks like chore(ec2): add missing interface VPC endpoint services #21220 added these to the public docs. Can you do that as well?
@TheRealAmazonKendra Went ahead and added the S3 endpoint in #21220. Unfortunately it appears that integ tests aren't working correctly in the library (some tests havent been updated to use the integ-runner yet). If you think this also needs an integ test I'll have to open a seperate PR to fix those tests and add one. Second, I added TSdoc entries for the endpoints as I do not understand why we are adding these endpoints doc entries to the ignore list. Happy to update the others with proper TSdoc update if we can agree on the verbiage so they're all consistent on the doc site |
I don't think integ tests are needed here. |
Oops, my comment above said they wrong thing. We went to add them to the exclude. TBH, I'm not sure why we've excluded these but we went it to be consistent with what we've done in the past. |
Sounds good! I'll add those to the exclude list for consistency |
…ws-cdk into add_codeartifact_endpoints
Pull request has been modified.
Don't want to bloat the ticket or add scope creep, but i can add #21338 to this |
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.
Go for it! Just putting this back in changes requested so I see when that's been done.
I see there's a few more endpoints requested in issues. Feel free to add as many of those as you feel like doing in this PR. |
…ws-cdk into add_codeartifact_endpoints
Pull request has been modified.
Anymore you want to add or is this ready to go? It looks good on my end. |
@TheRealAmazonKendra This is ready to go, I think the others in that list might be a little more effort the ones i've already added |
Thank you for contributing! Your pull request will be updated from main 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 main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…ws#21401) This add the following endpoints to `InterfaceVpcEndpointAwsService`: `codeartifact.api` `codeartifact.respositories` `batch` `autoscaling` `autoscaling-plan` `application-autoscaling` closes aws#21402 aws#21220 aws#21338 aws#19420 ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Too late to ask for the bedrock-runtime endpoints? |
This add the following endpoints to
InterfaceVpcEndpointAwsService
:codeartifact.api
codeartifact.respositories
batch
s3
autoscaling
autoscaling-plan
application-autoscaling
closes #21402 #21220 #21338 #19420 #14423
All Submissions:
Adding new Unconventional Dependencies:
New Features
yarn integ
to deploy the infrastructure and generate the snapshot (i.e.yarn integ
without--dry-run
)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license