collectstatic - override default ignore list

[Tomas Ehrlich]

Hi,
ticket #20189 proposes to override default ignore patterns for
collectstatic management command in project's settings.

Patch has been submitted about a year ago, but right now it's waiting
for 'design decision' whether add a new setting or not.

What is your opinion?

Cheers,
Tom

https://code.djangoproject.com/ticket/20189
https://github.com/django/django/pull/2854

[Florian Apolloner]

I see a point in ignoring hidden and swap files by default. CSV is imo something which shouldn't be used anymore and as such shouldn't be a default. Given that ignoring hidden and swap files should cover 80% of the use cases, I don't see much point in a new setting.

Cheers,
Florian

[Tim Graham]

Jannis commented on the ticket, "This is simple, don't add a global setting but a parameter to the staticfiles app config. That's where such things belong now."

This makes sense to me. I suppose we should look into whether it makes sense to move at least some of the other static files settings there as well. Aymeric, is there any particular process that you envisioned for apps to add arbitrary config options like this (besides the obvious way of class attributes; and sorry if I've missed any existing docs about this).

[Aymeric Augustin]

I've carefully deferred this topic until now...

Maybe the answer is just a "get_setting" function analogous to "get_model" :

apps.get_setting('myapp', 'MYSETTING')

app_config.get_setting('MYSETTING')

It could simply defer to class level variables by default.

The first only works once the app registry is ready, while you may need access to settings at import time. Technically we could make it work as soon as the app import phase is done. Then I t would be available during the module import phase. But I don't like basing an API on implementation constraints.

The second one suffers from the same issue because one will obtain the app_config through get_app_config which requires the app registry to be ready.

-- 

Aymeric.

--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/44fbb093-1337-45e9-9445-3a8340454d12%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Tomas Ehrlich]

Hi,
thank you for review.

I'm just a bit confused with final decision.

As Jannis commented on ticket: "This is simple, don't add a global setting

but a parameter to the staticfiles app config. That's where such things
belong now."

But according to Aymeric, app config isn't ready for custom settings yet --
"Maybe the answer is just a 'get_setting' function analogous to 'get_model'")


So the final conlusion is: New settings won't be added anywhere until this feature
will be implemented in AppConfig, right?


Thanks in advance

Cheers,
Tom


Dne Wed, 2 Jul 2014 08:17:52 +0200
Aymeric Augustin <aymeric.au...@polytechnique.org> napsal(a):

[Aymeric Augustin]

2014-07-02 17:21 GMT+02:00 Tomas Ehrlich <tomas....@gmail.com>:

I'm just a bit confused with final decision.

I don't think we have a decision yet ;-) 

 

As Jannis commented on ticket: "This is simple, don't add a global setting

but a parameter to the staticfiles app config. That's where such things
belong now."

That comment must have been written while I was working on app-loading and its final scope wasn't known yet.

I think a global setting isn't very different from a setting stored on the app config. The app config provides a bit of namespacing, but a prefix such as STATICFILES_ works just as well. 

At this time, we don't handle settings for contrib apps in their app config classes. So if we want to handle this with a setting, it should be a global setting. If we don't, my explanations are irrelevant.

--
Aymeric.

[Jannis Leidel]

Ugh, sorry for having created this confusion, I made the comment yesterday during my commute hence the brevity.

What I was thinking was indeed using a class attribute for such a configuration value because it could be overwritten on a per project basis if needed. I’m aware of that we have willfully punted on that last winter while discussing the app-loading refactor.

I was think something along the lines of this:

# django/contrib/staticfiles/apps.py
from django.apps import AppConfig

from django.utils.translation import ugettext_lazy as _


class StaticFilesConfig(AppConfig):
name = 'django.contrib.staticfiles'
verbose_name = _("Static Files")
ignore_patterns = ['CVS', '.*', '*~'] # <-- new line

# acme/apps.py
from django.contrib.staticfiles.apps import StaticFilesConfig

class AcmeStaticFilesConfig(StaticFilesConfig):
ignore_patterns = (StaticFilesConfig.ignore_patterns +
[‘vendor/*', ‘endless_pit/*'])

# acme/settings.py

INSTALLED_APPS = [
'acme.apps.AcmeStaticFilesConfig',
]

# django/contrib/staticfiles/management/commands/collectstatic.py
from django.apps import apps

class Command(BaseCommand):
# ...
def set_options(self, **options):
# ...
staticfiles_config = apps.get_app_config('staticfiles')
self.ignore_patterns = staticfiles_config.ignore_patterns <-- new line

I hope I’m not missing something. I guess this would only work once the app registry is populated, but isn’t that the case when the management command is called anyway?

Jannis

[Tomas Ehrlich]

Dne Wed, 2 Jul 2014 21:17:52 +0200
Aymeric Augustin <aymeric....@polytechnique.org> napsal(a):

> 2014-07-02 17:21 GMT+02:00 Tomas Ehrlich <tomas....@gmail.com>:
>
> > I'm just a bit confused with final decision.
> >
>
> I don't think we have a decision yet ;-)

Tim Graham made comment on PR:
"Closing for now as the consensus seems to be adding a setting on the
AppConfig is the correct approach."

>
>
> > As Jannis commented on ticket: "This is simple, don't add a global setting
> > but a parameter to the staticfiles app config. That's where such things
> > belong now."
> >
>
> That comment must have been written while I was working on app-loading and
> its final scope wasn't known yet.
>
> I think a global setting isn't very different from a setting stored on the
> app config. The app config provides a bit of namespacing, but a prefix such
> as STATICFILES_ works just as well.
>
> At this time, we don't handle settings for contrib apps in their app config
> classes. So if we want to handle this with a setting, it should be a global
> setting. If we don't, my explanations are irrelevant.
>

I would really like to see app configs with app-specific settings.
Personally, I don't like flat settings where namespaces are created
with prefixes (eg. STATICFILES_ ).

The idea of overriding class attributes in AppConfig seems promising.


Cheers,
Tom

[Andre Terra]

On Wed, Jul 2, 2014 at 5:14 PM, Tomas Ehrlich <tomas....@gmail.com> wrote:

I would really like to see app configs with app-specific settings.
Personally, I don't like flat settings where namespaces are created
with prefixes (eg. STATICFILES_ ).

The idea of overriding class attributes in AppConfig seems promising.

I wholeheartedly agree. Flat settings in faux namespaces seems very unpythonic to me. Even though it should prove slightly more cumbersome to have to edit settings in many different files during development, having the correct place for each setting makes for a much more organized structure.

Forgive me if this question sounds silly, but how would settings for, say, staticfiles be accessed from third-party apps? Would we still use from django.conf import settings? If not, then how would we know to look for 'acme.apps.AcmeStaticFilesConfig' or 'staticfiles'?

Also, shouldn't something like the ignore_patterns list be included in a wrapper settings attribute of StaticFilesConfig? This way we could easily access settings through apps.get_app_config('some_app').settings['ignore_patterns'].

Based on Thomas' proposal:

class Command(BaseCommand):
    # ...
    def set_options(self, **options):
    # ...
    staticfiles_config = apps.get_app_config('staticfiles')

-   self.ignore_patterns = staticfiles_config.ignore_patterns

+   self.ignore_patterns = staticfiles_config.settings['ignore_patterns']

Sorry if this is completely wrong! I'm a long time lurker and hobbyist, but not a developer by occupation.

Cheers,

AT

[Curtis Maloney]

OT...just writing down this which came to me on the train this morning as I read this thread

Does anyone else foresee the practice of having an "appconfig.py" full of subclasses of AppConfigs becoming common?

Seems like a nice parallel to settings, allowing a project to specify per-app config overrides in a central location.  And lets us dispense with per-app settings with faux namespaces...

--

C

--

You received this message because you are subscribed to the Google Groups "Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAKBiv3ygq-s7E2UkySW7ZEXEKZe5q1jOz9bhzy32ZCfc8316DQ%40mail.gmail.com.

[Aymeric Augustin]

On 2 juil. 2014, at 22:14, Tomas Ehrlich <tomas....@gmail.com> wrote:

> The idea of overriding class attributes in AppConfig seems promising.

My concern — and the reason why I started talking about an API — is,
if every app clutters the AppConfig namespace with arbitrary names,
improvements to AppConfig may create name clashes.

--
Aymeric.

[Andre Terra]

On Thu, Jul 3, 2014 at 3:12 AM, Aymeric Augustin <aymeric....@polytechnique.org> wrote:

My concern -- and the reason why I started talking about an API -- is,

if every app clutters the AppConfig namespace with arbitrary names,
improvements to AppConfig may create name clashes.

Hence my suggestion of encapsulating all those different "settings" in AppConfig.settings.

Cheers,

AT

[Tim Graham]

Claude proposed a PR which simply adds the setting as an AppConfig attribute: https://github.com/django/django/pull/6460

Feedback would be welcome as to whether this seems sufficient or if we need something more complex. I imagine this pattern will become fairly common if we decide to move in this direction.