Opened 7 years ago

Closed 7 years ago

#28163 closed Cleanup/optimization (wontfix)

Make the default settings.py pass pep8

Reported by: Yoni Lavi Owned by: nobody
Component: Core (Other) Version: 1.11
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

For most Django projects I see, everything passes pep8 checks except for migrations and the default settings.py file. Migrations aren't really source code, so I don't care about that, but it should be easy to make settings.py conform to all the rules.

From what I checked, by changing the AUTH_PASSWORD_VALIDATORS to fit in under 80 cols, the entire default file should now pass validation, so I suggest the following comprehension for the validators (Pull Request - https://github.com/django/django/pull/8452):

AUTH_PASSWORD_VALIDATORS = [
    {'NAME': 'django.contrib.auth.password_validation.%s' % validator}
    for validator in [
        'UserAttributeSimilarityValidator',
        'MinimumLengthValidator',
        'CommonPasswordValidator',
        'NumericPasswordValidator',
    ]
]

Change History (5)

comment:1 by Arne de Laat, 7 years ago

This change makes it a bit more difficult to add OPTIONS to one of the validators, as shown in the example: https://docs.djangoproject.com/en/dev/topics/auth/passwords/#enabling-password-validation
Or to add a validator not in django.contrib.auth.password_validation.

comment:2 by Tim Graham, 7 years ago

Component: UncategorizedCore (Other)

Agreed, I'm not so happy to obfuscate the settings merely to limit line length.

comment:3 by Claude Paroz, 7 years ago

Resolution: wontfix
Status: newclosed

The 80-chars limit of pep8 is controversial. Many projects customize it to a greater length. Readability and usability come first.

comment:4 by Yoni Lavi, 7 years ago

Resolution: wontfix
Status: closednew

Hi folks,
I'm sorry to rehash this, but the current validator verbosity keeps bugging me on my own projects and I'm sure I'm not the only one. So I just wanted to ask if you would be open to a more flexible approach such as using this sort of utility function (to be included in django.utils?):

def basic_password_validator(class_name, **kwargs):
    MODULE_PATH = 'django.contrib.auth.password_validation'
    validator = {'NAME': '.'.join([MODULE_PATH, class_name])}
    validator.update(kwargs)
    return validator


AUTH_PASSWORD_VALIDATORS = [
    basic_password_validator('UserAttributeSimilarityValidator'),
    basic_password_validator('MinimumLengthValidator',
                             OPTIONS={'min_length': 9}),
    basic_password_validator('CommonPasswordValidator'),
    basic_password_validator('NumericPasswordValidator'),
]

So I'm just looking for feedback about whether this sort of approach might be deemed acceptable, and if so whether you have advice about the specific implementation. If not, I promise to stop spamming this ticket :)

Last edited 7 years ago by Yoni Lavi (previous) (diff)

comment:5 by Tim Graham, 7 years ago

Resolution: wontfix
Status: newclosed

I don't see much difference between that suggestion and the original proposal. To reconsider a "wontfix" ticket, write to the DevelopersMailingList rather than reopening the ticket.

Note: See TracTickets for help on using tickets.
Back to Top