Skip to content

ref(integrations): Move methods to Managers #29159

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.

Sign up for GitHub

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

Jump to bottom
Merged
mgaeta merged 9 commits into from
Nov 9, 2021
Merged

ref(integrations): Move methods to Managers #29159

mgaeta merged 9 commits into from
Nov 9, 2021

Conversation

@mgaeta
Copy link
Contributor

@mgaeta mgaeta commented Oct 7, 2021

Refactor model methods for GroupLink and ExternalIssue and put them in Manager classes.

@mgaeta mgaeta requested a review from a team October 7, 2021 18:57
@mgaeta mgaeta requested a review from a team as a code owner October 7, 2021 18:57
ceorourke
ceorourke reviewed Oct 7, 2021
View reviewed changes
src/sentry/models/grouplink.py Outdated

group_id = BoundedBigIntegerField()
project_id = BoundedBigIntegerField(db_index=True)
group = FlexibleForeignKey("sentry.Group")
Copy link
Member

@ceorourke ceorourke Oct 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this require a migration? https://develop.sentry.dev/database-migrations/#altering-column-types

@mgaeta mgaeta force-pushed the fix/api-2163-slack-actions-3 branch 2 times, most recently from de1c1dc to 598929e Compare October 7, 2021 21:48
Base automatically changed from fix/api-2163-slack-actions-3 to master October 7, 2021 22:21
@mgaeta mgaeta requested a review from a team as a code owner October 11, 2021 23:17
markstory
markstory approved these changes Oct 12, 2021
View reviewed changes
Copy link
Member

@markstory markstory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Migration seems good as it is only state changes.

wedamija
wedamija reviewed Oct 13, 2021
View reviewed changes
@mgaeta mgaeta force-pushed the fix/api-2163-slack-actions-4 branch from c2cc183 to d1a9f72 Compare October 19, 2021 20:19
@github-actions
Copy link
Contributor

github-actions bot commented Oct 19, 2021

This PR has a migration; here is the generated SQL

BEGIN;
--
-- Custom state/database change combination
--
COMMIT;

@mgaeta mgaeta force-pushed the fix/api-2163-slack-actions-4 branch from b83620d to 44398cb Compare October 19, 2021 23:38
@mgaeta mgaeta force-pushed the fix/api-2163-slack-actions-4 branch from c2a86ce to b392b59 Compare October 25, 2021 18:24
@mgaeta mgaeta requested review from a team and wedamija October 25, 2021 18:25
@mgaeta
Copy link
Contributor Author

mgaeta commented Oct 25, 2021

Hi @wedamija I haven't gotten around to this PR in a while can you help me see if there's anything else I need to do?

wedamija
wedamija reviewed Oct 25, 2021
View reviewed changes
Copy link
Member

@wedamija wedamija left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, although we'll need to regenerate the migration to fix the conflicts

@mgaeta
Copy link
Contributor Author

mgaeta commented Oct 26, 2021

I think this is blocked by check migration / check migration. Do you know how to satisfy it?

@wedamija
Copy link
Member

wedamija commented Oct 29, 2021

I think this is blocked by check migration / check migration. Do you know how to satisfy it?

It's because the state in the migration doesn't match the state on the models - probably caused by adding db_constraint=False to the models but not the migration.

@mgaeta mgaeta force-pushed the fix/api-2163-slack-actions-4 branch from b92f943 to e11f46a Compare November 8, 2021 22:56
@mgaeta
Copy link
Contributor Author

mgaeta commented Nov 9, 2021 •
edited
Loading

@wedamija It worked! Thanks

@mgaeta mgaeta merged commit d529545 into master Nov 9, 2021
@mgaeta mgaeta deleted the fix/api-2163-slack-actions-4 branch November 9, 2021 17:09
@github-actions github-actions bot locked and limited conversation to collaborators Nov 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@wedamija wedamija wedamija left review comments

@ceorourke ceorourke ceorourke left review comments

@markstory markstory markstory approved these changes

+1 more reviewer

@scefali scefali scefali left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Component: Notifications

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@mgaeta @wedamija @markstory @scefali @ceorourke