-
Notifications
You must be signed in to change notification settings - Fork 83
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
Deployment fails because bundle is wrongly identified as API by inferAppMode()
#942
Comments
inferAppMode()
I think we need two fixes:
|
We had intended to use top-level files during app-mode inference, but used the wrong set of files: Lines 37 to 42 in 6ae93b2
|
Just to chime in - even without recursive searching to infer If there is someone (other than me :) ) that can take a look at this, it would be immensely appreciated... |
I've created #948 to track the app-mode override idea. |
Context
We are using a golem-style app-layout and an additional plumber API in
inst/plumber/api/plumber.R
, i.e.Undesired Behavior
rsconnect:::inferAppMode()
infers that the project hasappmode: api
, notappmode: shiny
. This function is called when usingrsconnect::writeManifest()
,rsconnect::bundleApp()
(which is called byrsconnect::deployApp()
), as well as when using the deploy button in the RStudio-Interface. Thus,rsconnect::deployApp()
and the deployButton will wrongly try to deploy the project as an API instead of as a shiny app.For example:
Creating a
manifest.json
in the directory and simply changingappmode
toshiny
in that file doesn't solve the issue, becausedeployApp()
callsbundleApp()
, which generates its ownmanifest.json
straight in a temporary bundle directory, so themanifest.json
file in the project directory is ignored.The root of the Problem
rsconnect:::inferAppMode()
checks for evidence of a plumber API (and returns) before it checks for evidence of a Shiny App (as can be seen here).The behavior was introduced in rsconnect version 1.0.0, because
inferAppMode()
was called with a non-recursive list of the files in the directory, which was obtained viabundleFiles()
. Thus, only the top level was searched for evidence of the different app modes andinst/api/plumber.R
would not have been part of the list.listDeploymentFiles()
. Thus, in our case, the list now includes the api file in one of the subdirectories 'inst/api/plumber.R`listBundleFIles()
, which now calls a function that returns a recursive list of files (old version in 0.8.29 vs. new version in 1.0.0)Workaround: setting
appPrimaryDoc = "app.R"
In the community forums, someone suggested to use the argument
appPrimaryDoc = "app.R"
.Unfortunately, this throws an error:
This error is thrown by a conditional in
rsconnect::checkAppLayout(appDir, appPrimaryDoc)
(here), a function that checks whether the app directory has any of the expected layouts. This function is called by both,writeManifest()
andbundleApp()
viarsconnect:::appMetadata()
and it explicitly prevents the user from settingappPrimaryDoc
to beapp.R
:A solution is to add an additional condition to test, whether
appPrimaryDoc == app.R
. Ideally, this would have to be fixed in{rsconnect}
itself, but one can temporarily inject a fix like this:(see below for full functional code)
This allows me to publish to rsconnect succesfully, since the error isn't thrown anymore and the generated
manifest.json
now hasappmode: shiny
. However, the solution requires me to modify a function in thersconnect
namespace and it still doesn't allow me to use the deploy Button in RStudio, because there I cannot supply theappPrimaryDoc
argument. (Also, this solution won't work if I want to setappPrimaryDoc = "plumber.R"
to publish the API.)It looks like currently there is no straightforward solution to use the project structure we have with the latest version of rsconnect.
Conclusion
Generally, I think, the preference of an API over an App by
deployApp()
is unexpected and should be removed. If I wanted to deploy the API that's in the package, I'd expect to usedeployApi()
to do so.Similarly, when clicking the deploy Button in
app.R
I'd expect it to deploy the app that's in that file, whereas clicking the deploy button inplumber.R
should deploy the API.It might be a solution to revert to the old behavior of searching only the root directory to infer the app mode. Another option would be to use the user-supplied "manifest.json" in
deployApp()
.Complete functional code of my 'workaround'
The text was updated successfully, but these errors were encountered: