Building in docker #40
Conversation
…. Need to make sure removing CMD works with Jenkins
… different docker engine versions
scripts/fetch/dv_data.R
Outdated
| @@ -12,7 +12,7 @@ fetch.dv_data <- function(viz){ | |||
| sites <- deps[["sites"]] | |||
|
|
|||
| startDate <- paste0(year, "-01-01") | |||
| endDate <- paste0(year, "-12-31") | |||
| endDate <- paste0(year, "-01-02") | |||
There was a problem hiding this comment.
Safe to revert this change now?
There was a problem hiding this comment.
Yes, although really I'd say these dates should all be moved into the viz.yaml so they can be set in a single place
There was a problem hiding this comment.
Fair. Up to you how you do it, but note that we're not currently sure that anybody will be developing this interactive viz any more this year, so code quality/maintainability matters somewhat less than usual.
There was a problem hiding this comment.
Right, probably wouldn't bother doing it now
scripts/fetch/dv_sites.R
Outdated
| @@ -12,7 +12,7 @@ fetch.dv_sites <- function(viz){ | |||
| year <- deps[["year"]][["year"]] | |||
|
|
|||
| startDate <- paste0(year, "-01-01") | |||
| endDate <- paste0(year, "-12-31") | |||
| endDate <- paste0(year, "-01-02") | |||
| @@ -18,7 +18,7 @@ info: | |||
| version: 0.9-7 | |||
| dataRetrieval: | |||
| repo: CRAN | |||
| version: 2.7.4 | |||
| version: 2.7.3 | |||
There was a problem hiding this comment.
does the older version work better?
There was a problem hiding this comment.
It was just to avoid having to set package installs to GRAN, since CRAN only has 2.7.3
| version: 1.1.6 | ||
| geojsonio: | ||
| repo: CRAN | ||
| version: 0.6.0 |
There was a problem hiding this comment.
It'd be nice to think about removing all of these declared package dependencies as we switch to Docker (because now the Dockerfile is where the decisions actually get made about which package version gets used). Is now too soon to do so, and if not, is it as simple as deleting all the lines in this subsection, or does vizlab give errors if you do that?
There was a problem hiding this comment.
We just would need to remove the checkVizPackages() call invizmake(), since that requires there to be a packages yaml block. Otherwise, I think we could probably just drop the packages section, unless we don't feel confident relying on the package versions that come through the rocker images.
There is probably a fair bit of code associated with checkVizPackages that could either be removed from vizlab, unless we wanted to preserve a way of checking package versions against some reference.
There was a problem hiding this comment.
OK, thanks for thinking it through. I'll create a GitHub issue in vizlab for this, and we can resolve it (including cleaning up the extra code) after we've had a chance to kick the tires on the Docker option for a while.
What happens if you make a packages YAML block but leave it empty?
There was a problem hiding this comment.
Added this issue: USGS-VIZLAB/vizlab#395
There was a problem hiding this comment.
That just returns a null from checkVizPackages, I think that would work
This adds the docker source code, a few little fixes to make the build go faster, and the jenkinsfile