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.

Sign up for GitHub

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

Change custom UA to environmental variable #737

Merged
merged 9 commits into from
Nov 5, 2024
Merged

Change custom UA to environmental variable #737

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6f7f6fd
Custom UAs not actually getting in user agent.
ldecicco-USGS Oct 31, 2024
46dfe13
Change custom UA to Sys.get.
ldecicco-USGS Oct 31, 2024
04ae771
forgot a GHA
ldecicco-USGS Oct 31, 2024
d859170
is this better for yaml?
ldecicco-USGS Oct 31, 2024
f78d99f
spelling
ldecicco-USGS Nov 1, 2024
ff9264e
yml?
ldecicco-USGS Nov 1, 2024
1227ba2
does this help
ldecicco-USGS Nov 1, 2024
a4a3e0a
yml
ldecicco-USGS Nov 1, 2024
2374fd6
Add some documentation
ldecicco-USGS Nov 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/R-CMD-check.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ jobs:
env:
GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
R_KEEP_PKG_SOURCE: yes
CUSTOM_DR_UA: 'GitHub_CI'

steps:
- uses: actions/checkout@9a9194f87191a7e9055e3e9b95b8cfb13023bb08
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/pkgdown.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ jobs:
group: pkgdown-${{ github.event_name != 'pull_request' || github.run_id }}
env:
GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
CUSTOM_DR_UA: 'GitHub_CI'
steps:
- uses: actions/checkout@c0a81a463886bb75afe234e07a9fd5bb79219196

Expand Down
6 changes: 6 additions & 0 deletions .github/workflows/test-coverage.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ name: test-coverage
jobs:
test-coverage:
runs-on: macOS-latest

env:
GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
R_KEEP_PKG_SOURCE: yes
CUSTOM_DR_UA: 'GitHub_CI'

steps:
- uses: actions/checkout@c0a81a463886bb75afe234e07a9fd5bb79219196

Expand Down
1 change: 1 addition & 0 deletions .gitlab-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ variables:
BUILD_LOGS_DIR: "$CI_PROJECT_DIR/ci/logs"
NOT_CRAN: "true"
PAGES_OUTDIR: "$CI_PROJECT_DIR/public"
CUSTOM_DR_UA: "GitLab_CI"

build-image:
stage: build
Expand Down
4 changes: 2 additions & 2 deletions R/getWebServiceData.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ default_ua <- function() {

ua <- paste0(names(versions), "/", versions, collapse = " ")

if ("UA.dataRetrieval" %in% names(options)) {
ua <- paste0(ua, "/", options()[["UA.dataRetrieval"]])
if (Sys.getenv("CUSTOM_DR_UA") != "") {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice alternative to the options argument. Where do you think would be a good place to document the feature? Would it be something to advertise to power users, included in the getWebServiceData function documentation, or elsewhere since it applies to other functions too?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another idea that we could potentially put off for later: what if we set the environment variable on package load to some random alphanumeric string if it isn't already set elsewhere? Then we'd be able to at least track requests originating from the same R session. Perhaps something like the following:

if (Sys.getenv("CUSTOM_DR_UA") == "") {
  Sys.setenv("CUSTOM_DR_UA" = paste0(sample(c(LETTERS, 0:9), size = 12, replace = TRUE), collapse = ""))
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My gut says we start with advertising the change in NEWS and documentation. Then we can point a few power users to that and ask them to add it to their pipelines. THEN after you've had a chance to see how that helps (or not!) in your analytic work, we can think about adding this sort of session info. I'm worried it would just overwhelm the server log work.

ua <- paste0(ua, "/", Sys.getenv("CUSTOM_DR_UA"))
}

return(ua)
Expand Down
Loading