Skip to content

ref(control_silo): Move Identitiy model to users module #76272

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

Merged

Conversation

Christinarlong
Copy link
Contributor

Part of moving control silo user related resources into the users module. Includes adding import shim for any getsentry refs. Also adds minor typing to some identity functions (will point out).

Apart of (#73856)

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Aug 15, 2024
Comment on lines 65 to 68
def get_provider(self) -> Identity:
from sentry.identity import get

return get(self.type)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I interpreted the get() function returning a cls("Identity") which would just be an Identity class... (???)

Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't this be IdentityProvider. The get() will return an instance of the registered types (Providers) based on the string name provided.

Providers are registered here

register(SlackIdentityProvider) # NOQA
register(GitHubIdentityProvider) # NOQA
register(GitHubEnterpriseIdentityProvider) # NOQA
register(VSTSIdentityProvider) # NOQA
register(VstsExtensionIdentityProvider) # NOQA
register(VercelIdentityProvider) # NOQA
register(BitbucketIdentityProvider) # NOQA
register(GitlabIdentityProvider) # NOQA
register(GoogleIdentityProvider) # NOQA
register(DiscordIdentityProvider) # NOQA

The Slack provider is found

class SlackIdentityProvider(OAuth2Provider):

and the class tree for those ends in Provider

class Provider(PipelineProvider, abc.ABC):

Copy link
Contributor Author

@Christinarlong Christinarlong Aug 15, 2024

Choose a reason for hiding this comment

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

Ohhhh, gotchas. I completely missed the whole provider registration and that the self.values in IdentityManager was populated with Providers from the registration. 🫠 Thank yous for the class by class explanation!

@Christinarlong Christinarlong marked this pull request as ready for review August 15, 2024 17:37
@Christinarlong Christinarlong requested review from a team as code owners August 15, 2024 17:37
@Christinarlong Christinarlong merged commit 4868f8e into master Aug 16, 2024
48 checks passed
@Christinarlong Christinarlong deleted the christinarlong/consolidate-users-models-identity branch August 16, 2024 18:21
@github-actions github-actions bot locked and limited conversation to collaborators Sep 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants