use rstudioapi instead of readline if calling oauth_flow_auth_code() from RStudio IDE #410
Conversation
fh-mthomson
changed the title
|
In case it's relevant, gargle uses The short version is to support Jupyter notebooks and, in particular, Google Colab. |
fh-mthomson
changed the title
Super relevant! I simplified a bit to only use I do think this PR is a meaningful improvement; it would prevent folks on our end from missing an Okta re-auth prompt several times a day. |
R/oauth-flow-password.R
Outdated
| if (is_rstudio_session()) { | ||
| check_installed("rstudioapi") | ||
| result <- rstudioapi::askForPassword(prompt) | ||
| } else { |
There was a problem hiding this comment.
Should this have an !is_interactive() branch with an informative error?
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
There was a problem hiding this comment.
There are two scenarios running outside of RStudio that I'm worried about:
- You're running R interactively and call
knit(): doesreadline()cause execution to pause? Do you see the correct prompt? - You're running R in batch mode and call
knit(): won'treadline()will return""causing an authentication problem that's hard to diagnose?
There was a problem hiding this comment.
Any more thoughts on this comment? I think it's the main blocker for merging this PR.
There was a problem hiding this comment.
Sorry for the delay while juggling some other things. Of note, this PR is a specific workaround for Snowflake auth in Workbench and was before I chatted with @atheriel about his efforts to integrate OAuth more directly via Workbench (rather than our current wrappers around httr2 which have been holding up reasonably well).
Regardless, I had a chance to think a bit more about this, motivated by Quarto's execution approach not supporting interactive user prompts.
I've pushed a change, per your suggestion and very helpful test cases! I think it's closer to a reasonable approach.
Current behavior:
- In RStudio:
a. .rmd -> knitting: pauses to receive prompt (new, yay!)
b. .qmd -> render: fails with an expected, or at least, well-documented error:
Error:
! RStudio not running
- Outside of RStudio: both of your test cases:
## Error in `prompt_user()` at script.R:18:3:
## ! Unable to obtain user input in a non-interactive session.
Open to any thoughts!
|
Thanks for working on this! |
R/oauth-flow-password.R
Outdated
| if (is_rstudio_session()) { | ||
| check_installed("rstudioapi") | ||
| result <- rstudioapi::askForPassword(prompt) | ||
| } else { |
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
R/oauth-flow-password.R
Outdated
| if (is_rstudio_session()) { | ||
| check_installed("rstudioapi") | ||
| result <- rstudioapi::askForPassword(prompt) | ||
| } else { |
There was a problem hiding this comment.
There are two scenarios running outside of RStudio that I'm worried about:
- You're running R interactively and call
knit(): doesreadline()cause execution to pause? Do you see the correct prompt? - You're running R in batch mode and call
knit(): won'treadline()will return""causing an authentication problem that's hard to diagnose?
Co-authored-by: Hadley Wickham <[email protected]>
|
Going to close, for the reasons above, namely that Workbench <> Snowflake authentication will be more robust for most use cases. Feel free to re-open if there's interest/benefits otherwise! |
Closes #406
Test plan
Running
oauth_flow_auth_code_read("state")successfully prompts twice (initial prompt, then state) in either case described in parent issue:Implementation considerations
askpass, given broader generalizability outside of RStudio IDE; however, this doesn't scale as well off the shelf to the non-interactive use case (e.g., knitting in RStudio) - per?askpass::askpass:rstudioapi>askpass, but this may be too layered - open to any feedback!Note: leaving this instance of readline() since it's not really used to solicit a character string from the user and is likely better handled by another prompt function, anyway.