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

use arc_token() to handle empty tokens #126

Merged
merged 6 commits into from
Jan 22, 2024
Merged

Conversation

kbvernon
Copy link
Contributor

@kbvernon kbvernon commented Jan 20, 2024

Checklist

  • update NEWS.md
  • documentation updated with devtools::document()
  • devtools::check() passes locally

Changes

When "ARCGIS_TOKEN" is an empty string, it will sometimes cause HTTP 498 "invalid token" error. Need to NULL the empty string to handle this.

Issues that this closes

No issue opened here. See R-ArcGIS/arcgisutils#5.

Follow up tasks

Waiting for R-ArcGIS/arcgisutils#6 to be merged first.

@kbvernon
Copy link
Contributor Author

May have jumped the gun not running devtools::document() first. Hopefully, that doesn't cause any issues. devtools::check() is going to break until the {arcgisutils} PR gets merged.

@JosiahParry
Copy link
Collaborator

We need to bump the minimum version of arcgisutils to 0.1.1.9000 to ensure compatibility. Can you change the minimum version? That should also get CI to download the correct version of arcgisutils.

@kbvernon
Copy link
Contributor Author

Can do, but is there a programmatic way to do that, maybe with {usethis}, or do I just go in and edit the DESCRIPTION by hand?

@JosiahParry
Copy link
Collaborator

Yup! usethis::use_package("arcgisutils", min_version = "0.1.1.9000")

@JosiahParry
Copy link
Collaborator

@kbvernon can you remove this portion here? It's causing R CMD Check to fail because it was single handedly handling this bug. But now it should be working without this conditional.

if (token != "") {
req <-
httr2::req_auth_bearer_token(
httr2::req_url_query(est_req, f = "json"),
token
)
resp <- httr2::req_perform(req)
} else {
resp <-
httr2::req_perform(
httr2::req_url_query(est_req, f = "json")
)
}

@kbvernon
Copy link
Contributor Author

kbvernon commented Jan 22, 2024

ha, it appears this issue has come up before...

edit: which you said in that last comment, but i was too thick to register

@kbvernon
Copy link
Contributor Author

it appears i chopped out that code uncritically, let me replace it with what needs to go there

@kbvernon
Copy link
Contributor Author

devtools::check() just ran without error on my local machine, hopefully it passes here, too

@JosiahParry JosiahParry merged commit b61a676 into R-ArcGIS:main Jan 22, 2024
7 checks passed
@JosiahParry
Copy link
Collaborator

We'll make a package dev of you yet!

@kbvernon kbvernon deleted the arc_token branch January 22, 2024 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants