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

Default to the "oob" flow if httpuv isn't installed. #168

Merged
merged 1 commit into from Dec 3, 2014

Conversation

Projects
None yet
3 participants
@craigcitro
Contributor

craigcitro commented Nov 13, 2014

This modifies the logic in oauth token fetching to default to using the oob
flow (i.e. provide a URL and ask the user to copy-paste a code) whenever
httpuv isn't available to use locally.

PTAL @hadley @deflaux

@craigcitro craigcitro force-pushed the craigcitro:oob branch from 7c5f169 to 46eb4d7 Nov 13, 2014

@deflaux

This comment has been minimized.

deflaux commented Dec 2, 2014

@craigcitro Looks good to me! This is a great change to reduce some getting-started friction.

#' @export
#' @keywords internal
init_oauth2.0 <- function(endpoint, app, scope = NULL, type = NULL,
use_oob = getOption("httr_oob_default"),
is_interactive = interactive()) {
if (!is_installed("httpuv")) {
use_oob = TRUE

This comment has been minimized.

@hadley

hadley Dec 2, 2014

Member

I'd print a message here: message("httpuv not installed, defaulting to out-of-band auth") (and then also check that use_oob isn't already true

(Also <- please!)

This comment has been minimized.

@craigcitro

craigcitro Dec 3, 2014

Contributor

done -- i'm now in the dubious position of having a condition like (!a && !b), which i generally try to avoid -- but in this case i think it reads more clearly than !(a || b). feel free to disagree. :)

fixed the = ... my days spent with python are showing.

@craigcitro craigcitro force-pushed the craigcitro:oob branch from 46eb4d7 to d27a878 Dec 3, 2014

@craigcitro

This comment has been minimized.

Contributor

craigcitro commented Dec 3, 2014

PTAL

@hadley

This comment has been minimized.

Member

hadley commented Dec 3, 2014

Looks good to me. Just needs a bullet in news.

Default to the "oob" flow if httpuv isn't installed.
This modifies the logic in oauth token fetching to default to using the `oob`
flow (i.e. provide a URL and ask the user to copy-paste a code) whenever
`httpuv` isn't available to use locally.

@craigcitro craigcitro force-pushed the craigcitro:oob branch from d27a878 to 410ed4d Dec 3, 2014

@craigcitro

This comment has been minimized.

Contributor

craigcitro commented Dec 3, 2014

done.

hadley added a commit that referenced this pull request Dec 3, 2014

Merge pull request #168 from craigcitro/oob
Default to the "oob" flow if httpuv isn't installed.

@hadley hadley merged commit fdbdfef into r-lib:master Dec 3, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@hadley

This comment has been minimized.

Member

hadley commented Dec 3, 2014

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment