-
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.1 #18830
Conversation
We need to be on >= 3.1.8 Locally there is an additional problem that somehow psycopg2 seemingly overshadows psycopg, making it appear that 3.1 works. Had to install pip install "psycopg[binary,pool]==3.1.2" to recreate the problem.
We use custom SQL that somehow doesn't get formatted in the right way using server or client side cursors.
Because full_clean validates since Django 4.1, see https://docs.djangoproject.com/en/4.2/releases/4.1/#validation-of-constraints
Come up as error: Unused "type: ignore" comment
# Conflicts: # requirements.txt
7a6a13a
to
a7dd0d8
Compare
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.
clicking around locally things seem to work
(Web analytics doesn't load anything (but it doesn't in master for me now and I'd not loaded it locally before)
I think the roll out is the interesting bit. Whether we merge with folk on standby for a fast rollback if things break in production unexpectedly or if we try and figure out how to get it into dev separately to deploying to production
AND NOT ("posthog_dashboard"."deleted" | ||
AND "posthog_dashboardtile"."deleted" | ||
AND "posthog_dashboardtile"."deleted" IS NOT NULL) |
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.
interesting change... I guess it's only enforcing that we don't return deleted things so probably fine
i added the deploy label - pushing a new commit deploys an ephemeral environment we could use as another functionality check |
Looks like it cannot deploy because of the branch name... is there something to circumvent this or do I need a new PR 😓 |
I think I've pushed a commit that fixes that 🤞
…On Tue, 28 Nov 2023 at 09:24, Julian Bez ***@***.***> wrote:
Looks like it cannot deploy because of the branch name... is there
something to circumvent this or do I need a new PR 😓
—
Reply to this email directly, view it on GitHub
<#18830 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAHQN4NLOKSPS6SOZEKPQCLYGWUUTAVCNFSM6AAAAAA7WYGIS2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMRZGQYTKNRVGY>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
# Conflicts: # posthog/api/test/__snapshots__/test_feature_flag.ambr
Found one issue with CSRF thanks due to hobby deploy 👏🏻 https://docs.djangoproject.com/en/4.2/releases/4.0/#csrf-trusted-origins-changes |
# Conflicts: # posthog/temporal/data_imports/external_data_job.py # posthog/temporal/tests/test_external_data_job.py
I can log in and things seem to work. I've just fixed some type problems so then CI should be green. What else can we check or how do we proceed? |
I'm not sure forcing the image for a deploy to dev gives us much more than the deploy label does. @frankh is infrastructure support this week. So maybe we make sure we've got his time booked and deploy this with him on hand to force a rollback if we see issues? (or we ask him how we can force an image in dev to try this there...) |
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.
LGTM
This reverts commit 580c7b1.
Problem
Upgrade to a more recent Django version.
Changes
How did you test this code?