Skip to content

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

Merged
Rugvip merged 5 commits intomasterfrom
rugvip/db
Sep 7, 2020
Merged

cli: forward docker args to backend:build-image, add --build flag, and use by default#2299
Rugvip merged 5 commits intomasterfrom
rugvip/db

Conversation

@Rugvip
Copy link
Copy Markdown
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
Comment thread app-config.yaml
app:
title: Backstage Example App
baseUrl: http://localhost:3000
baseUrl: http://localhost:7000
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

Comment thread docs/getting-started/deployment-other.md Outdated
Comment thread packages/cli/src/commands/index.ts Outdated
@freben
Copy link
Copy Markdown
Member

freben commented Sep 7, 2020

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

@freben
Copy link
Copy Markdown
Member

freben commented Sep 7, 2020

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

@dhenneke
Copy link
Copy Markdown
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.

3 participants