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

cli: forward docker args to backend:build-image, add --build flag, and use by default #2299

Merged
merged 5 commits into from
Sep 7, 2020

Conversation

Rugvip
Copy link
Member

@Rugvip Rugvip commented Sep 6, 2020

This makes the backstage:build-image command be pretty much all you need to get a full deployment up and running. The new --build option make the command build all of the input packages before packing them into the workspace, which will be much faster than building all packages. We now also forward all unknown options to docker image build, similar to how backstage-cli test forwards options to jest.

The config is switched around a bit too, moving the development config to app-config.development.yaml, but the examples still show how to run the image with NODE_ENV=development, since there's a bunch of required config to set otherwise.

@Rugvip Rugvip requested a review from a team as a code owner September 6, 2020 12:20
@@ -1,15 +1,11 @@
app:
title: Backstage Example App
baseUrl: http://localhost:3000
baseUrl: http://localhost:7000
Copy link
Member

Choose a reason for hiding this comment

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

I think I'm a bit unclear on the role of this file. Will it be used for like e2e tests, or as the default prod setup for forkers (as opposed to create-app users) or what? Just so I'm clear on why the default setup changed to using the backend instead of dockerised frontend, AND removed cors

Copy link
Member

Choose a reason for hiding this comment

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

(Could perhaps have a comment block at the top of all the config files, including in the skeleton, that describes when and by whom it is used

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah not super clear for me either, since we're suggesting running the backend with NODE_ENV=development in docker 😅 . In general this PR switches the base app-config.yaml to be the production configuration, where the app-config.development.yaml provides overrides for local development.

CORS is moved to local development only, since it's not needed when the app is bundled with the backend.

docs/getting-started/deployment-other.md Outdated Show resolved Hide resolved
packages/cli/src/commands/index.ts Outdated Show resolved Hide resolved
@freben
Copy link
Member

freben commented Sep 7, 2020

@Rugvip would you like to communicate with #2272 about the overlap?

@freben
Copy link
Member

freben commented Sep 7, 2020

Haha strike that - it was closed but the browser tab had not refreshed :)

@dhenneke
Copy link
Contributor

dhenneke commented Sep 7, 2020

I closed #2272 because I could solve my problem differently and wasn't really happy with my changes. But these changes look awesome 🚀

@Rugvip Rugvip merged commit 1db0615 into master Sep 7, 2020
@Rugvip Rugvip deleted the rugvip/db branch September 7, 2020 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants