-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Create OrganizationContributors table in region dbs #146
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
base: master
Are you sure you want to change the base?
feat: Create OrganizationContributors table in region dbs #146
Conversation
| indexes = [ | ||
| models.Index(fields=["date_updated"]), |
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.
[PerformanceOptimization]
Missing index on num_actions field could cause performance issues when resetting action counts at billing period boundaries. Based on the PR description, num_actions gets cleared at the start of every billing period, which likely requires querying contributors by this field.
Consider adding an index if you'll be running queries like:
- Finding all contributors with actions to reset
- Filtering contributors by action count thresholds
If such queries are needed, add to the model's Meta:
indexes = [
models.Index(fields=["date_updated"]),
models.Index(fields=["num_actions"]),
]Context for Agents
Missing index on `num_actions` field could cause performance issues when resetting action counts at billing period boundaries. Based on the PR description, `num_actions` gets cleared at the start of every billing period, which likely requires querying contributors by this field.
Consider adding an index if you'll be running queries like:
- Finding all contributors with actions to reset
- Filtering contributors by action count thresholds
If such queries are needed, add to the model's Meta:
```python
indexes = [
models.Index(fields=["date_updated"]),
models.Index(fields=["num_actions"]),
]
```
File: src/sentry/models/organizationcontributors.py
Line: 41|
|
||
| integration_id = HybridCloudForeignKey("sentry.Integration", on_delete="CASCADE") | ||
|
|
||
| external_identifier = models.CharField(max_length=255) |
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.
[BestPractice]
The migration defines external_identifier with db_index=True (line 41), but the model definition (line 25 in organizationcontributors.py) doesn't include this parameter. This inconsistency means the model and migration are out of sync.
The external_identifier field is part of a unique constraint, so it will be indexed as part of that constraint. However, if you need a separate index for queries that filter only by external_identifier, you should add db_index=True to the model:
external_identifier = models.CharField(max_length=255, db_index=True)Or if the unique constraint index is sufficient, remove db_index=True from the migration.
Context for Agents
The migration defines `external_identifier` with `db_index=True` (line 41), but the model definition (line 25 in `organizationcontributors.py`) doesn't include this parameter. This inconsistency means the model and migration are out of sync.
The `external_identifier` field is part of a unique constraint, so it will be indexed as part of that constraint. However, if you need a separate index for queries that filter only by `external_identifier`, you should add `db_index=True` to the model:
```python
external_identifier = models.CharField(max_length=255, db_index=True)
```
Or if the unique constraint index is sufficient, remove `db_index=True` from the migration.
File: src/sentry/models/organizationcontributors.py
Line: 25
Updated version of https://github.com/getsentry/getsentry/pull/18880
1 pager with table info: https://www.notion.so/sentry/1-Pager-AI-Code-Review-Sub-Spec-2a98b10e4b5d80e29737f9c58f54efb9
Adds a new table that will hold all of the OrganizationContributors for a particular sentry org / integration_id combo.
If a user belongs and/or swaps to a separate Github org or sentry org, there will be a separate entry for that individual.
num_actions gets updated every time a successful seer request occurs, and num_actions gets cleared at the start of every billing period
Closes https://linear.app/getsentry/issue/ENG-5929/create-organizationcontributors-table
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.
Copied from getsentry#103839
Original PR: getsentry#103839
Introduce
OrganizationContributorsmodel and corresponding schema migrationThis PR adds a new region-siloed Django model
OrganizationContributorsand a matching migration (1008_add_organization_contributors.py). The table captures per-organization, per-integration contributor metadata (external identifier, optional alias, action counter, timestamps) and enforces uniqueness on the (organization_id,integration_id,external_identifier) tuple. An index ondate_updatedis included to accelerate time-range queries. No application-level logic is added yet; future PRs will populate and mutatenum_actions.Key Changes
• Created
src/sentry/models/organizationcontributors.pydefining theOrganizationContributorsmodel with region silo decorator and data fields• Added migration
src/sentry/migrations/1008_add_organization_contributors.pyto create tablesentry_organizationcontributors, unique constraint, anddate_updatedindex• Set
__relocation_scope__ = RelocationScope.Excludedto keep rows from backup/restore flowsAffected Areas
• Database schema (new table)
•
sentrymodels moduleThis summary was automatically generated by @propel-code-bot