Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Increase path's max_length for ImportedFile model to 4096 #5345

Merged
merged 2 commits into from Feb 26, 2019
Merged

Increase path's max_length for ImportedFile model to 4096 #5345

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a9bdb46
Increase path to 4096 character
dojutsu-user Feb 23, 2019
a0c77e8
add comment
dojutsu-user Feb 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 20 additions & 0 deletions readthedocs/projects/migrations/0040_increase_path_max_length.py
View file
Open in desktop
@@ -0,0 +1,20 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.20 on 2019-02-23 15:05
from __future__ import unicode_literals

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('projects', '0039_update-doctype-helptext'),
]

operations = [
migrations.AlterField(
model_name='importedfile',
name='path',
field=models.CharField(max_length=4096, verbose_name='Path'),
),
]
2 changes: 1 addition & 1 deletion readthedocs/projects/models.py
View file
Open in desktop
Expand Up @@ -1089,7 +1089,7 @@ class ImportedFile(models.Model):
)
name = models.CharField(_('Name'), max_length=255)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Believe we want to update the name as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should be the optimal value for max_length. I didn't find that in the discussion of #5061

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name only saves the filename not the full path with directories, so the limitation for filenames is only 255, so we are safe.

slug = models.SlugField(_('Slug'))
path = models.CharField(_('Path'), max_length=255)
path = models.CharField(_('Path'), max_length=4096)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add a comment why 4096 is our magic number here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.
I have added the comment and pushed the changes.

md5 = models.CharField(_('MD5 checksum'), max_length=255)
commit = models.CharField(_('Commit'), max_length=255)
modified_date = models.DateTimeField(_('Modified date'), auto_now=True)
Expand Down