-
Notifications
You must be signed in to change notification settings - Fork 84
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
rsconnect::writeManifest
for Quarto content
#572
Conversation
rsconnect::writeManifest
for Quarto contentrsconnect::writeManifest
for Quarto content
583fdd7
to
052e689
Compare
Quarto inspection - now is gated by passing in a full Quarto path - should also work for single quarto documents
# Conflicts: # R/deployApp.R
- move duplicated complex functions to an inferQuartoInfo function - “quarto” used to mean either the path to the executable or the ready-for-manifest list. Now, the list is referred to as “quartoInfo”, matching “pyInfo”. Lookup doesn’t happen in the same place as “pyInfo”, but naming is more consistent.
rsconnect::writeManifest
for Quarto contentrsconnect::writeManifest
for Quarto content
CI should be fixed now. The check on Travis turned up an "unstated dependency" on
Weird. |
In its current state, this gives all Quarto content the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we have incoming Quarto metadata, let's avoid calling inspect.
R/bundle.R
Outdated
|
||
# Extract the Quarto version and engines, either from a parsed "quarto inspect" | ||
# result or from metadata provided by the quarto-r package. | ||
getQuartoManifestDetails <- function(inspect = list(), metadata = list()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I would prefer if this were split into two "constructor" functions, rather than one function that consumed the inputs differently depending upon what is present. Frankly, it might be simpler if the behavior were just inlined into inferQuartoInfo, but I don't feel strongly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. I'll inline it. I think that having two separate list()
statements in two separate functions is a bit too much indirection.
If you don't have For # If quarto::quarto_path() doesn't find Quarto, you need to use the IDE quarto
QUARTO_PATH <- "/Applications/RStudio.app/Contents/MacOS/quarto/bin/quarto"
# Quarto content
rsconnect::writeManifest("quarto-book-none", quarto = QUARTO_PATH)
rsconnect::writeManifest("quarto-doc-none", quarto = QUARTO_PATH)
rsconnect::writeManifest("quarto-multidoc-proj-none", quarto = QUARTO_PATH)
rsconnect::writeManifest("quarto-proj-none", quarto = QUARTO_PATH)
rsconnect::writeManifest("quarto-proj-none-ojs", quarto = QUARTO_PATH)
rsconnect::writeManifest("quarto-proj-py", quarto = QUARTO_PATH)
rsconnect::writeManifest("quarto-proj-r", quarto = QUARTO_PATH)
rsconnect::writeManifest("quarto-proj-r-py", quarto = QUARTO_PATH)
rsconnect::writeManifest("quarto-proj-r-shiny", quarto = QUARTO_PATH)
rsconnect::writeManifest("quarto-website-none", quarto = QUARTO_PATH)
rsconnect::writeManifest("quarto-website-py", quarto = QUARTO_PATH)
rsconnect::writeManifest("quarto-website-r", quarto = QUARTO_PATH)
rsconnect::writeManifest("quarto-website-r-py", quarto = QUARTO_PATH)
rsconnect::writeManifest("quarto-website-r-py-separate-files", quarto = QUARTO_PATH)
# Non-quarto content
rsconnect::writeManifest("rmd-shiny-runtime-shinyrmd-complete")
rsconnect::writeManifest("rmd-shiny-runtime-shinyrmd-simple")
rsconnect::writeManifest("rmd-site")
rsconnect::writeManifest("shinyapp") For # Assuming you have a server named "localhost" configured in the IDE / in the rsconnect package
server <- "localhost"
# If quarto::quarto_path() doesn't find Quarto, you need to use the IDE quarto
QUARTO_PATH <- "/Applications/RStudio.app/Contents/MacOS/quarto/bin/quarto"
# Quarto content
rsconnect::deployApp("quarto-book-none", quarto = QUARTO_PATH, server = server)
rsconnect::deployApp("quarto-doc-none", quarto = QUARTO_PATH, server = server)
rsconnect::deployApp("quarto-multidoc-proj-none", quarto = QUARTO_PATH, server = server)
rsconnect::deployApp("quarto-proj-none", quarto = QUARTO_PATH, server = server)
rsconnect::deployApp("quarto-proj-none-ojs", quarto = QUARTO_PATH, server = server)
rsconnect::deployApp("quarto-proj-py", quarto = QUARTO_PATH, server = server)
rsconnect::deployApp("quarto-proj-r", quarto = QUARTO_PATH, server = server)
rsconnect::deployApp("quarto-proj-r-py", quarto = QUARTO_PATH, server = server)
rsconnect::deployApp("quarto-proj-r-shiny", quarto = QUARTO_PATH, server = server)
rsconnect::deployApp("quarto-website-none", quarto = QUARTO_PATH, server = server)
rsconnect::deployApp("quarto-website-py", quarto = QUARTO_PATH, server = server)
rsconnect::deployApp("quarto-website-r", quarto = QUARTO_PATH, server = server)
rsconnect::deployApp("quarto-website-r-py", quarto = QUARTO_PATH, server = server)
rsconnect::deployApp("quarto-website-r-py-separate-files", quarto = QUARTO_PATH, server = server)
# Non-quarto content
rsconnect::deployApp("rmd-shiny-runtime-shinyrmd-complete", server = server)
rsconnect::deployApp("rmd-shiny-runtime-shinyrmd-simple", server = server)
rsconnect::deployApp("rmd-site", server = server)
rsconnect::deployApp("shinyapp", server = server) |
- Added test for null Quarto on content that Quarto can’t render
This draft PR adds Quarto support to
rsconnect::writeManifest
. I think that some of the compatibility affordances should also be added torsconnect::deployApp
, for consistency's sake.Intent
Add support for Quarto content to
rsconnect::writeManifest()
and expands support inrsconnect::deployApp()
. It adds support for Quarto documents with the following runtimes:It also adds partial support for Python-only Quarto projects, using the Jupyter engine. However, these manifests contain
packages
objects listing R packages, and they should not.Approach
To avoid adding a dependency on the Quarto package, we call
quarto inspect
ourselves in a function namedquartoInspect()
. We only attempt this if the user supplies aquarto
parameter containing the full path to a Quarto executable.rsconnect::deployApp()
continues to support deployment viaquarto::quarto_publish_app()
viaquarto_version
andquarto_engines
fields onmetadata
objects. This information is only used if thequarto
path is not supplied.Additions and changes to internal functions
inferQuartoInfo(appDir, appPrimaryDoc, quarto, metadata = NULL)
quartoInspect()
.NULL
andmetadata
was passed, attempts to find Quarto info in metadata.metadata
defaults to NULL becausemetadata
doesn't exist in some locations this function is called; it exists forquarto-r
compatibility indeployApp()
.getQuartoManifestDetails(inspect = list(), metadata = list())
inspect
ormetadata
, will return Quarto info ready for insertion into manifest.quartoInspect(appDir = NULL, appPrimaryDoc = null, quarto = NULL)
quarto
is notNULL
, attempts to callquarto inspect
first onappDir
, and if that fails (which it will for single documents), onappDir/appPrimaryDoc
.inspect
results orNULL
.Changes to main deployment functions
writeManifest()
quarto
parameter.inferQuartoInfo()
early on. Results are assigned toquartoInfo
object, followingpyInfo
. This is passed into otherinfer
functions, and then tocreateAppManifest
.contentCategory
is provided, andquartoInfo
is notNULL
, setscontentCategory
to"site"
, following current treatment of Quarto content.deployApp()
quarto
parameter.quarto
parameter, along withmetadata
, tobundleApp()
bundleApp()
quarto
andmetadata
parameters.inferQuartoInfo()
etc. exactly likewriteManifest()
.Regarding the
deployApp()
=>bundleApp()
call chain, I decided to have theinferQuartoInfo()
call occur withinbundleApp()
because in many ways, that function is analogous towriteManifest()
, and I thought that it would be much easier to maintain the shared parts of their code if replicated functionality looked the same, between them, rather than being spread betweendeployApp()
andbundleApp()
. But becausedeployApp()
needs to support determining Quarto info frommetadata
,bundleApp()
had to gain it as an optional argument.In
bundle.R
, the variable namequarto
was used interchangeably to refer to a Quarto path or to the extracted Quarto info. It now uses the namequartoInfo
anywhere that the object must be thequartoInfo
list, and usesquarto
anywhere it must be a path, and left that name in places where the code doesn't care (e.g.inferAppMode()
just checks that the object is notNULL
).Documentation
quarto
parameter.on.failure
parameter indeployApp
to match its position in the function documentation.Testing & QA
quartoInspect()
,inferQuartoInfo()
, andwriteManifest()
on Quarto content.writeManifest()
tests, using cleaned up items fromconnect-content
:quarto-doc-none
,quarto-proj-r-shiny
,quarto-website-py
,quarto-website-r-py
,quarto-website-r
.The most direct way to validate these changes for a piece of content is to run
writeManifest()
on a piece of content and examine the generated manifest. The changes in this PR only affect the output when the full path to a Quarto binary is passed to thequarto
parameter (which doesn't exist in the version ofrsconnect
on main). The easiest way to pass that binary requires you to have thequarto
R package installed. You'll also want the latestrmarkdown
andshiny
installed. I don't think there will be any other dependencies — I've omitted the extra-dependency packages from the list below.If you see any other dependency errors from the content, you should just "install.packages()" them.
Inspecting
writeManifest
changesIf you work within the
connect-content
repo, you can take advantage of git to directly diff the changes. The manifests of Quarto packages inconnect-content
were generated with the IDE, so they aren't a direct comparison torsconnect
main
.I've found that the easiest way to make direct comparisons is to work on a new local branch of
connect-content
Then, you can:main
version ofrsconnect
. Check out themain
branch; open thersconnect.Rroj
project in the IDE; click "Install" under the "Build" tab in the top-right pane.connect-content
repo, runrsconnect::writeManifest()
on all the content you want to check. Commit those changes to your local branch.For example, running R in the
bundles
directory ofconnect-content
:Run this command again for the
appDir
of each content bundle you want to examine. Commit all of the changes.rsconnect
. Check out themain
branch; open thersconnect.Rroj
project in the IDE; click "Install" under the "Build" tab in the top-right pane. After you've reinstalled the dev version, you should quit and reopen all your R sessions.connect-content
repo, in your new R session, runrsconnect::writeManifest()
on all the content you want to check. Don't commit the changes.To examine the Quarto-mode changes, you'd run:
You could also run without the Quarto argument to ensure that nothing has changed.
git status
andgit diff
to see what files have changed between the versions.If you run R from the
bundles
directory, you should be able to just runrsconnect
for all the bundles you want to check. I've included code snippets below.What To Look For
Compared to the
main
version, you should see the following changes in thewriteManifest
tests:quarto
argument:rmd-static
as their appmode should getquarto-static
as their appmode.rmd-shiny
as their appmode should getquarto-shiny
as their appmode.quarto
object in withversion
andengines
.quarto
argument, no changes should occur.For the
main
version, run:For this branch, run:
Testing `deployApp()
The same code changes are used in
deployApp
. You can run the following code snippet to deploy the listed content to Connect.Set your working directory to
connect-content/bundles
, and open a fresh R session. Make sure the dev branch ofrsconnect
is installed to your R library.Quarto content should show "Quarto *" under "Content Type".
Other things to test
deployApp()
also uses the changed workflow. Deploying Quarto and non-quarto content should work the same, e.g.deployApp(appDir = "quarto-proj-r-shiny", quarto = quarto::quarto_path()
.quarto::quarto_publish_app()
calls deployApp to deploy Quarto content. Instead of passing in aquarto
path, it passes in ametadata
object. To test that this method of deployment still works, you can usequarto_publish_app()
to publish some Quarto content (ensuring that it runs correctly on the server) or pass in a metadata object yourself.This code assumes that you have set up a local Connect dev instance as the server
localhost
in R. You can do this in the RStudio IDE, under the "Publishing" section in preferences.Appendix: Performance experiments
One of the reasons we gated
quarto inspect
behind thequarto
parameter is its performance impact: about little over half a second on average.plumber
bundle, running vanillarsconnect::writeManifest
after initial package load takes about 0.45 seconds. Withquarto inspect
it takes about 1.05 seconds. Runningquarto inspect
at the command line takes about 0.58 seconds, so this tracks.quarto-website-r-py
bundle, runningrsconnect::writeManifest
with a metadata option takes about 1.55 seconds (skips thequarto inspect
codepath).rsconnect::writeManifest
withquarto inspect
takes about 2.1 seconds. Again, a penalty of about 0.6 seconds, which corresponds to how longquarto inspect
takes to run.