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

Fixed #34744 -- Prevented recreation of migration for constraints with a dict_keys. #17114

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

shangxiao
Copy link
Contributor

A possible fix for Ticket 34744 - Migration re-add constraints when check condition contains a dict_keys object.

I'm not super-familiar with how migration serialisation works. I had a look to see if there was a way to serialise an entire project state but couldn't see anything 🤔 If you can do this then it's probably better to serialise the state before passing to the Autodetector.

This alternative is to serialise constraints before checking their equality. This solution is too narrow though - perhaps this problem could appear again in other parts of the state? 🤷‍♂️

Maybe we should try to serialise everything manually.

@shangxiao shangxiao force-pushed the ticket_34744 branch 2 times, most recently from fe8c78b to ef592a9 Compare July 27, 2023 07:38
@felixxm
Copy link
Member

felixxm commented Aug 22, 2023

@shangxiao Do you have time to keep working on this?

@shangxiao
Copy link
Contributor Author

@felixxm Oops I missed the review from 2 weeks ago ❤️

Your approach sounds more reasonable. Looks like you'd already written up the patch and tested it? If you like I can work with what you posted though? 🤷‍♂️

@felixxm
Copy link
Member

felixxm commented Aug 22, 2023

If you like I can work with what you posted though?

Thanks, that would be appreciated.

@felixxm felixxm changed the title Fixed #34744 -- Serialize new constraints during migration change detection Fixed #34744 -- Prevented recreation of migration for constraints with a dict_keys. Aug 23, 2023
Copy link
Member

@felixxm felixxm left a comment

Choose a reason for hiding this comment

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

@shangxiao Thanks 👍 I moved tests to the tests/migrations/test_autodetector.py.

@felixxm felixxm marked this pull request as ready for review August 23, 2023 08:43
@felixxm felixxm merged commit 76c3e31 into django:main Aug 23, 2023
@shangxiao shangxiao deleted the ticket_34744 branch August 24, 2023 07:18
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