update converter check location

R/get_pvol_nl.R

@@ -1,4 +1,29 @@
 get_pvol_nl <- function(radar, time, ..., call = rlang::caller_env()) {
+  # Dutch files need to be converted to the odim format therefore we check early for the availability of the converter
+  converter <- getOption("getRad.nl_converter", "KNMI_vol_h5_to_ODIM_h5")
+  if (!file.exists(converter)) {
+    converter <- Sys.which(converter)
+  }
+  if (converter == "") {
+    cli::cli_abort(
+      c(
+        "The program to convert KNMI data to the ODIM format can't be found.",
+        "i" = "The source code for this binary can be obtained from {.file
+                 {system.file(\"extra/KNMI_vol_h5_to_ODIM_h5.c\",
+                 package=\"getRad\")}}.",
+        "i" = "Please compile the binary and include it in the search path as
+                 a program named {.val KNMI_vol_h5_to_ODIM_h5}. On linux systems
+                 this can be done with the command {.code h5cc
+                 KNMI_vol_h5_to_ODIM_h5.c -o KNMI_vol_h5_to_ODIM_h5}. If another
+                 name is used or the program is not in the search path, use
+                 options to locate the program ({.code
+                 options(getRad.nl_converter=\"\")})."
+      ),
+      class = "getRad_error_no_nl_converter_found",
+      call = call
+    )
+  }
+
   #  Convert radar names into the dirname and version used by the KNMI data platform.
   mapped_radar <-
     radar_recode(
@@ -53,33 +78,25 @@ get_pvol_nl <- function(radar, time, ..., call = rlang::caller_env()) {
       httr2::req_url(req = resp$request) |>
       httr2::req_headers(Authorization = NULL) |>
       httr2::req_perform(path = file, error_call = call)
-    # Dutch files need to be converted to the odim format
-    converter <- getOption("getRad.nl_converter", "KNMI_vol_h5_to_ODIM_h5")
-    if (!file.exists(converter)) {
-      converter <- Sys.which(converter)
-    }
-    if (converter == "") {
-      cli::cli_abort(
-        c(
-          "The program to convert KNMI data to the ODIM format can't be found.",
-          "i" = "The source code for this binary can be obtained from {.file
-                 {system.file(\"extra/KNMI_vol_h5_to_ODIM_h5.c\",
-                 package=\"getRad\")}}.",
-          "i" = "Please compile the binary and include it in the search path as
-                 a program named {.val KNMI_vol_h5_to_ODIM_h5}. On linux systems
-                 this can be done with the command {.code h5cc
-                 KNMI_vol_h5_to_ODIM_h5.c -o KNMI_vol_h5_to_ODIM_h5}. If another
-                 name is used or the program is not in the search path, use
-                 options to locate the program ({.code
-                 options(getRad.nl_converter=\"\")})."
-        ),
-        class = "getRad_error_no_nl_converter_found",
-        call = call
-      )
-    }
     pvol_path <- NULL # to prevent no visible global variable warnings
     withr::with_tempfile("pvol_path", fileext = ".odim.h5", {
-      system(paste(converter, pvol_path, req$body))
+      system(
+        paste(converter, pvol_path, req$body),
+        ignore.stdout = TRUE,
+        ignore.stderr = TRUE
+      )
+      if (!file.exists(pvol_path)) {
+        cli::cli_abort(
+          c(
+            x = "The converter for the Netherland has not generated the
+              expected output file.",
+            i = "Using {.file {converter}} an attempt was made to convert
+              {.file {req$body}} to the odim format. Most likely the
+              converter is incorrect."
+          ),
+          class = "getRad_error_dutch_converter_failed"
+        )
+      }
       bioRad::read_pvolfile(pvol_path, ...)
     })
   })

tests/testthat/test-get_pvol_nl.R

@@ -1,18 +1,22 @@
 test_that("Pvol for the Netherlands can be downloaded", {
-  skip_if_offline()
-  skip_if(
-    Sys.which("KNMI_vol_h5_to_ODIM_h5") == "",
-    message = "Skip trying to read Dutch pvol data as no converter is present"
-  )
+  skip_if(Sys.which("KNMI_vol_h5_to_ODIM_h5") == "")
+  skip_if_offline(host = "api.dataplatform.knmi.nl")
+
   # make sure local env is used by keyring so that api key can be set
-  withr::local_options(list("keyring_backend" = "env"))
+  withr::local_options(list(
+    "keyring_backend" = "env"
+  ))
   # get public key here https://developer.dataplatform.knmi.nl/open-data-api#token
   withr::local_envvar(
     list(
       "getRad_nl_api_key" = "eyJvcmciOiI1ZTU1NGUxOTI3NGE5NjAwMDEyYTNlYjEiLCJpZCI6ImVlNDFjMWI0MjlkODQ2MThiNWI4ZDViZDAyMTM2YTM3IiwiaCI6Im11cm11cjEyOCJ9"
     )
   )
   time <- as.POSIXct("2024-4-4 20:00:00", tz = "Europe/Helsinki")
+  expect_s3_class(
+    get_pvol("nlhrw", time, param = "all"),
+    "pvol"
+  )
   pvol <- expect_s3_class(get_pvol("nlhrw", time, param = "all"), "pvol")
   expect_true(bioRad::is.pvol(pvol))
   expect_identical(
@@ -21,13 +25,34 @@ test_that("Pvol for the Netherlands can be downloaded", {
   )
 })
 
+
+test_that("Pvol for the Netherlands can be downloaded. Incorrect converter results in failure.", {
+  skip_if_offline(host = "api.dataplatform.knmi.nl")
+
+  # make sure local env is used by keyring so that api key can be set
+  withr::local_options(list(
+    "keyring_backend" = "env",
+    "getRad.nl_converter" = "ls"
+  ))
+  # get public key here https://developer.dataplatform.knmi.nl/open-data-api#token
+  withr::local_envvar(
+    list(
+      "getRad_nl_api_key" = "eyJvcmciOiI1ZTU1NGUxOTI3NGE5NjAwMDEyYTNlYjEiLCJpZCI6ImVlNDFjMWI0MjlkODQ2MThiNWI4ZDViZDAyMTM2YTM3IiwiaCI6Im11cm11cjEyOCJ9"
+    )
+  )
+  time <- as.POSIXct("2024-4-4 20:00:00", tz = "Europe/Helsinki")
+  expect_error(
+    get_pvol("nlhrw", time, param = "all"),
+    class = "getRad_error_dutch_converter_failed"
+  )
+})
+
 test_that("failure to find converter", {
-  skip_if_offline()
   withr::with_envvar(c("PATH" = ""), {
-    skip_if_not(Sys.which("KNMI_vol_h5_to_ODIM_h5") == "")
-
     # make sure local env is used by keyring so that api key can be set
-    withr::local_options(list("keyring_backend" = "env"))
+    withr::local_options(list(
+      "keyring_backend" = "env"
+    ))
     # get public key here https://developer.dataplatform.knmi.nl/open-data-api#token
     withr::local_envvar(
       list(
@@ -46,6 +71,10 @@ test_that("failure to find converter", {
 })
 
 test_that("The Netherlands non existing radar", {
+  withr::local_options(list(
+    # use random binary so checking for converter step is passed
+    "getRad.nl_converter" = "ls"
+  ))
   expect_error(
     pvol <- get_pvol(
       "nlaaa",
@@ -55,9 +84,13 @@ test_that("The Netherlands non existing radar", {
   )
 })
 test_that("Pvol for the Netherlands authenication failure", {
-  skip_if_offline()
+  skip_if_offline(host = "api.dataplatform.knmi.nl")
   # make sure local env is used by keyring so that api key can be set
-  withr::local_options(list("keyring_backend" = "env"))
+  withr::local_options(list(
+    "keyring_backend" = "env",
+    # use random binary so checking for converter step is passed
+    "getRad.nl_converter" = "ls"
+  ))
   # get public key here https://developer.dataplatform.knmi.nl/open-data-api#token
   withr::local_envvar(list("getRad_nl_api_key" = "wrongkey"))
   expect_error(