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

feat: add support for remote templates #645

Merged
merged 19 commits into from
Apr 21, 2021
Merged

feat: add support for remote templates #645

merged 19 commits into from
Apr 21, 2021

Conversation

ansgarm
Copy link
Member

@ansgarm ansgarm commented Apr 19, 2021

Currently supports specifying a public url pointing to a zip file.

Refactored some test helper methods to be async because else they block the js main loop and the template-server utility class could not serve the request for a template zip file.

Screenshot 2021-04-20 at 00 46 48

Closes #339.

To test this locally, you can use this url to a copy of our typescript template:

https://github.com/ansgarm/cdktf-template-example/archive/refs/tags/v0.0.1.zip

Open tasks

  • Finish writing docs (see todos in markdown file)
  • Create follow up issue to add a full example / tutorial of how to create a remote template to the docs -> Add step by step tutorial for creating a remote template #652
  • Check for possible issues with templates not using cdktf versions we supply as inputs to sscaff

ansgarm added 3 commits April 20, 2021 00:40
Currently supports specifying a public url to a zip file

refactored some test helper methods to be async because else they block the js main loop and the template-server could not serve the request for a template zip file
Copy link
Contributor

@skorfmann skorfmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left a few minor comments, but looks good!

docs/working-with-cdk-for-terraform/remote-templates.md Outdated Show resolved Hide resolved
packages/cdktf-cli/bin/cmds/init.ts Outdated Show resolved Hide resolved
packages/cdktf-cli/bin/cmds/init.ts Outdated Show resolved Hide resolved
packages/cdktf-cli/lib/util.ts Show resolved Hide resolved
ansgarm added 3 commits April 20, 2021 16:09
apparently jest does not wait for an async function to complete in BeforeAll hooks (see: jestjs/jest#9527). With the default timeout of 5 seconds this had the effect of our test cases being run without initialization to be complete. this was no issue prior as we used to use execSync which blocked the whole js main loop and thereby also blocked jest in running our test cases.

switched exec to spawn for some test-helper shell calls. this allows to capture all logs and can also allow to print them as they appear (line per line) which helped discover the cause of the jest related issue
@ansgarm ansgarm marked this pull request as ready for review April 20, 2021 21:21
@ansgarm ansgarm requested a review from skorfmann April 21, 2021 11:26
Copy link
Contributor

@skorfmann skorfmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! 👍

@ansgarm ansgarm merged commit 6d60757 into main Apr 21, 2021
@ansgarm ansgarm deleted the ansgarm/issue339 branch April 21, 2021 12:39
@github-actions
Copy link
Contributor

I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remote Templates
2 participants