skoczen / django-longer-username

An app to easily provide a longer username field for django.
Other
63 stars 43 forks source link

Application doesn't work with django 1.3 #1

Closed marltu closed 12 years ago

marltu commented 13 years ago

class_prepared is called on User class before signal that changes length is registered.

This happens because of bug in django that causes django to load User before loading apps that are configured in settings. This is fixed with changeset https://code.djangoproject.com/changeset/16541 i think.

Hopefully it'll be fixed with 1.3.1

skoczen commented 13 years ago

Hm, I'm using it successfully with 1.3, what errors are you seeing?

marltu commented 13 years ago

Here's what I get when installing with django 1.3, longerusername app on top of all apps.

mariuskde@laptop:/tmp/django-starter$ bin/django syncdb --all
Syncing...
Creating tables ...
Creating table auth_permission
Creating table auth_group_permissions
Creating table auth_group
Creating table auth_user_user_permissions
Creating table auth_user_groups
Creating table auth_user
Creating table auth_message
Creating table django_content_type
Creating table django_session
Creating table django_site
Creating table django_admin_log
Creating table south_migrationhistory

You just installed Django's auth system, which means you don't have any superusers defined.
Would you like to create one now? (yes/no): no
Installing custom SQL ...
Installing indexes ...
Installed 2 object(s) from 1 fixture(s)

Synced:
 > longerusername
 > django.contrib.auth
 > django.contrib.contenttypes
 > django.contrib.sessions
 > django.contrib.sites
 > django.contrib.sitemaps
 > django.contrib.messages
 > django.contrib.staticfiles
 > django.contrib.admin
 > south
 > debug_toolbar
 > django_extensions
 > test_utils

Not synced (use migrations):
 - 
(use ./manage.py migrate to migrate these)
mariuskde@laptop:/tmp/django-starter$ bin/django dbshell
SQLite version 3.7.4
Enter ".help" for instructions
Enter SQL statements terminated with a ";"
sqlite> .schema auth_user
CREATE TABLE "auth_user" (
    "id" integer NOT NULL PRIMARY KEY,
    "username" varchar(30) NOT NULL UNIQUE,
    "first_name" varchar(30) NOT NULL,
    "last_name" varchar(30) NOT NULL,
    "email" varchar(75) NOT NULL,
    "password" varchar(128) NOT NULL,
    "is_staff" bool NOT NULL,
    "is_active" bool NOT NULL,
    "is_superuser" bool NOT NULL,
    "last_login" datetime NOT NULL,
    "date_joined" datetime NOT NULL
);
skoczen commented 13 years ago

Did you run ./manage.py migrate ?

Didn't see it in the traceback above.

marltu commented 13 years ago

Yeah migrate would change database table but model is still not affected and I think validations would fail.

Here's correct traceback with django 1.2.5, which changes length of username when running sync db.


mariuskde@laptop:/tmp/django-starter$ rm var/development.db 
mariuskde@laptop:/tmp/django-starter$ bin/django syncdb --all
Syncing...
Creating table auth_permission
Creating table auth_group_permissions
Creating table auth_group
Creating table auth_user_user_permissions
Creating table auth_user_groups
Creating table auth_user
Creating table auth_message
Creating table django_content_type
Creating table django_session
Creating table django_site
Creating table django_admin_log
Creating table south_migrationhistory

You just installed Django's auth system, which means you don't have any superusers defined.
Would you like to create one now? (yes/no): no
Installing index for auth.Permission model
Installing index for auth.Group_permissions model
Installing index for auth.User_user_permissions model
Installing index for auth.User_groups model
Installing index for auth.Message model
Installing index for admin.LogEntry model
Installing json fixture 'initial_data' from absolute path.
Installed 2 object(s) from 1 fixture(s)

Synced:
 > longerusername
 > django.contrib.auth
 > django.contrib.contenttypes
 > django.contrib.sessions
 > django.contrib.sites
 > django.contrib.sitemaps
 > django.contrib.messages
 > django.contrib.admin
 > south
 > debug_toolbar
 > django_extensions
 > test_utils

Not synced (use migrations):
 - 
(use ./manage.py migrate to migrate these)
mariuskde@laptop:/tmp/django-starter$ bin/django dbshell
SQLite version 3.7.4
Enter ".help" for instructions
Enter SQL statements terminated with a ";"
sqlite> .schema auth_user
CREATE TABLE "auth_user" (
    "id" integer NOT NULL PRIMARY KEY,
    "username" varchar(255) NOT NULL UNIQUE,
    "first_name" varchar(30) NOT NULL,
    "last_name" varchar(30) NOT NULL,
    "email" varchar(75) NOT NULL,
    "password" varchar(128) NOT NULL,
    "is_staff" bool NOT NULL,
    "is_active" bool NOT NULL,
    "is_superuser" bool NOT NULL,
    "last_login" datetime NOT NULL,
    "date_joined" datetime NOT NULL
);
skoczen commented 13 years ago

This works 100% correctly for me, on 1.3.0. If you can provide a test case where it fails, I'm happy to take a look.

$ mkvirtualenv test
New python executable in test/bin/python
Installing setuptools............done.
Installing pip...............done.
$ pip install django south -e git://github.com/GoodCloud/django-longer-username.git#egg=longerusername
...
Successfully installed django south longerusername
Cleaning up...
$ django-admin.py startproject test_proj

Settings.py

INSTALLED_APPS = (
    'longerusername',
    'south',
    'django.contrib.auth',
    'django.contrib.contenttypes',
    'django.contrib.sessions',
    'django.contrib.sites',
    'django.contrib.messages',
    'django.contrib.staticfiles',
)
$ python manage.py syncdb
Syncing...
Creating tables ...
Creating table south_migrationhistory
Creating table auth_permission
Creating table auth_group_permissions
Creating table auth_group
Creating table auth_user_user_permissions
Creating table auth_user_groups
Creating table auth_user
Creating table auth_message
Creating table django_content_type
Creating table django_session
Creating table django_site

You just installed Django's auth system, which means you don't have any superusers defined.
Would you like to create one now? (yes/no): no
Installing custom SQL ...
Installing indexes ...
No fixtures found.

Synced:
 > south
 > django.contrib.auth
 > django.contrib.contenttypes
 > django.contrib.sessions
 > django.contrib.sites
 > django.contrib.messages
 > django.contrib.staticfiles

Not synced (use migrations):
 - longerusername
(use ./manage.py migrate to migrate these)

$ python manage.py migrate
Running migrations for longerusername:
 - Migrating forwards to 0001_initial.
 > longerusername:0001_initial
 - Loading initial data for longerusername.
No fixtures found.

$ python manage.py shell
>>> from django.contrib.auth.models import User
>>> u = User.objects.create_user("A very long username that is far longer than 30 characters", "test@example.com", "testpassword")
>>> u
<User: A very long username that is far longer than 30 characters>
>>> u.save()
>>> u.username
'A very long username that is far longer than 30 characters'
>>> len(u.username)
58
>>> import django
>>> django.VERSION
(1, 3, 0, 'final', 0)
>>> ^D

$ python ./manage.py dbshell
sqlite> select username from auth_user;
A very long username that is far longer than 30 characters

sqlite> .schema auth_user
CREATE TABLE "auth_user" (
     "username" varchar(255) NOT NULL, 
     "first_name" varchar(30), 
     "last_name" varchar(30), 
     "is_active" bool, 
     "email" varchar(75), 
     "is_superuser" bool, 
     "is_staff" bool, 
     "last_login" datetime, 
     "password" varchar(128), 
     "id" integer PRIMARY KEY, 
     "date_joined" datetime
);
marltu commented 13 years ago

Screenshot of django admin:

http://i.imgur.com/uOz3P.png

I added code in longerusername to print sender of signal class_prepared.

When I run runserver I get following classes there:

Validating models...

<class 'south.models.MigrationHistory'>
<class 'django.contrib.sessions.models.Session'>
<class 'django.contrib.admin.models.LogEntry'>
0 errors found
Django version 1.3, using settings 'project.development'
Development server is running at http://127.0.0.1:8000/
Quit the server with CONTROL-C.

That's it

marltu commented 13 years ago

Actual bug appears when adding django.contrib.admin to INSTALLED_APPS

marltu commented 13 years ago

settings.py:

# Django settings for test_proj project.

DEBUG = True
TEMPLATE_DEBUG = DEBUG

ADMINS = (
    # ('Your Name', 'your_email@example.com'),
)

MANAGERS = ADMINS

DATABASES = {
    'default': {
        'ENGINE': 'django.db.backends.sqlite3', # Add 'postgresql_psycopg2', 'postgresql', 'mysql', 'sqlite3' or 'oracle'.
        'NAME': 'test.db',                      # Or path to database file if using sqlite3.
        'USER': '',                      # Not used with sqlite3.
        'PASSWORD': '',                  # Not used with sqlite3.
        'HOST': '',                      # Set to empty string for localhost. Not used with sqlite3.
        'PORT': '',                      # Set to empty string for default. Not used with sqlite3.
    }
}

# Local time zone for this installation. Choices can be found here:
# http://en.wikipedia.org/wiki/List_of_tz_zones_by_name
# although not all choices may be available on all operating systems.
# On Unix systems, a value of None will cause Django to use the same
# timezone as the operating system.
# If running in a Windows environment this must be set to the same as your
# system time zone.
TIME_ZONE = 'America/Chicago'

# Language code for this installation. All choices can be found here:
# http://www.i18nguy.com/unicode/language-identifiers.html
LANGUAGE_CODE = 'en-us'

SITE_ID = 1

# If you set this to False, Django will make some optimizations so as not
# to load the internationalization machinery.
USE_I18N = True

# If you set this to False, Django will not format dates, numbers and
# calendars according to the current locale
USE_L10N = True

# Absolute filesystem path to the directory that will hold user-uploaded files.
# Example: "/home/media/media.lawrence.com/media/"
MEDIA_ROOT = ''

# URL that handles the media served from MEDIA_ROOT. Make sure to use a
# trailing slash.
# Examples: "http://media.lawrence.com/media/", "http://example.com/media/"
MEDIA_URL = ''

# Absolute path to the directory static files should be collected to.
# Don't put anything in this directory yourself; store your static files
# in apps' "static/" subdirectories and in STATICFILES_DIRS.
# Example: "/home/media/media.lawrence.com/static/"
STATIC_ROOT = ''

# URL prefix for static files.
# Example: "http://media.lawrence.com/static/"
STATIC_URL = '/static/'

# URL prefix for admin static files -- CSS, JavaScript and images.
# Make sure to use a trailing slash.
# Examples: "http://foo.com/static/admin/", "/static/admin/".
ADMIN_MEDIA_PREFIX = '/static/admin/'

# Additional locations of static files
STATICFILES_DIRS = (
    # Put strings here, like "/home/html/static" or "C:/www/django/static".
    # Always use forward slashes, even on Windows.
    # Don't forget to use absolute paths, not relative paths.
)

# List of finder classes that know how to find static files in
# various locations.
STATICFILES_FINDERS = (
    'django.contrib.staticfiles.finders.FileSystemFinder',
    'django.contrib.staticfiles.finders.AppDirectoriesFinder',
#    'django.contrib.staticfiles.finders.DefaultStorageFinder',
)

# Make this unique, and don't share it with anybody.
SECRET_KEY = '4tnih%d@a@+1nz-3=ljnrmszp26bjl#t5r=8eh$$pbk-__vins'

# List of callables that know how to import templates from various sources.
TEMPLATE_LOADERS = (
    'django.template.loaders.filesystem.Loader',
    'django.template.loaders.app_directories.Loader',
#     'django.template.loaders.eggs.Loader',
)

MIDDLEWARE_CLASSES = (
    'django.middleware.common.CommonMiddleware',
    'django.contrib.sessions.middleware.SessionMiddleware',
    'django.middleware.csrf.CsrfViewMiddleware',
    'django.contrib.auth.middleware.AuthenticationMiddleware',
    'django.contrib.messages.middleware.MessageMiddleware',
)

ROOT_URLCONF = 'test_proj.urls'

TEMPLATE_DIRS = (
    # Put strings here, like "/home/html/django_templates" or "C:/www/django/templates".
    # Always use forward slashes, even on Windows.
    # Don't forget to use absolute paths, not relative paths.
)

INSTALLED_APPS = (
    'longerusername',
    'south',
    'django.contrib.auth',
    'django.contrib.contenttypes',
    'django.contrib.sessions',
    'django.contrib.sites',
    'django.contrib.messages',
    'django.contrib.staticfiles',
    # Uncomment the next line to enable the admin:
    'django.contrib.admin',
    # Uncomment the next line to enable admin documentation:
    # 'django.contrib.admindocs',
)

# A sample logging configuration. The only tangible logging
# performed by this configuration is to send an email to
# the site admins on every HTTP 500 error.
# See http://docs.djangoproject.com/en/dev/topics/logging for
# more details on how to customize your logging configuration.
LOGGING = {
    'version': 1,
    'disable_existing_loggers': False,
    'handlers': {
        'mail_admins': {
            'level': 'ERROR',
            'class': 'django.utils.log.AdminEmailHandler'
        }
    },
    'loggers': {
        'django.request': {
            'handlers': ['mail_admins'],
            'level': 'ERROR',
            'propagate': True,
        },
    }
}
(test)mariuskde@laptop:/tmp/test_proj$ python manage.py shell
Python 2.7.1+ (r271:86832, Apr 11 2011, 18:13:53) 
Type "copyright", "credits" or "license" for more information.

IPython 0.10.1 -- An enhanced Interactive Python.
?         -> Introduction and overview of IPython's features.
%quickref -> Quick reference.
help      -> Python's own help system.
object?   -> Details about 'object'. ?object also works, ?? prints more.

In [1]: from django.contrib.auth.models import User

In [2]: User._meta.get_field('username').max_length
Out[2]: 30
skoczen commented 13 years ago

Yeah, that I can replicate. I've got a new version that passes your last test ( User._meta.get_field('username').max_length ), but admin still lists it as 30. I really hate django admin's magic sometimes!

Will keep you posted.

marltu commented 13 years ago

As I've said before that's because of bug in django validation code.

If you apply this patch to django 1.3.0 it works fine: https://code.djangoproject.com/changeset/16541

Relative ticket: https://code.djangoproject.com/ticket/16283

Already fixed in 1.3.X branch.

skoczen commented 13 years ago

Yes, I understand that, and have since you first posted the link to it.

My goal for this project is to have it "just work", regardless of the version. Thus my attempt to get it fixed. If you have an idea on how to fix it without patching django, your pull request is welcome. Otherwise, I'll keep working on it, and either update the code or the description to be most accurate.

Thank you.

cardinal27513 commented 13 years ago

I just installed Django 1.31, seems that if admin app is installed, can’t see the class_prepared fired for “User in django.contrib.auth.models”. My app requires Admin any thought??? Here is what I did to confirm that…. Added logging statement in def longer_username(sender, _args, *_kwargs): logger.info ("%s--%s" % (sender.name, sender.module))

A) admin app is NOT installed manage.py runserver INFO MigrationHistory--south.models INFO ContentType--django.contrib.contenttypes.models INFO Permission--django.contrib.auth.models INFO Group_permissions--django.contrib.auth.models INFO Group--django.contrib.auth.models INFO User_user_permissions--django.contrib.auth.models INFO User_groups--django.contrib.auth.models INFO User--django.contrib.auth.models <<<<------------This signal is fired INFO Message--django.contrib.auth.models INFO Session--django.contrib.sessions.models INFO Site--django.contrib.sites.models

B) admin app installed manage.py runserver INFO MigrationHistory--south.models INFO Session--django.contrib.sessions.models INFO LogEntry--django.contrib.admin.models

skoczen commented 13 years ago

Hey cardinal27513,

I haven't yet found a way to make the admin play nicely. Supposedly, there was a fix on trunk, but it doesn't look like that's in 1.3.1. (Nor would it help the folks left on 1.3.0).

Ideas welcome :)

cardinal27513 commented 12 years ago

I couldn't figure it out either. My guess is that the signal may be fired but it may be too late by the time it is registered the with signal. At this moment, I added code to init.py to just update the DB so that I can use that in the app and not worry about the Admin site.

from django.utils.translation import ugettext as _ from django.db.models.signals import class_prepared from django.db import models

MAX_USERNAME_LENGTH = 32

def customizeUserModel(sender, _args, *_kwargs): if sender.name == "User" and sender.module == "django.contrib.auth.models": sender._meta.get_field("username").max_length = MAX_USERNAME_LENGTH sender._meta.get_field("username").helptext = ("Required, %s characters or fewer. Only letters, numbers, and @, ., +, -, or _ characters." % MAX_USERNAME_LENGTH )

class_prepared.connect(customizeUserModel)

On Mon, Sep 12, 2011 at 3:07 PM, Steven Skoczen < reply@reply.github.com>wrote:

Hey cardinal27513,

I haven't yet found a way to make the admin play nicely. Supposedly, there was a fix on trunk, but it doesn't look like that's in 1.3.1. (Nor would it help the folks left on 1.3.0).

Ideas welcome :)

Reply to this email directly or view it on GitHub:

https://github.com/GoodCloud/django-longer-username/issues/1#issuecomment-2074138

marltu commented 12 years ago

Looks like solution mentioned here https://gist.github.com/1772793 seems to work with 1.3 + admin installed.

I will try to integrate it into django-longer-username.

skoczen commented 12 years ago

Merged in, closing this up!