Ignoring Django collectstatic errors. about s2i-python-container HOT 6 CLOSED

Now I found it:

• https://devcenter.heroku.com/changelog-items/824

from s2i-python-container.

@GrahamDumpleton thanks for bringing this up!

I've done a quick review on our assemble scripts, unless I'm misreading it, we will also fail builds if the collectstatic step fails.

1. We have set -e: assemble#L11
2. We run python $manage_file collectstatic --noinput: assemble#L46
3. The DISABLE_COLLECTSTATIC flag is documented in using_images/s2i_images/python.html#configuration.

Is there anything else we're missing?

from s2i-python-container.

@GrahamDumpleton bump

from s2i-python-container.

If you want to look at wider issues with this there are a couple.

1 - It is assumed that if manage.py exists that it is for Django. This need not be the case as Flask-Script also makes use of a manage.py file for making it easier to start up a Flask application, along with support for management commands.

Your solution to this is to attempt a dry run of the command:

    if ! python $manage_file collectstatic --dry-run --noinput &> /dev/null; then
      echo "WARNING: could not run 'manage.py collectstatic'. To debug, run:"
      echo "    $ python $manage_file collectstatic --noinput"
      echo "Ignore this warning if you're not serving static files with Django."
      exit
    fi

The problem with this is that if there was a mis-configuration in Django settings, you would mask it at this point and allow the build to continue with collectstatic skipped. An actual problem with the configuration would only later manifest when the deployment occurred and the web application was started.

It would be much better to detect it during the build phase as then you wouldn't affect the already running web application if using Recreate strategy and the old web application was pulled down first.

The better approach would be to try and validate that the manage.py script was in fact for Django.

I do this by using:

    if [ -f manage.py ]; then
        if grep -q DJANGO_SETTINGS_MODULE manage.py; then
            echo " -----> Collecting static files for Django"
            django_collectstatic
        fi
    fi

So I don't use a dry run and instead if DJANGO_SETTINGS_MODULE appears in the manage.py file assume that should be good to assume it actually is the Django management script.

2 - Why even bother with running collectstatic in the first place.

Because you are using the Django development server, it doesn't need to have collectstatic actually run. This is because the way it handles serving of static files it pulls them in from the different locations of the different installed applications.

Even if you use gunicorn, you would use Whitenoise WSGI middleware, but that has Django integration and can do similar magic to the Django development server and sets up an internal map which references static files from their locations in installed applications and it doesn't need to have them copied into one location.

The only reason you would want to run collectstatic would be if running mod_wsgi-express or uWSGI, both of which is non trivial for normal user with current S2I builder.

Only other reason would be if someone didn't use the Whitenoise Django integration and relied on doing collectstatic and then pointing Whitenoise at that one directory.

So right now the value of running collectstatic is very limited. Since it isn't readily necessary is questionable why you would do it. It may have been better to implement a proper actions hooks mechanism and just leave the task up to the user if they really needed it.

from s2i-python-container.

As far as I understand this, case number one is no longer a problem because collectstatic in assemble script is run only if Django is installed so if you use manage.py for Flask configuration, you probably would not install Django in a container.

Case two can be solved by not running collectstatic command when Django development server will be used for serving Django app - in other words - run collectstatic only if you are going to use gunicorn as a web server.

Am I right? What do you think about this?

from s2i-python-container.

The check for django being installed is not full proof, but chance of it being installed at same time as flask is extremely slim. Better check may have been to look for DJANGO_SETTINGS_MODULE string in manage.py but the check for django module is sufficient for now. Closing.

from s2i-python-container.