-
Notifications
You must be signed in to change notification settings - Fork 955
Upgrade django pipeline #4151
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
Upgrade django pipeline #4151
Conversation
The latest django-pipeline (1.6.8) adds a setting to disable collection in dev mode. This will allow us to handle our own file collection via node scripts and greatly speed page loads for local development, while still getting the production benefits that the full asset pipeline gets us.
| gulp.task('test', function(done) { | ||
| gulp.task('media:watch', function () { | ||
| return gulp.src('./media/**/*') | ||
| .pipe(watch('./media/**/*', { |
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.
Does this auto-compile .less files?
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.
Not currently. The .less files are still being compiled by django-pipeline. We've only disabled the asset collecting. We can do the less compilation here too, if you can find a way to turn it off? Either way, this is still a considerable time-saver as-is.
|
Just a thought - will this change break integration tests that run in the pipeline via the docker container? Not sure if we would need to make sure assets are copied over there as they will no longer be automatically? |
The integration tests are run against the bedrock_code image, which runs collectstatic as part of the image build process so I don't think that should be an issue. |
gulpfile.js
Outdated
| gulp.task('serve:backend', function () { | ||
| process.env.PYTHONUNBUFFERED = 1; | ||
| process.env.PYTHONDONTWRITEBITECODE = 1; | ||
| spawn('python', ['manage.py', 'runserver'], { |
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.
By default runserver binds to localhost, so this doesn't work in a docker container. I got this working by adding a 0.0.0.0:8000 to the args list, but ideally it that could be a default overridden by an environment variable
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.
Ah yes! Good catch. I'll fix that.
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.
@jgmize can you think of a reason to not have the default be "0.0.0.0:$PORT", with a default value for the PORT env var of 8000? That's what I'm leaning toward, but if we need to default to 127.0.0.1 then I can work that out as well.
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.
That's what I was thinking as well, thanks.
|
r+wc, nice work @alexgibson and @pmac! |
|
runserver command updated to use |
Description
/mediato/staticas and when they change (this task copies everything in/mediaover when first run, and incrementally from then on).This should hopefully give local bedrock development a nice speed boost 👍. Currently django-pipeline copies everything from
/mediato/staticon each page load in dev mode (which is well over 100mb of files). This PR stops that from happening, and Gulp now handles copying over/staticassets in a more efficient way. We could (and probably should) take this even further in the longer term, but this is hopefully a quick win.Bugzilla link
N/A
Testing
gulp.