Skip to content

Fix parsing of DISABLE_COLLECTSTATIC env var values#1208

Merged
edmorley merged 1 commit intomainfrom
fix-disable-collectstatic-handling
May 24, 2021
Merged

Fix parsing of DISABLE_COLLECTSTATIC env var values#1208
edmorley merged 1 commit intomainfrom
fix-disable-collectstatic-handling

Conversation

@edmorley
Copy link
Copy Markdown
Member

@edmorley edmorley commented May 21, 2021

By default if Django is installed and a manage.py file found, the buildpack will run Django's collectstatic command at the end of the build.

This step can be skipped by setting DISABLE_COLLECTSTATIC=1:
https://devcenter.heroku.com/articles/django-assets#disabling-collectstatic

However previously, if DISABLE_COLLECTSTATIC was instead set to 0 or the empty string (such as when toggling it on and off during debugging), then collectstatic would still be skipped.

This behaviour was made more confusing due to there being no build output that explained why collectstatic was being skipped.

Now:

  • Using values of 0 or the empty string are treated the same as if DISABLE_COLLECTSTATIC was not set at all (all other values are unaffected).
  • If collectstatic is skipped (either due to that env var, the legacy skip file, or because a manage.py file is not found), then the build output now contains the reason why.
  • A deprecation warning is output when collectstatic is skipped via the legacy .heroku/collectstatic_disabled file method.

I've also added some more tests around collectstatic, since there was only one before, and refactored the collectstatic step script to use early returns for improved readability.

See:
https://heroku.support/950668
https://heroku.support/986549

Closes GUS-W-8813947.

@edmorley edmorley added the bug label May 21, 2021
@edmorley edmorley self-assigned this May 21, 2021
@edmorley edmorley requested a review from a team as a code owner May 21, 2021 14:39
@edmorley edmorley force-pushed the fix-disable-collectstatic-handling branch from 05c1e5b to f751d5e Compare May 24, 2021 08:01
By default if Django is installed and a `manage.py` file found, the
buildpack will run Django's `collectstatic` command at the end of the
build.

This step can be skipped by setting `DISABLE_COLLECTSTATIC=1`:
https://devcenter.heroku.com/articles/django-assets#disabling-collectstatic

However previously, if `DISABLE_COLLECTSTATIC` was instead set to `0` or
the empty string (such as when toggling it on and off during debugging),
then collectstatic would still be skipped.

This behaviour was made more confusing due to there being no build output
that explained why collectstatic was being skipped.

Now:
* Using values of `0` or the empty string are treated the same as if
  `DISABLE_COLLECTSTATIC` was not set at all (all other values are
  unaffected).
* If `collectstatic` is skipped (either due to that env var, the legacy
  skip file, or because a `manage.py` file is not found), then the build
  output now contains the reason why.
* A deprecation warning is output when collectstatic is skipped via the
  legacy `.heroku/collectstatic_disabled` file method.

I've also added some more tests around collectstatic, since there was
only one before, and refactored the collectstatic step script to use
early returns for improved readability.

See:
https://heroku.support/950668
https://heroku.support/986549

Closes GUS-W-8813947.
@edmorley edmorley force-pushed the fix-disable-collectstatic-handling branch from f751d5e to 46625a3 Compare May 24, 2021 08:14
@edmorley edmorley merged commit a928a2f into main May 24, 2021
@edmorley edmorley deleted the fix-disable-collectstatic-handling branch May 24, 2021 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants