-
Notifications
You must be signed in to change notification settings - Fork 166
Built files are not built, only asserted when the app starts in a container. #8680
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
Conversation
Dockerfile.app
Outdated
| ENV PUB_CACHE="/project/.pub-cache" | ||
| # Let the pub server know that it is running inside a container. | ||
| # It will not rebuild web-app assets, only assert their presence. | ||
| ENV PUB_DEV_IN_CONTAINER="1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ENV PUB_DEV_IN_CONTAINER="1" | |
| ENV PUB_DEV_ENVIRONMENT="production" |
I think that's a better indicator, or even just ENVIRONMENT=production.
app/lib/frontend/static_files.dart
Outdated
| Future<void> assertExists(String path) async { | ||
| final file = File(path); | ||
| if (!file.existsSync()) { | ||
| throw AssertionError('`$path`` is missing.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this prevent startup?
We could opt to make the build fail, if they are not present, and just serve empty file or 404s if they are requested and not present.
Along with shouting in logs at startup, on the other hand crashing when not present is very reliable :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will throw in the HTTP frontend isolate, before the HTTP server is up and with that AppEngine will surely kill it. Though, you are right, it may not be a good idea, instead, we should serve 404 + shout in the logs. The current way this is is implemented will look like the build succeeded, but serving will never happen. I'll redo this.
|
Closing in favor of #8729 |
Fixes #8673.