Skip to content

Conversation

@yograterol
Copy link

No description provided.

@coveralls
Copy link

coveralls commented May 20, 2016

Coverage Status

Coverage increased (+0.05%) to 97.794% when pulling f70e872 on yograterol:master into 9eae37f on owais:master.

@coveralls
Copy link

coveralls commented May 20, 2016

Coverage Status

Coverage increased (+0.05%) to 97.794% when pulling 89d8a43 on yograterol:master into 9eae37f on owais:master.

@coveralls
Copy link

coveralls commented May 20, 2016

Coverage Status

Coverage increased (+0.05%) to 97.794% when pulling 89d8a43 on yograterol:master into 9eae37f on owais:master.

@yograterol yograterol changed the title [WIP] Fixed render_bundle tag when the stats file does not exists Fixed render_bundle tag when the stats file does not exists May 20, 2016
@owais
Copy link
Collaborator

owais commented May 21, 2016

render_bundle tag is supposed to fail load. That was the intention. Is there a scenario where you think it should ignore the error and load a "broken" site?

@yograterol
Copy link
Author

yograterol commented May 22, 2016

@owais yes, when we are running the tests without the bundles (for speed) or when some bundle was not generated. Why I need a 500 error if a bundle doesn't exist?

@owais
Copy link
Collaborator

owais commented May 23, 2016

Why I need a 500 error if a bundle doesn't exist?

So that you know your bundle doesn't exist and webpack build failed. Otherwise you've a good chance to run a production site with broken frontend code.

when we are running the tests without the bundles (for speed)

This is a very good usecase but I think the right way to handle it is to mock the template tag calls. I don't see why we cannot ship a mock utility function by default. Would you be willing to contribute that?

@yograterol
Copy link
Author

Right now we need to merge our PR of Webpack... For the next week, I will able to check about the template tag mock.

@pxg
Copy link

pxg commented Jul 25, 2016

I like the idea of the template tag mock to speed up tests.

Also I've just the forked the repo because I have a similar error where I get 500 error during deployments while the bundle generates. In my case the more desirable behaviour is to serve an older valid bundle rather than return a 500 error to the users.

What do you think of the idea of having a fallback_bundle setting for this use case? I'd be happy to create a PR for this if you like the idea.

@owais
Copy link
Collaborator

owais commented Jul 25, 2016

@pxg I've faced this issue as well but I'm not sure if fallback_bundle is the best thing we can do. An alternative is to properly cache the stats file for the lifetime of the python process so that the process loads up the stats file on first invocation and then keeps on using it for the rest of it's life. We have something like this implemented but it needs a re-write to make it more robust.

That said, this is a much easier problem to outside webpack_loader IMO. What I do in my projects is to have django read from webpack-stats-prod.json but have webpack generate webpack-stats.json. Once, webpack build finishes successfully, I replace webpack-stats-prod.json with the newly generated webpack-stats.json file. It works really well in practice.

If you are using a CI server to build you app, it gets even easier. You can first generate new bundles, upload them to something like S3 and then replace webpack-stats.json on your servers with the newly created webpack-stats.json generated on your CI env.

@pxg
Copy link

pxg commented Jul 26, 2016

@owais I like both your suggestions. I'll try the webpack-stats-prod.json solution out, this will mean I'll no longer need a fork of the repo.

I'd considered having CI server build the bundles, but want to do this as a larger piece of work creating artifacts including all Python dependencies using http://platter.pocoo.org/dev/ or Linux packages.

@owais owais closed this Nov 6, 2016
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.

4 participants