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.

Sign up for GitHub

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

Jump to bottom

core: Disable export-by-default #903

Closed
rix0rrr opened this issue Oct 11, 2018 · 2 comments · Fixed by #1624
Closed

core: Disable export-by-default #903

rix0rrr opened this issue Oct 11, 2018 · 2 comments · Fixed by #1624
Labels
feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md

Comments

@rix0rrr
Copy link
Contributor

rix0rrr commented Oct 11, 2018

This might run us into Export limits at some point (50-ish per account?)

Suggested fix: only make them exports once makeImportValue() is called (or export: true is passed).
@rix0rrr rix0rrr added bug This issue is a bug. good first issue Related to contributions. See CONTRIBUTING.md labels Oct 11, 2018
rix0rrr pushed a commit that referenced this issue Oct 12, 2018
fix(cdk): only make Outputs Exports when necessary
91066a7 
Exports are a limited limited resource, so automatically turning
every Output into an Export is a little wasteful. We'll turn
them into exports when requested or when .makeImportValue() is called.

Fixes #903.
@rix0rrr rix0rrr added enhancement and removed bug This issue is a bug. labels Nov 7, 2018
@srchase srchase added feature-request A feature should be added or improved. and removed enhancement labels Jan 3, 2019
@rix0rrr rix0rrr changed the title core: all Outputs are Exports by default core: Disable export-by-default Jan 24, 2019
@rix0rrr rix0rrr mentioned this issue Jan 24, 2019
Closed
@eladb
Copy link
Contributor

eladb commented Jan 27, 2019

related: #1095
we are going to stop using this pattern anyway

rix0rrr pushed a commit that referenced this issue Jan 28, 2019
fix(cdk): only make Outputs Exports when necessary
d7328c6 
Export names must be unique and can conflict, so automatically turning
every Output into an Export can lead to problems for customers who
deploy the same template multiple times. Especially when the outputs
are created for them and they have no control over them.

We'll turn Outputs into exports on-demand (when .makeImportValue() is
called).

Fixes #903, fixes #1611.
@rix0rrr rix0rrr mentioned this issue Jan 28, 2019
Merged
4 tasks
rix0rrr added a commit that referenced this issue Feb 5, 2019
@rix0rrr
fix(cdk): only make Outputs Exports when necessary (#1624)
ebb8aa1 
Export names must be unique and can conflict, so automatically turning
every Output into an Export can lead to problems for customers who
deploy the same template multiple times. Especially when the outputs
are created for them and they have no control over them.

We'll turn Outputs into exports on-demand (when .makeImportValue() is
called).

Fixes #903, fixes #1611.
@tsykora-verimatrix
Copy link

I didn't find a way how to disable outputs in CDK. Was this feature implemented? I can't see it in the documentation.
Without it the 200 resource hard-limit per template doesn't seem achievable.
I'm running into the number of outputs limit per template although all of those outputs are automatically generated by CDK, I do not generate any outputs in my code:
failed: Error [ValidationError]: Template format error: Outputs count 77 is greater than max allowed 60

It seems like CF/CDK prefer many small stacks over fewer bigger stacks, is it true @rix0rrr @eladb ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(cdk): only make Outputs Exports when necessary aws/aws-cdk
4 participants
@rix0rrr @eladb @srchase @tsykora-verimatrix