Skip to content

Conversation

@alexgibson
Copy link
Contributor

Description

  • Upgrades django-pipeline so we can disable automatic collection of static assets in local dev mode.
  • Adds a new default gulp task, which copies assets from /media to /static as and when they change (this task copies everything in /media over 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 /media to /static on 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 /static assets 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

  • Once you pull the branch, update requirements for both bedrock and npm.
  • Start bedrock locally, then just run gulp.

pmac and others added 2 commits May 23, 2016 14:37
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/**/*', {
Copy link
Contributor

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?

Copy link
Contributor Author

@alexgibson alexgibson May 24, 2016

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.

@alexgibson
Copy link
Contributor Author

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?

@jgmize
Copy link
Contributor

jgmize commented May 25, 2016

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'], {
Copy link
Contributor

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

Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor

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.

@jgmize
Copy link
Contributor

jgmize commented May 25, 2016

r+wc, nice work @alexgibson and @pmac!

@pmac
Copy link
Contributor

pmac commented May 25, 2016

runserver command updated to use 0.0.0.0 and a configurable port, defaulting to 8000. I punted on the question of also running collectstatic. We can deal with that later when/if it becomes an issue.

@jgmize jgmize merged commit 8dfcf78 into master May 25, 2016
@jgmize jgmize deleted the upgrade-django-pipeline branch May 25, 2016 20:52
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