Skip to content
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

[aws-ec2] Isn't IVpc supposed to be implemented by users? #11406

Closed
2 tasks
foriequal0 opened this issue Nov 11, 2020 · 4 comments
Closed
2 tasks

[aws-ec2] Isn't IVpc supposed to be implemented by users? #11406

foriequal0 opened this issue Nov 11, 2020 · 4 comments
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p1

Comments

@foriequal0
Copy link

Current IVpc has both properties and methods. These methods are implemented in VpcBase, and they are reused throughout the aws-cdk codebases.
I've built my MyVpc construct with L1 constructs from scratch to match my requirements. (#5927 (comment)) As I've commented, they don't play well with existing L2 constructs.
In order to plug my MyVpc into other L2 constructs, I have to re-implement methods that are already implemented in VpcBase. It is tedious and worrisome.
I'm getting an impression that IVpc isn't supposed to be implemented by users.

Use Case

Proposed Solution

To lower the barrier of implementing IVpc, we should improve the reusability of methods that are implemented in VpcBase.

  1. Separate IVpcProps from IVpc to only contain properties.
  2. Refactor methods in VpcBase to only depend on IVpcProps, not this IVpc.
  3. Replace IVpc method usages with the methods that are refactored in 2.

Other

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@foriequal0 foriequal0 added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Nov 11, 2020
@github-actions github-actions bot added the @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud label Nov 11, 2020
@rix0rrr
Copy link
Contributor

rix0rrr commented Nov 23, 2020

Are there specific reasons you cannot inherit from VpcBase?

@foriequal0
Copy link
Author

@rix0rrr It is not exported...

@rix0rrr
Copy link
Contributor

rix0rrr commented Nov 24, 2020

Oh I see.

Well yeah, this needs consideration in the VPC redesign, so good call on linking that in. I'm not sure there's something we can do at short notice without committing to an API we won't want to maintain in the future.

I'd recommend copy/pasting the class for now. Sorry I don't really have a better answer for you.

@rix0rrr rix0rrr added effort/large Large work item – several weeks of effort p1 labels Nov 24, 2020
@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Nov 25, 2020
@github-actions
Copy link

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Jun 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p1
Projects
None yet
Development

No branches or pull requests

3 participants