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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade django pipeline #4151

Merged
merged 4 commits into from May 25, 2016

Conversation

Projects
None yet
3 participants
@alexgibson
Member

alexgibson commented May 24, 2016

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 some commits May 23, 2016

Upgrade django-pipeline so we can disable collection
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.
Show outdated Hide outdated gulpfile.js
gulp.task('test', function(done) {
gulp.task('media:watch', function () {
return gulp.src('./media/**/*')
.pipe(watch('./media/**/*', {

This comment has been minimized.

@pmac

pmac May 24, 2016

Member

Does this auto-compile .less files?

@pmac

pmac May 24, 2016

Member

Does this auto-compile .less files?

This comment has been minimized.

@alexgibson

alexgibson May 24, 2016

Member

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

alexgibson May 24, 2016

Member

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

This comment has been minimized.

Show comment
Hide comment
@alexgibson

alexgibson May 25, 2016

Member

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?

Member

alexgibson 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?

@jgmize

This comment has been minimized.

Show comment
Hide comment
@jgmize

jgmize May 25, 2016

Member

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.

Member

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.

Show outdated Hide outdated gulpfile.js
gulp.task('serve:backend', function () {
process.env.PYTHONUNBUFFERED = 1;
process.env.PYTHONDONTWRITEBITECODE = 1;
spawn('python', ['manage.py', 'runserver'], {

This comment has been minimized.

@jgmize

jgmize May 25, 2016

Member

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

@jgmize

jgmize May 25, 2016

Member

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

This comment has been minimized.

@pmac

pmac May 25, 2016

Member

Ah yes! Good catch. I'll fix that.

@pmac

pmac May 25, 2016

Member

Ah yes! Good catch. I'll fix that.

This comment has been minimized.

@pmac

pmac May 25, 2016

Member

@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.

@pmac

pmac May 25, 2016

Member

@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.

This comment has been minimized.

@jgmize

jgmize May 25, 2016

Member

That's what I was thinking as well, thanks.

@jgmize

jgmize May 25, 2016

Member

That's what I was thinking as well, thanks.

@jgmize

This comment has been minimized.

Show comment
Hide comment
@jgmize

jgmize May 25, 2016

Member

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

Member

jgmize commented May 25, 2016

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

@pmac

This comment has been minimized.

Show comment
Hide comment
@pmac

pmac May 25, 2016

Member

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.

Member

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

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@jgmize jgmize deleted the upgrade-django-pipeline branch May 25, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment