-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
chore(deps): Upgrade to Django 4.2 #18653
Merged
Merged
Changes from all commits
Commits
Show all changes
52 commits
Select commit
Hold shift + click to select a range
78d795a
Upgrade dependencies
webjunkie 60c4351
Fix middleware error
webjunkie cb6b389
Upgrade psycopg
webjunkie f764d13
Go to Django 4.1 because of problems with psycopg3
webjunkie 197cdc8
Update query snapshots
github-actions[bot] 1513680
Update query snapshots
github-actions[bot] eabba3b
Update query snapshots
github-actions[bot] d0d6ee6
Update query snapshots
github-actions[bot] fd8212d
Switch TaggedItem tests to assert ValidationError
webjunkie bb8d7a5
Remove type: ignore comments
webjunkie faad0ca
Update query snapshots
github-actions[bot] c474c03
Figure out psycopg problem and try Django 4.2 again
webjunkie a1731eb
Update query snapshots
github-actions[bot] 15fe236
Fix other IN errors
webjunkie 9c9bcb4
Fix getting status
webjunkie 677d38a
Fix psycopg3 issues
webjunkie 9cc26d4
Fix psycopg issues
webjunkie 1dfef28
Update query snapshots
github-actions[bot] 3170b03
Update query snapshots
github-actions[bot] ccba439
Update query snapshots
github-actions[bot] 210ef2e
Merge branch 'master' into chore/Django-4
webjunkie 8eb8a77
Update query snapshots
github-actions[bot] 97f35db
Merge branch 'master' into chore/Django-4
webjunkie 8d44eea
Update deps
webjunkie 2dfa67c
Update query snapshots
github-actions[bot] 89e0728
Update query snapshots
github-actions[bot] 2b6b6d9
Update query snapshots
github-actions[bot] 63833af
Update query snapshots
github-actions[bot] e444658
Fix more tests
webjunkie 6fcc0c9
Merge branch 'master' into chore/Django-4
webjunkie a7ef5df
Adjust baseline
webjunkie d8d212b
Remove sqlcommenter (should be PostgresQL only anyways)
webjunkie d24e7f4
Fix file
webjunkie 62a6965
Update query snapshots
github-actions[bot] 48c8ac9
Update query snapshots
github-actions[bot] 94368c6
Update query snapshots
github-actions[bot] 180548e
Merge branch 'master' into chore/Django-4
webjunkie e9db3ac
Fix queries
webjunkie 8e95b93
Fix query
webjunkie 4fc7873
Revert
webjunkie 2724b91
Update requirements.in
webjunkie c2a7ef8
Remove restore-virtualenv
webjunkie d22a44e
Revert "Remove restore-virtualenv"
webjunkie 53b965b
Merge branch 'master' of github.com:PostHog/posthog into chore/Django-4
neilkakkar 8c0d9c4
mypy
neilkakkar 70e54d3
Adjust num queries
webjunkie b91b5e5
Adjust num queries
webjunkie e1b37a5
Adjust num queries
webjunkie f0d9fdc
Update query snapshots
github-actions[bot] 007d96b
Merge branch 'master' into chore/Django-4
webjunkie 3f2bf63
Add to updated_fields
webjunkie f0cfd29
Merge branch 'master' into chore/Django-4
webjunkie File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't remember where they're set but it looks like the useful SQL commenting has stopped working
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reasonably sure it's the sql commenter: https://github.com/PostHog/posthog/blob/master/posthog/settings/web.py#L90
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems Google gave this sqlcommenter to opentelemetry and it was not updated in a long time.
This might be where it ended up:
https://opentelemetry-python-contrib.readthedocs.io/en/latest/instrumentation/django/django.html#sqlcommenter
However, then there's this about supporting psycopg3, which I think might be the problem we have:
open-telemetry/opentelemetry-python-contrib#1751
How much do we need it anyways?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh dang it!
It's pretty useful for knowing where the request is coming from, specially when looking in, say, pganalyze for most popular sources of sql requests.
But, we do this manually for clickhouse ourselves, so wonder if there's an easy / small-scoped way to just do it ourselves
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've investigated further and also tried to use the middleware from opentelemetry that has some modifications as it seems.
Unfortunately, then immediately the following issue comes up 🥴
open-telemetry/opentelemetry-python-contrib#1554
It seems there are some differences between how the queries are handled and how the wrappers are executed. This would also explain why the
pg_sleep
wrapper has no effect anymore.I'd say for now I'll make another PR just to upgrade to Django 4.1, where I had all CI green. Or does anyone have any suggestions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep... a step on the road that works 🚀