build: Upgrading django-simple-history. It has some breaking changes.
#1787
Conversation
credentials/apps/core/models.py
Outdated
| @@ -201,6 +201,7 @@ class User(AbstractUser): | |||
|
|
|||
| full_name = models.CharField(_("Full Name"), max_length=255, blank=True, null=True) | |||
| lms_user_id = models.IntegerField(null=True, db_index=True) | |||
| first_name = models.CharField(_("first name"), max_length=30, blank=True) | |||
There was a problem hiding this comment.
In django32 they have increased the first_name character length. But credentials is using 30 characters max length. This line avoids the new migration.
There was a problem hiding this comment.
Otherwise make migrations command brings new migration with 150 characters and it will put load on db while making this change on prod.
There was a problem hiding this comment.
Is there another dependency that is limiting the first_name to 30 characters? Is that related to django_social? (I'm looking into an error with the length of first_name)
There was a problem hiding this comment.
We have added the migration to extend the length of first_name, so this should not be needed now.
|
Hi @awais786 -- unfortunately the PR has a few conflicts. Is this still actively being worked on? Would you be able to fix these? or can we close this PR? Thanks! |
|
@justinhynes I will fix these conflicts soon. |
|
FYI we have a bug that may or may not be related to this, and we are investigating that bug and then we will come back around to looking at this. |
hurtstotouchfire
requested review from
hurtstotouchfire
and removed request for
Tj-Tracy and
MaxFrank13
|
@awais786 Just checking in on this. There are still a number of conflicts, should we keep this PR open? |
I will look into this in coming week. |
awais786
force-pushed
the
upgrade-history
branch
from
c62011e
to
c037601
Compare
| @@ -559,3 +559,6 @@ | |||
| # Plugin Django Apps | |||
| INSTALLED_APPS.extend(get_plugin_apps(PROJECT_TYPE)) | |||
| add_plugins(__name__, PROJECT_TYPE, SettingsType.BASE) | |||
|
|
|||
| # disable indexing on history_date | |||
| SIMPLE_HISTORY_DATE_INDEX = False | |||
There was a problem hiding this comment.
Don't need this for this PR. Owning team can decide this in future.
build: Upgrading `django-simple-history`.
awais786
force-pushed
the
upgrade-history
branch
from
cfc5023
to
ce40458
Compare
|
@justinhynes sorry for late response and fixing this PR. This pr looks fine now. you can merge it. After this deployment we can remove the pin and upgrade to latest version to get django4 support. |
https://django-simple-history.readthedocs.io/en/latest/#section-1
Upgrading the package.
jazzband/django-simple-history#862 ( this pr shows the get_latest_by usage )