ThinkR-open / dockerfiler

Easy Dockerfile Creation from R
https://thinkr-open.github.io/dockerfiler/
Other
169 stars 25 forks source link

[BUG] `golem::add_dockerfile_shinyproxy` adds `"` around `NA` #61

Closed danielquintao closed 9 months ago

danielquintao commented 9 months ago

Description It seems that the Dockerfile generated by the function golem::add_dockerfile_shinyproxy inserts quotes around NA in lines RUN Rscript -e 'remotes::install_version(..., version="NA")'. Replacing "NA" by NA seems to fix the Dockerfile and allow for proper building.

To Reproduce Steps to reproduce the behavior:

  1. Installed version of golem: 0.4.1
  2. Clone or download ColinFay's tidytuesday201942 example
  3. Use golem::add_dockerfile_shinyproxy(from="rocker/shiny-verse:4.3.1")
  4. Try docker built -t tidytuesday . and see error ERROR [12/26] RUN Rscript -e 'remotes::install_version("pkgload",upgrade="never", version = "NA")'
  5. replace all thge "NA" in the Dockerfile by NA and save the file
  6. docker build -t tidytuesday . again; it should work now

Expected behavior Building the Docker image should have worked without modifying the Dockerfile (although this is a quite simple change, it would be nice to have things as automatic as possible).

Additional context

VincentGuyader commented 9 months ago

thanks,

it's quite strange that version="NA" is returned... I don't use this function anymore, I think golem::add_dockerfile_with_renv_shinyproxy() is way more robust way to do

VincentGuyader commented 9 months ago

issue with :


Browse[6]> remotes::package_deps(packages)
Needs update -----------------------------
 package      installed available is_cran remote
 utf8         1.2.3     1.2.4     TRUE    CRAN  
 vctrs        0.6.1     0.6.4     TRUE    CRAN  
 rlang        1.1.0     1.1.1     TRUE    CRAN  
 fansi        1.0.4     1.0.5     TRUE    CRAN  
 processx     3.8.1     3.8.2     TRUE    CRAN  
 prettyunits  1.1.1     1.2.0     TRUE    CRAN  
 withr        2.5.0     2.5.1     TRUE    CRAN  
 pkgbuild     1.4.0     1.4.2     TRUE    CRAN  
 fs           1.5.2     1.6.3     TRUE    CRAN  
 digest       0.6.31    0.6.33    TRUE    CRAN  
 sass         0.4.5     0.4.7     TRUE    CRAN  
 jsonlite     1.8.4     1.8.7     TRUE    CRAN  
 htmltools    0.5.5     0.5.6.1   TRUE    CRAN  
 cachem       1.0.7     1.0.8     TRUE    CRAN  
 Rcpp         1.0.10    1.0.11    TRUE    CRAN  
 later        1.3.0     1.3.1     TRUE    CRAN  
 promises     1.2.0.1   1.2.1     TRUE    CRAN  
 bslib        0.4.2     0.5.1     TRUE    CRAN  
 fontawesome  0.5.1     0.5.2     TRUE    CRAN  
 httpuv       1.6.9     1.6.12    TRUE    CRAN  
 viridisLite  0.4.1     0.4.2     TRUE    CRAN  
 labeling     0.4.2     0.4.3     TRUE    CRAN  
 waldo        0.4.0     0.5.1     TRUE    CRAN  
 pkgload      1.3.2     1.3.3     TRUE    CRAN  
 evaluate     0.20      0.22      TRUE    CRAN  
 xfun         0.39      0.40      TRUE    CRAN  
 purrr        1.0.1     1.0.2     TRUE    CRAN  
 shiny        1.7.4     1.7.5.1   TRUE    CRAN  
 config       0.3.1     0.3.2     TRUE    CRAN  
 gtable       0.3.3     0.3.4     TRUE    CRAN  
 testthat     3.1.7     3.2.0     TRUE    CRAN  
 markdown     1.6       1.11      TRUE    CRAN  
 styler       1.9.1     1.10.2    TRUE    CRAN  
 ggplot2      3.4.2     3.4.4     TRUE    CRAN  
 dplyr        1.1.2     1.1.3     TRUE    CRAN  
 colourvalues NA        0.3.9     TRUE    CRAN  
danielquintao commented 9 months ago

thanks,

it's quite strange that version="NA" is returned... I don't use this function anymore, I think golem::add_dockerfile_with_renv_shinyproxy() is way more robust way to do

Thank you for your fast responsiveness. Indeed, we have an alert golem::add_dockerfile_shinyproxy() is not recommended anymore. Please use golem::add_dockerfile_with_renv_shinyproxy() instead.... I was using the old function because I wanted to have something that works for no-renv golem packages and at that time I didn't no that golem::add_dockerfile_with_renv_shinyproxy() does that too (although the deprecation warning is indeed very suggestive...). Since you say it is more robust, I will consider using that one.

VincentGuyader commented 9 months ago
  if (length(packages_on_cran > 0)) {
    ping <- mapply(function(dock, ver, nm) {
      res <- dock$RUN(sprintf("Rscript -e 'remotes::install_version(\"%s\",upgrade=\"never\", version = \"%s\")'", 
        nm, ver))
    }, ver = packages_on_cran, nm = names(packages_on_cran), 
      MoreArgs = list(dock = dock))
  }

if ver is a NA it's create "NA"

VincentGuyader commented 9 months ago

you can try with the new version on remotes::install_github("ThinkR-open/dockerfiler") ?

Regards

danielquintao commented 9 months ago

you can try with the new version on remotes::install_github("ThinkR-open/dockerfiler") ?

Regards

It worked fine! Thank you very much

Regards