Skip to content

chore(auth): Update sentry.identity.oauth2 to support customer domains#39052

Merged
dashed merged 2 commits intomasterfrom
hybrid-cloud/oauth2-customer-domains-pipeline-pt-2
Oct 4, 2022
Merged

chore(auth): Update sentry.identity.oauth2 to support customer domains#39052
dashed merged 2 commits intomasterfrom
hybrid-cloud/oauth2-customer-domains-pipeline-pt-2

Conversation

@dashed
Copy link
Copy Markdown
Member

@dashed dashed commented Sep 20, 2022

Branches from #39016.

This delegates the pipeline redis store to propagate customer domain information for the OAuth2 implementation that resides in sentry.identity.oauth2.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Was responses not able to do the mocking you needed? It should be able to mock out anything using urllib3.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@markstory I added asserts to check the arguments passed to safe_urlopen() for the token exchange.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can do that with responses as well. responses.calls gives you access to all the requests that were made and the url/parameters that were used for each mocked request.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@markstory I've updated to responses.calls.

@armenzg armenzg removed the request for review from a team September 22, 2022 19:34
@armenzg
Copy link
Copy Markdown
Member

armenzg commented Sep 22, 2022

Untagging ecosystem. Retag if our review is needed.

@dashed dashed force-pushed the hybrid-cloud/oauth2-customer-domains-pipeline branch from 0ec004c to fec27dd Compare September 27, 2022 00:25
@dashed dashed requested a review from a team as a September 27, 2022 00:25
@dashed dashed force-pushed the hybrid-cloud/oauth2-customer-domains-pipeline-pt-2 branch from 0f9c583 to 8039526 Compare September 27, 2022 00:25
@dashed dashed force-pushed the hybrid-cloud/oauth2-customer-domains-pipeline branch from fec27dd to 8d9f92f Compare September 27, 2022 01:55
@dashed dashed force-pushed the hybrid-cloud/oauth2-customer-domains-pipeline-pt-2 branch from 8039526 to 1728d32 Compare September 27, 2022 01:55
@dashed dashed force-pushed the hybrid-cloud/oauth2-customer-domains-pipeline-pt-2 branch from 1728d32 to 8595bc7 Compare September 27, 2022 02:20
Base automatically changed from hybrid-cloud/oauth2-customer-domains-pipeline to master September 28, 2022 17:53
@dashed dashed merged commit ca7dff9 into master Oct 4, 2022
@dashed dashed deleted the hybrid-cloud/oauth2-customer-domains-pipeline-pt-2 branch October 4, 2022 23:11
provider_key=view.provider_key,
config=view.config,
)
return nested_pipeline.fetch_state(key)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why didn’t you just put this in the base pipeline fetch_state?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@github-actions github-actions bot locked and limited conversation to collaborators Oct 21, 2022
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.

5 participants