Opened 8 years ago
Closed 8 years ago
#28313 closed Cleanup/optimization (fixed)
Add a contenttypes check to prohibit model names greater than 100 characters
| Reported by: | Michal Dabski | Owned by: | |
|---|---|---|---|
| Component: | contrib.contenttypes | Version: | 1.11 |
| Severity: | Normal | Keywords: | contenttype, model, name, length, migration |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description (last modified by )
When creating a model with name longer than 100 characters, migrate will fail with the following trace:
Traceback (most recent call last):
File "C:/Users/Mick/Desktop/Projects/django/meg-forms/manage.py", line 14, in <module>
execute_from_command_line(sys.argv)
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\core\management\__init__.py", line 363, in execute_from_command_line
utility.execute()
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\core\management\__init__.py", line 355, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\core\management\base.py", line 283, in run_from_argv
self.execute(*args, **cmd_options)
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\core\management\base.py", line 330, in execute
output = self.handle(*args, **options)
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\core\management\commands\migrate.py", line 227, in handle
self.verbosity, self.interactive, connection.alias, apps=post_migrate_apps, plan=plan,
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\core\management\sql.py", line 53, in emit_post_migrate_signal
**kwargs
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\dispatch\dispatcher.py", line 193, in send
for receiver in self._live_receivers(sender)
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\contrib\auth\management\__init__.py", line 63, in create_permissions
ctype = ContentType.objects.db_manager(using).get_for_model(klass)
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\contrib\contenttypes\models.py", line 60, in get_for_model
model=opts.model_name,
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\db\models\manager.py", line 85, in manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\db\models\query.py", line 466, in get_or_create
return self._create_object_from_params(lookup, params)
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\db\models\query.py", line 498, in _create_object_from_params
obj = self.create(**params)
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\db\models\query.py", line 394, in create
obj.save(force_insert=True, using=self.db)
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\db\models\base.py", line 806, in save
force_update=force_update, update_fields=update_fields)
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\db\models\base.py", line 836, in save_base
updated = self._save_table(raw, cls, force_insert, force_update, using, update_fields)
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\db\models\base.py", line 922, in _save_table
result = self._do_insert(cls._base_manager, using, fields, update_pk, raw)
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\db\models\base.py", line 961, in _do_insert
using=using, raw=raw)
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\db\models\manager.py", line 85, in manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\db\models\query.py", line 1063, in _insert
return query.get_compiler(using=using).execute_sql(return_id)
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\db\models\sql\compiler.py", line 1099, in execute_sql
cursor.execute(sql, params)
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\db\backends\utils.py", line 81, in execute
return super(CursorDebugWrapper, self).execute(sql, params)
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\db\backends\utils.py", line 66, in execute
return self.cursor.execute(sql, params)
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\db\utils.py", line 94, in __exit__
six.reraise(dj_exc_type, dj_exc_value, traceback)
File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\db\backends\utils.py", line 66, in execute
return self.cursor.execute(sql, params)
django.db.utils.DataError: value too long for type character varying(100)
saving sqlmigrate to a file and executing it will work fine, which makes it harder to determine what the issue is - the schema migration does not have any inserts.
I found that the issue was caused by model name length not fitting into ContentType's 100 character limit.
I suggest any of the following
- Validate model names in ContentType model and raise a validation error so it's more clear to the user what is causing it
- Enforce model name length limit
- Truncate model name in ContentType objects down to 100 characters
Change History (14)
comment:1 by , 8 years ago
| Description: | modified (diff) |
|---|
comment:2 by , 8 years ago
| Version: | 1.8 → 1.11 |
|---|
comment:3 by , 8 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:4 by , 8 years ago
| Summary: | Model name length is not enforced or validated → Add a contenttypes check to prohibit model names greater than 100 characters |
|---|
comment:5 by , 8 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:6 by , 8 years ago
Merge request:
https://github.com/django/django/pull/8647
Is a test case needed?
Can this be backported to LTS?
comment:7 by , 8 years ago
| Has patch: | set |
|---|---|
| Needs tests: | set |
Yes, tests are always required unless it's infeasible.
This does not qualify for a backport based on our supported versions policy. I don't think model names longer than 100 characters are common.
comment:8 by , 8 years ago
I added tests and pushed to my branch, but the pull request did not update for me... should I open a new one or is this some temporary hiccup on github?
comment:9 by , 8 years ago
The merge request appears to have tests, except the test_model_name_too_long test seems to fail.
comment:10 by , 8 years ago
I noticed the PR got updated the next day. I'll fix the test. I didn't know it was failing since I couldn't get the test suite to run on my pc
comment:11 by , 8 years ago
Should be ready now; tests added, documentation updated. Any more feedback?
comment:12 by , 8 years ago
| Needs tests: | unset |
|---|
comment:13 by , 8 years ago
| Owner: | removed |
|---|---|
| Status: | assigned → new |
I guess we could add a
django.contrib.contenttypessystem check for that.