From aaf4a0913188cba4a1af7239de62c3e7ed318677 Mon Sep 17 00:00:00 2001
From: Koen Hufkens <koen.hufkens@gmail.com>
Date: Fri, 23 Feb 2024 17:48:57 +0100
Subject: [PATCH 01/14] update git ignore

---
 .gitignore      | 1 +
 R/service-cds.R | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index 07d97ee..56687b8 100644
--- a/.gitignore
+++ b/.gitignore
@@ -5,3 +5,4 @@
 .Ruserdata
 inst/doc
 docs/
+CRAN-SUBMISSION
diff --git a/R/service-cds.R b/R/service-cds.R
index 2fe540a..0e33117 100644
--- a/R/service-cds.R
+++ b/R/service-cds.R
@@ -42,7 +42,7 @@ cds_service <- R6::R6Class("ecmwfr_cds",
       private$status <- "submitted"
       private$code <- ct$code
       private$name <- ct$request_id
-      private$retry <- 5
+      private$retry <- 30
       private$next_retry <- Sys.time() + private$retry
       private$url <- wf_server(id = ct$request_id, service = "cds")
       return(self)

From a95b9d0e12a39d4d962eac81809f053ade4e8367 Mon Sep 17 00:00:00 2001
From: Koen Hufkens <koen.hufkens@gmail.com>
Date: Fri, 23 Feb 2024 18:06:24 +0100
Subject: [PATCH 02/14] dynamic retry

---
 DESCRIPTION       | 2 +-
 NAMESPACE         | 1 +
 NEWS.md           | 1 +
 R/service-ads.R   | 1 -
 R/service-cds.R   | 1 -
 R/service.R       | 2 ++
 R/wf_request.R    | 4 ++++
 man/wf_request.Rd | 6 +++++-
 8 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/DESCRIPTION b/DESCRIPTION
index 0022779..804cedd 100644
--- a/DESCRIPTION
+++ b/DESCRIPTION
@@ -41,7 +41,7 @@ Imports:
     uuid
 License: AGPL-3
 ByteCompile: true
-RoxygenNote: 7.2.1
+RoxygenNote: 7.2.3
 Suggests:
     rmarkdown,
     covr,
diff --git a/NAMESPACE b/NAMESPACE
index c10800b..bd02cb2 100644
--- a/NAMESPACE
+++ b/NAMESPACE
@@ -12,6 +12,7 @@ export(wf_services)
 export(wf_set_key)
 export(wf_transfer)
 export(wf_user_info)
+import(uuid)
 importFrom(R6,R6Class)
 importFrom(memoise,memoise)
 importFrom(utils,browseURL)
diff --git a/NEWS.md b/NEWS.md
index 96779e9..6922b5a 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,6 +1,7 @@
 # ecmwfr 1.5.1
 
 * Logic patch for 202 http error on long runs
+* dynamic retry polling to avoid API rate limiting (default = 30s)
 
 # ecmwfr 1.5.0
 
diff --git a/R/service-ads.R b/R/service-ads.R
index 8207d9d..0cca79e 100644
--- a/R/service-ads.R
+++ b/R/service-ads.R
@@ -46,7 +46,6 @@ ads_service <- R6::R6Class("ecmwfr_ads", inherit = cds_service,
       private$status <- "submitted"
       private$code <- ct$code
       private$name <- ct$request_id
-      private$retry <- 5
       private$next_retry <- Sys.time() + private$retry
       private$url <- wf_server(id = ct$request_id, service = "ads")
       return(self)
diff --git a/R/service-cds.R b/R/service-cds.R
index 0e33117..748beb5 100644
--- a/R/service-cds.R
+++ b/R/service-cds.R
@@ -42,7 +42,6 @@ cds_service <- R6::R6Class("ecmwfr_cds",
       private$status <- "submitted"
       private$code <- ct$code
       private$name <- ct$request_id
-      private$retry <- 30
       private$next_retry <- Sys.time() + private$retry
       private$url <- wf_server(id = ct$request_id, service = "cds")
       return(self)
diff --git a/R/service.R b/R/service.R
index 7c745dd..4568d1b 100644
--- a/R/service.R
+++ b/R/service.R
@@ -3,11 +3,13 @@ service <- R6::R6Class("ecmwfr_service", cloneable = FALSE,
     initialize = function(request,
                           user,
                           url,
+                          retry,
                           path = tempdir(),
                           verbose = TRUE) {
       private$user <- user
       private$request <- request
       private$path <- path
+      private$retry <- retry
       private$file <- file.path(path, request$target)
       private$verbose <- verbose
       private$url <- url
diff --git a/R/wf_request.R b/R/wf_request.R
index e3244ac..a783e22 100644
--- a/R/wf_request.R
+++ b/R/wf_request.R
@@ -27,6 +27,8 @@
 #' @param path path were to store the downloaded data
 #' @param time_out how long to wait on a download to start (default =
 #' \code{3*3600} seconds).
+#' @param retry polling frequency of submitted request for downloading (default =
+#' \code{30} seconds).
 #' @param transfer logical, download data TRUE or FALSE (default = TRUE)
 #' @param request nested list with query parameters following the layout
 #' as specified on the ECMWF APIs page
@@ -76,6 +78,7 @@ wf_request <- function(
     transfer = TRUE,
     path = tempdir(),
     time_out = 3600,
+    retry = 30,
     job_name,
     verbose = TRUE
 ) {
@@ -161,6 +164,7 @@ wf_request <- function(
     request = request,
     user = service_info$user,
     url = service_info$url,
+    retry = retry,
     path = path
     )
 
diff --git a/man/wf_request.Rd b/man/wf_request.Rd
index 50f6c6c..d68c646 100644
--- a/man/wf_request.Rd
+++ b/man/wf_request.Rd
@@ -11,6 +11,7 @@ wf_request(
   transfer = TRUE,
   path = tempdir(),
   time_out = 3600,
+  retry = 30,
   job_name,
   verbose = TRUE
 )
@@ -28,7 +29,7 @@ wf_request_batch(
 \item{request}{nested list with query parameters following the layout
 as specified on the ECMWF APIs page}
 
-\item{user}{user (email address) used to sign up for the ECMWF data service,
+\item{user}{user (email address or ID) provided by the ECMWF data service,
 used to retrieve the token set by \code{\link[ecmwfr]{wf_set_key}}}
 
 \item{transfer}{logical, download data TRUE or FALSE (default = TRUE)}
@@ -38,6 +39,9 @@ used to retrieve the token set by \code{\link[ecmwfr]{wf_set_key}}}
 \item{time_out}{how long to wait on a download to start (default =
 \code{3*3600} seconds).}
 
+\item{retry}{polling frequency of submitted request for downloading (default =
+\code{30} seconds).}
+
 \item{job_name}{optional name to use as an RStudio job and as output variable
 name. It has to be a syntactically valid name.}
 

From 90945259eece1a05a0bd2aef67efadcdf6a5ecb7 Mon Sep 17 00:00:00 2001
From: Koen Hufkens <koen.hufkens@gmail.com>
Date: Fri, 23 Feb 2024 18:17:11 +0100
Subject: [PATCH 03/14] dynamic http code

---
 R/service-ads.R |  3 +--
 R/service-cds.R | 18 ++++++++++++++++--
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/R/service-ads.R b/R/service-ads.R
index 0cca79e..74d7e83 100644
--- a/R/service-ads.R
+++ b/R/service-ads.R
@@ -34,8 +34,7 @@ ads_service <- R6::R6Class("ecmwfr_ads", inherit = cds_service,
 
       # grab content, to look at the status
       ct <- httr::content(response)
-
-      ct$code <- 202
+      ct$code <- httr::status_code(response)
 
       # some verbose feedback
       if (private$verbose) {
diff --git a/R/service-cds.R b/R/service-cds.R
index 748beb5..fbeff05 100644
--- a/R/service-cds.R
+++ b/R/service-cds.R
@@ -29,9 +29,9 @@ cds_service <- R6::R6Class("ecmwfr_cds",
       }
 
       # grab content, to look at the status
+      # and code
       ct <- httr::content(response)
-
-      ct$code <- 202
+      ct$code <- httr::status_code(response)
 
       # some verbose feedback
       if (private$verbose) {
@@ -65,6 +65,7 @@ cds_service <- R6::R6Class("ecmwfr_cds",
 
       key <- wf_get_key(user = private$user, service = private$service)
 
+      # set retry time
       retry_in <- as.numeric(private$next_retry) - as.numeric(Sys.time())
 
       if (retry_in > 0) {
@@ -90,6 +91,19 @@ cds_service <- R6::R6Class("ecmwfr_cds",
       ct <- httr::content(response)
       private$status <- ct$state
 
+      # trap general http error most likely
+      # will fail on spamming the service too fast
+      # with a high retry rate
+      if (httr::http_error(response)) {
+        stop(paste0(
+          httr::content(response),
+          "--- check your retry rate!"),
+          call. = FALSE
+        )
+      }
+
+      # checks the status of the true download, not the http status
+      # of the call itself
       if (private$status != "completed" || is.null(private$status)) {
         private$code <- 202
         private$file_url <- NA # just ot be on the safe side

From b2cd432ebaee33db65b812542f605e3889945ed4 Mon Sep 17 00:00:00 2001
From: Koen Hufkens <koen.hufkens@gmail.com>
Date: Fri, 23 Feb 2024 18:40:51 +0100
Subject: [PATCH 04/14] xml2 dependency code coverage

---
 DESCRIPTION | 1 +
 1 file changed, 1 insertion(+)

diff --git a/DESCRIPTION b/DESCRIPTION
index 804cedd..ba038f1 100644
--- a/DESCRIPTION
+++ b/DESCRIPTION
@@ -45,6 +45,7 @@ RoxygenNote: 7.2.3
 Suggests:
     rmarkdown,
     covr,
+    xml2,
     testthat,
     terra,
     maps,

From 882cf7ef3f5b73c7b2ac4c2b6c6dcaa3702af721 Mon Sep 17 00:00:00 2001
From: Koen Hufkens <koen.hufkens@gmail.com>
Date: Fri, 23 Feb 2024 19:13:26 +0100
Subject: [PATCH 05/14] update runner specs

---
 .github/workflows/test-coverage.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/test-coverage.yaml b/.github/workflows/test-coverage.yaml
index 8182af8..4680ba1 100644
--- a/.github/workflows/test-coverage.yaml
+++ b/.github/workflows/test-coverage.yaml
@@ -12,8 +12,8 @@ name: test-coverage
 
 jobs:
   test-coverage:
-    timeout-minutes: 30
-    runs-on: ubuntu-20.04
+    timeout-minutes: 60
+    runs-on: Ubuntu-latest
     env:
       GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
       ADS: ${{secrets.ADS}}

From 0986205bd1943d623f240e8c1ad9694101f987eb Mon Sep 17 00:00:00 2001
From: Koen Hufkens <koen.hufkens@gmail.com>
Date: Fri, 23 Feb 2024 19:25:27 +0100
Subject: [PATCH 06/14] new covr workflow from rlib

---
 .github/workflows/test-coverage.yaml | 62 ++++++++++++++--------------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/.github/workflows/test-coverage.yaml b/.github/workflows/test-coverage.yaml
index 4680ba1..dd1d410 100644
--- a/.github/workflows/test-coverage.yaml
+++ b/.github/workflows/test-coverage.yaml
@@ -1,19 +1,16 @@
+# Workflow derived from https://github.com/r-lib/actions/tree/v2/examples
+# Need help debugging build failures? Start at https://github.com/r-lib/actions#where-to-find-help
 on:
   push:
-    branches:
-      - main
-      - master
+    branches: [main, master]
   pull_request:
-    branches:
-      - main
-      - master
+    branches: [main, master]
 
 name: test-coverage
 
 jobs:
   test-coverage:
-    timeout-minutes: 60
-    runs-on: Ubuntu-latest
+    runs-on: ubuntu-latest
     env:
       GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
       ADS: ${{secrets.ADS}}
@@ -21,33 +18,36 @@ jobs:
       WEBAPI: ${{secrets.WEBAPI}}
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
+
       - uses: r-lib/actions/setup-r@v2
-      - uses: r-lib/actions/setup-pandoc@v2
+        with:
+          use-public-rspm: true
 
-      - name: Query dependencies
-        run: |
-          install.packages('remotes')
-          saveRDS(remotes::dev_package_deps(dependencies = TRUE), ".github/depends.Rds", version = 2)
-          writeLines(sprintf("R-%i.%i", getRversion()$major, getRversion()$minor), ".github/R-version")
-        shell: Rscript {0}
+      - uses: r-lib/actions/setup-r-dependencies@v2
+        with:
+          extra-packages: any::covr
+          needs: coverage
 
-      - name: Install system dependencies
-        if: runner.os == 'Linux'
+      - name: Test coverage
         run: |
-          while read -r cmd
-          do
-            eval sudo $cmd
-          done < <(Rscript -e 'writeLines(remotes::system_requirements("ubuntu", "20.04"))')
-          sudo apt-get install libgdal-dev libproj-dev libgeos-dev libudunits2-dev netcdf-bin libsodium-dev libsodium23
+          covr::codecov(
+            quiet = FALSE,
+            clean = FALSE,
+            install_path = file.path(Sys.getenv("RUNNER_TEMP"), "package")
+          )
+        shell: Rscript {0}
 
-      - name: Install dependencies
+      - name: Show testthat output
+        if: always()
         run: |
-          install.packages(c("remotes"))
-          remotes::install_deps(dependencies = TRUE)
-          remotes::install_cran("covr")
-        shell: Rscript {0}
+          ## --------------------------------------------------------------------
+          find ${{ runner.temp }}/package -name 'testthat.Rout*' -exec cat '{}' \; || true
+        shell: bash
 
-      - name: Test coverage
-        run: covr::codecov()
-        shell: Rscript {0}
+      - name: Upload test results
+        if: failure()
+        uses: actions/upload-artifact@v4
+        with:
+          name: coverage-test-failures
+          path: ${{ runner.temp }}/package

From 99a1c7db3eada9f074367c3d08a256b3a2724dab Mon Sep 17 00:00:00 2001
From: Koen Hufkens <koen.hufkens@gmail.com>
Date: Fri, 23 Feb 2024 21:33:23 +0100
Subject: [PATCH 07/14] ignore webapi

---
 tests/testthat/test_webapi.r | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/testthat/test_webapi.r b/tests/testthat/test_webapi.r
index acdc26b..d546225 100644
--- a/tests/testthat/test_webapi.r
+++ b/tests/testthat/test_webapi.r
@@ -16,6 +16,8 @@ ON_GIT <- ifelse(
   TRUE
 )
 
+ON_GIT <- TRUE
+
 # format request (see below)
 my_request <- list(
   stream = "oper",

From a8761a5dfb614faa66cb68e4823646ae05571519 Mon Sep 17 00:00:00 2001
From: Koen Hufkens <koen.hufkens@gmail.com>
Date: Fri, 23 Feb 2024 21:40:59 +0100
Subject: [PATCH 08/14] disable webapi checks

---
 R/wf_request_batch.R      | 3 ++-
 tests/testthat/test_cds.R | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/R/wf_request_batch.R b/R/wf_request_batch.R
index 3043247..69a26a9 100644
--- a/R/wf_request_batch.R
+++ b/R/wf_request_batch.R
@@ -53,7 +53,8 @@ wf_request_batch <- function(
           queue[[1]],
           user = user[1],
           time_out = time_out[1],
-          path = path[1], transfer = FALSE
+          path = path[1],
+          transfer = FALSE
           )
         queue <- queue[-1]
         user <- user[-1]
diff --git a/tests/testthat/test_cds.R b/tests/testthat/test_cds.R
index 37274d9..70c76e3 100644
--- a/tests/testthat/test_cds.R
+++ b/tests/testthat/test_cds.R
@@ -6,8 +6,6 @@ if(!("ecmwfr" %in% keyring::keyring_list()$keyring)){
   keyring::keyring_create("ecmwfr", password = "test")
 }
 
-login_check <- FALSE
-
 # check if on github
 ON_GIT <- ifelse(
   Sys.getenv("GITHUB_ACTION") == "",
@@ -89,6 +87,8 @@ test_that("cds datasets returns data.frame or list", {
                                    simplify = FALSE), "list"))
 })
 
+login_check <- FALSE
+
 # Testing the cds request function
 test_that("cds request", {
   skip_on_cran()

From 64fce5cc6e84bb7d0dafb2058eb81651b702a6ec Mon Sep 17 00:00:00 2001
From: Koen Hufkens <koen.hufkens@gmail.com>
Date: Fri, 23 Feb 2024 21:56:11 +0100
Subject: [PATCH 09/14] update checks

---
 tests/testthat/test_cds.R    | 2 --
 tests/testthat/test_webapi.r | 1 +
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/testthat/test_cds.R b/tests/testthat/test_cds.R
index 70c76e3..fc83878 100644
--- a/tests/testthat/test_cds.R
+++ b/tests/testthat/test_cds.R
@@ -87,8 +87,6 @@ test_that("cds datasets returns data.frame or list", {
                                    simplify = FALSE), "list"))
 })
 
-login_check <- FALSE
-
 # Testing the cds request function
 test_that("cds request", {
   skip_on_cran()
diff --git a/tests/testthat/test_webapi.r b/tests/testthat/test_webapi.r
index d546225..28ed122 100644
--- a/tests/testthat/test_webapi.r
+++ b/tests/testthat/test_webapi.r
@@ -16,6 +16,7 @@ ON_GIT <- ifelse(
   TRUE
 )
 
+# force to skip webapi checks
 ON_GIT <- TRUE
 
 # format request (see below)

From 2f7f1922398883c39a3c91b505856630eb3a4b35 Mon Sep 17 00:00:00 2001
From: Koen Hufkens <koen.hufkens@gmail.com>
Date: Fri, 23 Feb 2024 22:17:04 +0100
Subject: [PATCH 10/14] update batch downloads - unstable api gives test
 routine grief

---
 R/wf_request_batch.R      | 4 ++++
 man/wf_request.Rd         | 1 +
 tests/testthat/test_cds.R | 3 ++-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/R/wf_request_batch.R b/R/wf_request_batch.R
index 69a26a9..ba33127 100644
--- a/R/wf_request_batch.R
+++ b/R/wf_request_batch.R
@@ -3,6 +3,8 @@
 #' to the service. Most ECMWF services are limited to 20 concurrent requests
 #' (default = 2).
 #' @param total_timeout overall timeout limit for all the requests in seconds.
+#' @param retry polling frequency of submitted request for downloading (default =
+#' \code{30} seconds).
 #' @importFrom R6 R6Class
 #'
 #' @rdname wf_request
@@ -13,6 +15,7 @@ wf_request_batch <- function(
     user,
     path = tempdir(),
     time_out = 3600,
+    retry = 5,
     total_timeout = length(request_list)*time_out/workers
 ) {
 
@@ -53,6 +56,7 @@ wf_request_batch <- function(
           queue[[1]],
           user = user[1],
           time_out = time_out[1],
+          retry = retry,
           path = path[1],
           transfer = FALSE
           )
diff --git a/man/wf_request.Rd b/man/wf_request.Rd
index d68c646..99ca46c 100644
--- a/man/wf_request.Rd
+++ b/man/wf_request.Rd
@@ -22,6 +22,7 @@ wf_request_batch(
   user,
   path = tempdir(),
   time_out = 3600,
+  retry = 5,
   total_timeout = length(request_list) * time_out/workers
 )
 }
diff --git a/tests/testthat/test_cds.R b/tests/testthat/test_cds.R
index fc83878..4f52c67 100644
--- a/tests/testthat/test_cds.R
+++ b/tests/testthat/test_cds.R
@@ -262,7 +262,7 @@ test_that("batch request tests", {
   skip_on_cran()
   skip_if(login_check)
 
-  years <- c(2017,2018)
+  years <- c(2017,2017)
   requests <- lapply(years, function(y) {
     list(
       "dataset_short_name" = "reanalysis-era5-pressure-levels",
@@ -281,6 +281,7 @@ test_that("batch request tests", {
 
   expect_output(wf_request_batch(
     requests,
+    retry = 5,
     user = "2088")
     )
 

From c790e48efdcfd23243a1242e2e8296be82324e09 Mon Sep 17 00:00:00 2001
From: Koen Hufkens <koen.hufkens@gmail.com>
Date: Fri, 23 Feb 2024 22:21:46 +0100
Subject: [PATCH 11/14] can't cheat my own error traps, no duplicate entries
 allowed

---
 tests/testthat/test_cds.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/testthat/test_cds.R b/tests/testthat/test_cds.R
index 4f52c67..644815c 100644
--- a/tests/testthat/test_cds.R
+++ b/tests/testthat/test_cds.R
@@ -262,7 +262,7 @@ test_that("batch request tests", {
   skip_on_cran()
   skip_if(login_check)
 
-  years <- c(2017,2017)
+  years <- c(2017,2018)
   requests <- lapply(years, function(y) {
     list(
       "dataset_short_name" = "reanalysis-era5-pressure-levels",

From 1161daecca4f69c577f9a03f6ae8b32302b11d10 Mon Sep 17 00:00:00 2001
From: Koen Hufkens <koen.hufkens@gmail.com>
Date: Sat, 24 Feb 2024 14:10:26 +0100
Subject: [PATCH 12/14] workflow

---
 .github/workflows/R-CMD-check.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml
index a3ac618..9c89781 100644
--- a/.github/workflows/R-CMD-check.yaml
+++ b/.github/workflows/R-CMD-check.yaml
@@ -26,6 +26,9 @@ jobs:
 
     env:
       GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
+      ADS: ${{secrets.ADS}}
+      CDS: ${{secrets.CDS}}
+      WEBAPI: ${{secrets.WEBAPI}}
       R_KEEP_PKG_SOURCE: yes
 
     steps:

From fb2e939ede890d557604bd479035b1165e8b4724 Mon Sep 17 00:00:00 2001
From: Koen Hufkens <koen.hufkens@gmail.com>
Date: Sat, 24 Feb 2024 14:10:38 +0100
Subject: [PATCH 13/14] syntax

---
 tests/testthat/test_cds.R | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/testthat/test_cds.R b/tests/testthat/test_cds.R
index 644815c..22924af 100644
--- a/tests/testthat/test_cds.R
+++ b/tests/testthat/test_cds.R
@@ -279,7 +279,8 @@ test_that("batch request tests", {
       "target"         = paste0(y, "-era5-demo.nc"))
   })
 
-  expect_output(wf_request_batch(
+  expect_output(
+    wf_request_batch(
     requests,
     retry = 5,
     user = "2088")

From 3744fb18c2f30cc97bca34afd6d4daceb9a7a959 Mon Sep 17 00:00:00 2001
From: Koen Hufkens <koen.hufkens@gmail.com>
Date: Sat, 24 Feb 2024 14:41:52 +0100
Subject: [PATCH 14/14] faulty logic

---
 tests/testthat/test_ads.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/testthat/test_ads.R b/tests/testthat/test_ads.R
index 58da064..f3bcf53 100644
--- a/tests/testthat/test_ads.R
+++ b/tests/testthat/test_ads.R
@@ -62,7 +62,7 @@ test_that("Could the login be set? Fails if not",{
   skip_on_cran()
 
   # check retrieval
-  expect_true(login_check)
+  expect_true(!login_check)
 })
 
 #----- formal checks ----