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

Instantiate constructs after all variables are resolved #31

Closed
wants to merge 4 commits into from
Closed

Instantiate constructs after all variables are resolved #31

wants to merge 4 commits into from

Conversation

mnapoli
Copy link
Member

@mnapoli mnapoli commented Jun 14, 2021

This should fix #29.

On master:

  • constructs are instantiated in the constructor of the Lift plugin
  • that means that variables resolved after the Lift plugin is instantiated are not resolved in constructs

In this branch:

  • constructs are instantiated in the initialize hook (at this point, all variables are resolved)
  • to achieve that, commands are now defined statically
  • and Lift variables are resolved lazily using CDK tokens (CDK lazy variables)

TODO:

  • additional testing

@mnapoli
Resolve construct variables lazily using CDK tokens
3fe764f
@mnapoli
Separate the definition of a construct from the construct itself
d4d68ef 
That allows to define commands without instantiating constructs, resolving the chicken and egg problem of booting everything.
@mnapoli mnapoli mentioned this pull request Jun 14, 2021
Closed
@mnapoli
Merge branch 'master' into lazy-variable-resolution-2
6c1744e 
# Conflicts:
#	src/classes/Construct.ts
#	src/constructs/Queue.ts
#	src/constructs/Storage.ts
@mnapoli
Fix linting errors
f32b35a
@mnapoli
Copy link
Member Author

mnapoli commented Jun 14, 2021

OK now I'm facing the problem described in #8 again (the failing test). Will continue to think about this.

@mnapoli
Copy link
Member Author

mnapoli commented Jun 14, 2021

Idea: try implementing an asyncInit() method (https://github.com/serverless/serverless/blob/c563581ac98764edf653c1a5337d1b7d2b61ea63/lib/classes/PluginManager.js#L751-L753) instead of creating constructs in initialize().

@mnapoli mnapoli mentioned this pull request Jun 21, 2021
Merged
1 task
@mnapoli
Copy link
Member Author

mnapoli commented Jun 21, 2021

Replaced by #50

@mnapoli mnapoli closed this Jun 21, 2021
@mnapoli mnapoli deleted the lazy-variable-resolution-2 branch June 21, 2021 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

${param} variables are not resolved in constructs
1 participant
@mnapoli