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

1633: Update Django dependency to version 5.1.5 #1792

Merged
merged 4 commits into from
Jan 22, 2025

Conversation

ulgens
Copy link
Contributor

@ulgens ulgens commented Dec 4, 2024

@bmispelon bmispelon changed the title 1633: Update Django dependency to version 5.1.3 1633: Update Django dependency to version 5.1.4 Dec 5, 2024
@ulgens ulgens changed the title 1633: Update Django dependency to version 5.1.4 1633: Update Django dependency to version 5.1.5 Jan 21, 2025
@ulgens
Copy link
Contributor Author

ulgens commented Jan 21, 2025

@bmispelon The failure in the test looks like a flake but I don't have permission to re-run the pipeline. I'm checking this again.

@ulgens ulgens marked this pull request as ready for review January 21, 2025 18:51
@bmispelon
Copy link
Member

The test failure looks like it might have something to do with tracdb.test_utils.TracDBCreateDatabaseMixin. It might be a fluke, but it might also be genuine (it could be that the mixin relied on an internal Django API that was modified).

You might be able to reproduce the failure locally by running all tests (not just a subset).

@ulgens
Copy link
Contributor Author

ulgens commented Jan 21, 2025

I'm trying to reproduce it locally but turned out creating the local env was harder than I remember, and docker setup is not ready for local testing. When I find a quick solution on this, I'll be creating several other issues 🙂

@ulgens
Copy link
Contributor Author

ulgens commented Jan 21, 2025

It looks like the errors are somehow related to composite PKs, but I thought it was merged after 5.1, and I couldn't find anything related on the 5.1 changelog. I'll try to tackle it another day, but if it's really about composite PKs, I guess it will require bigger changes than I thought.

Added another comment on https://github.com/django/djangoproject.com/pull/1479/files#r1924320453

@pauloxnet
Copy link
Member

I'm trying to reproduce it locally but turned out creating the local env was harder than I remember, ...

Please open an issue for the Readme if you found that something is missing or wrong.

Write here if you need help in the local setup.

@bmispelon
Copy link
Member

Using git bisect on the Django repo, I found that our tests started breaking after this change: django/django@e65deb7

This points further to something being broken with the TracDBCreateDatabaseMixin hack. I'll look further into it. 🔬

@bmispelon
Copy link
Member

Don't ask me how it works, but I just pushed a commit that fixes the tests 😅

@ulgens
Copy link
Contributor Author

ulgens commented Jan 22, 2025

I feel like we pushed the bug into the 5.2 update 😬 but I'm okay with passing tests for now ✨

@bmispelon
Copy link
Member

I feel like we pushed the bug into the 5.2 update 😬 but I'm okay with passing tests for now ✨

I tried and the tracdb tests still pass with 5.2 so I think we should be good on that front. And once we have 5.2 we should be able to get rid of all the hacks and use real composite primary keys I think 🎉

I think all is good with this PR now, I'm going to squash all the commits then merge, let's see how the deploy goes 🤞🏻

@bmispelon bmispelon merged commit 667bf22 into django:main Jan 22, 2025
4 checks passed
@ulgens ulgens deleted the 1633-django5.1 branch January 22, 2025 18:00
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.

Update Django to 5.1
3 participants