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

Merge token_blacklist migrations into one (#615) #781

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

Conversation

afoalb
Copy link

@afoalb afoalb commented Feb 8, 2024

  • Initial migrations defined id as AutoField, later migrations changed the type to BigAutoField. This conversion is not allowed in MSSQL.

afoalb and others added 2 commits February 8, 2024 20:06
- Initial migrations defined id as AutoField, later migrations changed the type to BigAutoField. This conversion is not allowed in MSSQL
@afoalb afoalb changed the title Merge token_blacklist migrations into one ([#615](https://github.com/jazzband/djangorestframework-simplejwt/issues/615)) Merge token_blacklist migrations into one (#615) Feb 8, 2024
@afoalb
Copy link
Author

afoalb commented Feb 8, 2024

I ran all tox tests locally and they all passed. Not clear why they are failling here on github

Copy link
Member

@50-Course 50-Course left a comment

Choose a reason for hiding this comment

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

By default, Postgres favors BigAutoField. Talking about database-specific implementations:

AutoField as 32-bit INT, and BigAutoField to its 64-bit BIGINT character, in both MySQL and MariaDB.

Notably, AutoField maps to INT (32-bit) or BIGINT (64-bit) depending on DEFAULT_AUTO_FIELD setting or explicit field declaration - which by default is AutoField in django unless otherwise specified.

That said, this is a good move, and should we get the tests up, would be merging this MR

@50-Course
Copy link
Member

On deeper considerations, let's review the backwards compatibility that might revolve around this change - potential impact on existing projects.

Thoughts?

@50-Course
Copy link
Member

Hi, any updaes?

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.

2 participants