Skip to content
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

check_rt_login function #22

Open
dmullen17 opened this issue Jul 17, 2018 · 3 comments
Open

check_rt_login function #22

dmullen17 opened this issue Jul 17, 2018 · 3 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@dmullen17
Copy link
Member

I'm using a simple helper to check if i'm logged in to RT or not for unit testing in https://github.com/NCEAS/awards-bot. It's useful for skipping unit tests when not logged in to RT.

It might make more sense for it (or a more elegant version) to live here. Thoughts?

check_rt_login <- function(rt_base) {
  base_api <- paste(stringr::str_replace(rt_base, "\\/$", ""),
                    "REST", "1.0", sep = "/")
  content <- httr::GET(base_api) %>%
    httr::content()
  
  if (stringr::str_detect(content, "Credentials required")) {
    return(FALSE)
  } else {
    return(TRUE)
  }
}
@isteves
Copy link
Collaborator

isteves commented Aug 9, 2018

@dmullen17 sorry, just getting back into RT a bit now. @amoeba and I are currently planning to do some package restructuring to get it CRAN-ready. We're trying to maintain matches to the RT API as much as possible, but I see this as a possible way to modularize the connection-checking part of rt_login. Don't have the bandwidth to fully look into this now, but will try to come back to this eventually!

@amoeba
Copy link
Collaborator

amoeba commented Aug 9, 2018

We could certainly include something like this. Can you imagine someone using it in an interactive session though or just in a using-rt-in-another-package scenario like yours? If we think enough users might use it, it might be worth considering.

If most of the utility here can capture the behavior that rt_login's return value could accomplish, I'd probably just stick with rt_login.

For your unit tests, could you just see if the RT_BASE_URL env var is set to detect whether you should try to run the tests?

@amoeba amoeba added this to the v1.0 milestone Feb 26, 2020
@amoeba amoeba self-assigned this Feb 26, 2020
@amoeba
Copy link
Collaborator

amoeba commented Mar 3, 2020

I think having something like this is probably useful but I didn't like the idea of having to make an HTTP call just to find out whether we're logged in. I'm going to move this off of the 1.0 release as it's not a critical.

If we revisit this in the future, it might be possible to figure out whether we're logged in without making an HTTP call. For example, we could capture the cookies on the rt_login call, store them in the package's state, and query that.

@amoeba amoeba modified the milestones: v1.0, Next release Mar 3, 2020
@amoeba amoeba added the enhancement New feature or request label Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants