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

feat(StateLog): Migrated state log from integerfield to charfield #97

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

josh-stableprice
Copy link

@josh-stableprice josh-stableprice commented Jul 22, 2019

Here is a basic implementation of the migration from integer primary key to allow UUIDS, with a check to warn people who use the field that they may need to migrate.

The check can be acknowledged and ignored by adding 'django_fsm_log.W001' to SILENCED_SYSTEM_CHECKS as documented here https://docs.djangoproject.com/en/2.2/ref/settings/#std:setting-SILENCED_SYSTEM_CHECKS

I also suggest adding some kind of warning on the main page and bumping the version number significantly to stop it being pulled in automatically by Poetry and Pipenv.

Fixes #34

@josh-stableprice
Copy link
Author

Tests failed, however I'm unsure why

@tysonclugg
Copy link

Tests failed, however I'm unsure why

It's a flake8 (linter) error.

https://travis-ci.org/gizmag/django-fsm-log/jobs/562015145#L507

django_fsm_log/conf.py:1:1: F401 'django.conf.settings' imported but unused

@josh-stableprice
Copy link
Author

josh-stableprice commented Jul 23, 2019 via email

),
obj=field,
id='django_fsm_log.W001',
)

Choose a reason for hiding this comment

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

Is this warning absolutely necessary? I'm assuming it relates to https://code.djangoproject.com/ticket/25012 or similar?

Copy link
Author

Choose a reason for hiding this comment

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

Basically yeah, one of the only solid bits of contention in the issue where this was discussed was surrounding the use of the field by user based code which implicitly expected an integer.
I am also aware that I've been tripped up by types of foreign key references changing in the past in my own code and it's within the scope of the check framework to automate looking for these kinds of errors and warning users about them.
We can also nerf it again at some point in the futurewhen we feel we've had enough of a deprecation period. I just wish there was a way I could also check to see if any python code references these fields.

@diegoponciano
Copy link

Is there any plans to merge this PR? I use UUID as PKs on my models, and I can't see any workaround to use this project.

@MRigal MRigal linked an issue Jan 14, 2022 that may be closed by this pull request
@MRigal
Copy link
Member

MRigal commented Jan 14, 2022

@josh-stableprice the core of your PR is actually very valuable.

The project recently got a massive update. Would you mind rebasing or cherry-picking the core of your changes ? Also adding some failing tests for for example UUIDs as PK would be desirable. Thanks in advance!

@ddahan
Copy link

ddahan commented Sep 9, 2022

Hi guys. Any reason to not merge this PR? The project is still unusable to many people just because of this :/

@achillis2
Copy link

Can this PR be fixed and merged so the UUID issue can be solved please? Thanks!

@ticosax
Copy link
Member

ticosax commented Apr 5, 2023

Will be solved by #176
Please help there (review, test, give a feedback, ...)

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.

integer out of range when pk is uuid StateLog can't handle non-numeric object_id
8 participants