-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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(projects): Increase project slug limit from 50 -> 100 #75737
Merged
schew2381
merged 10 commits into
master
from
seiji/chore/increase-project-slug-max-length
Sep 30, 2024
Merged
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
50c0852
chore(projects): Increase project slug limit from 50 -> 100
schew2381 6f6ca66
use constant for slug name
schew2381 936d105
fix constant imports
schew2381 070cf83
switch to post deploy
schew2381 3e06be4
Merge branch 'master' into seiji/chore/increase-project-slug-max-length
schew2381 1bc1415
Merge branch 'master' into seiji/chore/increase-project-slug-max-length
schew2381 59d634e
Merge branch 'master' into seiji/chore/increase-project-slug-max-length
schew2381 ec0e1ff
Merge branch 'master' into seiji/chore/increase-project-slug-max-length
schew2381 4e81456
change to post_deploy
schew2381 311b2c4
fix migration oops
schew2381 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
34 changes: 34 additions & 0 deletions
34
src/sentry/migrations/0748_increase_project_slug_max_length.py
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
# Generated by Django 5.0.7 on 2024-08-07 17:15 | ||
|
||
from django.db import migrations | ||
|
||
import sentry.db.models.fields.slug | ||
from sentry.new_migrations.migrations import CheckedMigration | ||
|
||
|
||
class Migration(CheckedMigration): | ||
# This flag is used to mark that a migration shouldn't be automatically run in production. | ||
# This should only be used for operations where it's safe to run the migration after your | ||
# code has deployed. So this should not be used for most operations that alter the schema | ||
# of a table. | ||
# Here are some things that make sense to mark as post deployment: | ||
# - Large data migrations. Typically we want these to be run manually so that they can be | ||
# monitored and not block the deploy for a long period of time while they run. | ||
# - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to | ||
# run this outside deployments so that we don't block them. Note that while adding an index | ||
# is a schema change, it's completely safe to run the operation after the code has deployed. | ||
# Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment | ||
|
||
is_post_deployment = False | ||
|
||
dependencies = [ | ||
("sentry", "0747_create_datasecrecywaiver_table"), | ||
] | ||
|
||
operations = [ | ||
migrations.AlterField( | ||
model_name="project", | ||
name="slug", | ||
field=sentry.db.models.fields.slug.SentrySlugField(max_length=100, null=True), | ||
), | ||
] |
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.
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.
should we make this a post deployment migration? not quite sure how many projects there are or the scale of this migration.
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.
No, post deployment is for backfills and large indexes only, not things that modify the schema.
Increasing the size of this column should be fast. If it's not, we have a statement timeout that will cause the migration to abort within a few seconds.
I'd say that just make sure you're around to monitor this and revert it if it won't run - it should be ok though.
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.
We might get hit with an index rebuild which could throw off query performance for slug lookups. The postgres docs state
We have had an incident where a column resize invalided stats and caused an outage. We could have a similar outcome here.
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.
How do you run ANALYZE, the example in docs is a little unclear
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.
Good call @markstory, I'd forgotten about that. Maybe in that case, we should make this a post_deploy migration and trigger the analyze as part of it, or even have ops run the entire thing manually for us.
I doubt we can run the ANALYZE as part of the deploy, it'll very likely time out.
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 switched it to post deploy, do I have to include the ANALYZE as part of the migration code?