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.

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

fix migration for request.prof_file field #245

Merged
merged 1 commit into from
Dec 29, 2017

Conversation

dennybiasiolli
Copy link
Contributor

Fixes #244

@albertyw
Copy link
Member

Just to confirm, there were no other migration files generated?

@dennybiasiolli
Copy link
Contributor Author

@albertyw no, this was the only migration file generated by python manage.py makemigration

@albertyw albertyw requested review from albertyw and avelis December 28, 2017 16:27
@xrmx
Copy link
Contributor

xrmx commented Dec 28, 2017

@dennybiasiolli thank you sir!

@avelis
Copy link
Collaborator

avelis commented Dec 28, 2017

@dennybiasiolli Thanks for making the contribution. Before I approve, would you happen to know if the field will evaluate as an indexable field? I ask because MySQL has a limitation of 191 and we had issues in the past with indexable fields going past max_length of 191. I don't think that is the case here but I wanted to be double sure.

@dennybiasiolli
Copy link
Contributor Author

@avelis I don't think prof_file will evaluate as indexable field, because in models.py it's defined as

prof_file = FileField(max_length=300, blank=True, storage=silk_storage)

In Class Request the only indexed fields are the following: path, start_time and view_name.

start_time is a DateTimeField, whilst path and view_name have already a max_length=190, so I think there are no problems with MySQL in this case! 😉

@avelis avelis merged commit 5ba0853 into jazzband:master Dec 29, 2017
@dennybiasiolli dennybiasiolli deleted the fix-request-migration branch December 29, 2017 05:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants