Skip to content
This repository was archived by the owner on Aug 30, 2023. It is now read-only.

Conversation

chinskiy
Copy link

Greetings!

Like PR #10 it allows configuration but via sentry.options
I saw that PR #10 was more than year ago that's why I create new and make changes like in PR comments

@codecov-io
Copy link

Codecov Report

Merging #19 into master will increase coverage by 0.48%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #19      +/-   ##
==========================================
+ Coverage   61.78%   62.26%   +0.48%     
==========================================
  Files           8        8              
  Lines         157      159       +2     
==========================================
+ Hits           97       99       +2     
  Misses         60       60
Impacted Files Coverage Δ
sentry_auth_google/constants.py 100% <100%> (ø) ⬆️
sentry_auth_google/provider.py 70.45% <100%> (+0.68%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 52020f5...001606a. Read the comment docs.

@evanpurkhiser
Copy link
Member

This looks pretty nice, @dcramer I can resolve the conflicts if this looks good to you.

@dcramer
Copy link
Member

dcramer commented Nov 27, 2017

I think I tried doing this elsewhere and the main concern is we can't call options.get at runtime. If we can refactor things to be in helper methods (see how the Slack/OAuth stuff looks in core) then I'm definitely +1

@evanpurkhiser
Copy link
Member

we can't call options.get at runtime

Do you mean options.register ?

@dcramer
Copy link
Member

dcramer commented Nov 28, 2017

@evanpurkhiser nah, its the get() calls in the plugin code here

@evanpurkhiser
Copy link
Member

Oh I see, at module load time, yeah I see how you're doing it with slack in the new integrations stuff.

@dcramer
Copy link
Member

dcramer commented Jan 29, 2019

This is super old, but I'd still love for us to be able to do this with integrations. We likely need to either address this with the options framework (might be tough/not doable), or make client_id/secret callable.

@dcramer
Copy link
Member

dcramer commented Jan 29, 2019

Closing this as it'll be implemented as part of Sentry core. It could still be implemented in a third party plugin as the required changes upstream will also land as part of the referenced PR.

@dcramer dcramer closed this Jan 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants