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.

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

Standardize single doc deployment #698

Merged
merged 12 commits into from
Feb 24, 2023
2 changes: 2 additions & 0 deletions R/appDependencies.R
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#' parses all .R files in the application directory to determine what packages
#' the application depends on; and for each of those packages what other
#' packages they depend on.
#'
#' @inheritParams writeManifest
#' @inheritParams deployApp
#' @return Returns a data frame listing the package
#' dependencies detected for the application: \tabular{ll}{ `package`
Expand Down
40 changes: 20 additions & 20 deletions R/deployApp.R
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@
#' control to ensure that future you and other collaborators will publish
#' to the same location.
#'
#' @param appDir Directory containing application. Defaults to current working
#' directory.
#' @param appDir Either a directory containing an application (e.g. a Shiny app
#' or plumber API) or a path to a document, like an `.html`, `.Rmd`, or
#' `.Qmd`. Defaults to the working directory.
#' @param appFiles A character vector given relative paths to the files and
#' directories to bundle and deploy. The default, `NULL`, will include all
#' files in `appDir`, apart from any listed in an `.rscignore` file.
Expand Down Expand Up @@ -154,7 +155,23 @@ deployApp <- function(appDir = getwd(),

condaMode <- FALSE

check_directory(appDir)
check_string(appDir)
if (dirExists(appDir)) {
# ok
} else if (file.exists(appDir) && isStaticFile(appDir)) {
doc <- standardizeSingleDocDeployment(appDir)
Copy link
Member Author

Choose a reason for hiding this comment

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

One thing that's slightly unappealing about this approach is that we use a different approach to determining appFiles depending on whether you do deployApp("foo.Rmd") or deployApp(appPrimaryDoc = "foo.Rmd"). An alternative approach would be to add an appPrimaryDoc argument to appStandardizeFiles() and call rmarkdown::find_external_dependencies() there when appPrimaryDoc is supplied.

Copy link
Member Author

Choose a reason for hiding this comment

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

Another approach, since this never appears to have been documented, would be to deprecate this use appDir and tell folks to call deployDoc() instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting. I forgot about this difference.

# Uses the default appDir=getwd() and enumerates all files in the directory for bundling
deployApp(appPrimaryDoc = "foo.Rmd")
# Uses the foo.Rmd to identify the dependencies of the document; only those are bundled
deployDoc("foo.Rmd")

The existing deployApp("foo.Rmd") feels like a convenience, but we should nudge folks towards using deployDoc should they provide a file.

Added in: #16, and before the notion of a "primary doc".

I'm uneasy about the automatic use of rmarkdown::find_external_dependencies. On the one hand, it does feel like it will produce the set of files that most folks want to include -- directory explosion is often too greedy. On the other hand, its use would add to the "magic" that happens in the "lower-level" deployApp. Its inclusion could suddenly force more folks to compute their own appFiles.

Copy link
Contributor

Choose a reason for hiding this comment

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

Deprecating the deployApp -> deployDoc -> deployApp workflow feels like the right path.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I think the fundamentally different mechanism of file discovery suggests that this is a deployDoc() feature, and we should deprecate this old path.


appDir <- doc$appDir
appPrimaryDoc <- doc$appPrimaryDoc
if (is.null(appFiles) && is.null(appFileManifest)) {
appFiles <- doc$appFiles
}
} else {
cli::cli_abort(
"{.arg appDir} must be a directory or a .Rmd, .Qmd, or .html file"
)
}

check_string(appName, allow_null = TRUE)

# set up logging helpers
Expand Down Expand Up @@ -207,23 +224,6 @@ deployApp <- function(appDir = getwd(),
}
}

# if a specific file is named, make sure it's an Rmd or HTML, and just deploy
# a single document in this case (this will call back to deployApp with a list
# of supporting documents)
rmdFile <- ""
if (!dirExists(appDir)) {
if (grepl("\\.[Rq]md$", appDir, ignore.case = TRUE) ||
grepl("\\.html?$", appDir, ignore.case = TRUE)) {
return(deployDoc(appDir, appName = appName, appTitle = appTitle,
account = account, server = server, upload = upload,
recordDir = recordDir, launch.browser = launch.browser,
logLevel = logLevel, lint = lint))
} else {
stop(appDir, " must be a directory, an R Markdown document, or an HTML ",
"document.")
}
}

# directory for recording deployment
if (is.null(recordDir)) {
recordDir <- appPath
Expand Down
61 changes: 36 additions & 25 deletions R/deployDoc.R
Original file line number Diff line number Diff line change
Expand Up @@ -23,39 +23,50 @@
#' @family Deployment functions
#' @export
deployDoc <- function(doc, ...) {
doc <- standardizeSingleDocDeployment(doc)
deployApp(
appDir = doc$appDir,
appPrimaryDoc = doc$appPrimaryDoc,
appFiles = doc$appFiles,
...
)
}

standardizeSingleDocDeployment <- function(path,
quiet = FALSE,
error_call = caller_env(),
error_arg = caller_arg(path)) {
check_installed(
"rmarkdown",
version = "0.5.2",
reason = "to deploy individual R Markdown documents"
)
check_file(path, error_call = error_call, error_arg = error_arg)

check_file(doc)

# get qualified doc
qualified_doc <- normalizePath(doc, winslash = "/")
withStatus <- withStatus(quiet)

# see if this doc has runtime: shiny_prerendered, if it does then
# appFiles will be NULL (bundle the entire directory)
if (isShinyRmd(doc)) {
app_files <- NULL
if (isShinyRmd(path)) {
# deploy entire directory
appFiles <- NULL
} else if (isStaticFile(path)) {
# deploy file + dependenciy
withStatus("Discovering document dependencies", {
resources <- rmarkdown::find_external_resources(path)
})
appFiles <- c(basename(path), resources$path)
} else {
# default to deploying just the single file specified
app_files <- basename(qualified_doc)

# if this document's type supports automated resource discovery, do that now,
# and add the discovered files to the deployment list
ext <- tolower(tools::file_ext(doc))
if (ext %in% c("rmd", "qmd", "html", "htm")) {
message("Discovering document dependencies... ", appendLF = FALSE)
res <- rmarkdown::find_external_resources(qualified_doc)
message("OK")
app_files <- c(app_files, res$path)
}
# deploy just the file
appFiles <- basename(path)
}

# deploy the document with the discovered dependencies
deployApp(appDir = dirname(qualified_doc),
appFiles = app_files,
appPrimaryDoc = basename(qualified_doc),
...)
list(
appDir = dirname(path),
appPrimaryDoc = basename(path),
appFiles = appFiles
)
}

isStaticFile <- function(path) {
ext <- tolower(tools::file_ext(path))
ext %in% c("rmd", "qmd", "html", "htm")
}
1 change: 1 addition & 0 deletions R/usage.R
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ showUsage <- function(appDir = getwd(), appName = NULL, account = NULL, server =
#' Show application metrics of a currently deployed application.
#' This function only works for ShinyApps servers.
#'
#' @inheritParams writeManifest
#' @param metricSeries Metric series to query. Refer to the
#' [shinyapps.io documentation](<https://docs.posit.co/shinyapps.io/metrics.html#ApplicationMetrics>)
#' for available series.
Expand Down
2 changes: 2 additions & 0 deletions R/writeManifest.R
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#' that directory describing the deployment requirements for that content.
#'
#' @inheritParams deployApp
#' @param appDir Path to an application directory. Defaults to the
#' working directory.
#' @param verbose If TRUE, prints progress messages to the console
#' @export
writeManifest <- function(appDir = getwd(),
Expand Down
4 changes: 2 additions & 2 deletions man/appDependencies.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions man/deployApp.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions man/showMetrics.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions man/writeManifest.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions tests/testthat/_snaps/deployDoc.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# deployDoc correctly reports bad path

Code
deployDoc("doesntexist.Rmd")
Condition
Error in `deployDoc()`:
! `doc`, "doesntexist.Rmd", does not exist.

43 changes: 43 additions & 0 deletions tests/testthat/test-deployDoc.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
test_that("deployDoc correctly reports bad path", {
expect_snapshot(deployDoc("doesntexist.Rmd"), error = TRUE)
})

# standardizeSingleDocDeployment ------------------------------------------

test_that("turns appDir into appDir + appPrimarySourceDoc", {
dir <- local_temp_app(list("foo.R" = ""))

doc <- standardizeSingleDocDeployment(file.path(dir, "foo.R"))
expect_equal(doc$appDir, dir)
expect_equal(doc$appPrimaryDoc, "foo.R")
})

test_that("shiny rmd deploys whole directory", {
dir <- local_temp_app(list("foo.Rmd" = c(
"---",
"runtime: shiny",
"---"
)))
doc <- standardizeSingleDocDeployment(file.path(dir, "foo.Rmd"))
expect_equal(doc$appFiles, NULL)
})

test_that("regular rmd deploys file and dependencies", {
dir <- local_temp_app(list(
"foo.Rmd" = c(
"---",
"resource_files: [foo.csv]",
"---"
),
"foo.csv" = ""
))

doc <- standardizeSingleDocDeployment(file.path(dir, "foo.Rmd"), quiet = TRUE)
expect_equal(doc$appFiles, c("foo.Rmd", "foo.csv"))
})

test_that("other types deploy that one file", {
dir <- local_temp_app(list("foo.R" = ""))
doc <- standardizeSingleDocDeployment(file.path(dir, "foo.R"))
expect_equal(doc$appFiles, "foo.R")
})