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

Support appId when publishing to Posit Cloud #819

Closed
mbaynton opened this issue Apr 20, 2023 · 4 comments · Fixed by #829
Closed

Support appId when publishing to Posit Cloud #819

mbaynton opened this issue Apr 20, 2023 · 4 comments · Fixed by #829

Comments

@mbaynton
Copy link
Contributor

deployApp() includes the parameter appId whose intended usage is to allow publishing to a specific existing output on the server. This functionality needs to be given some love when the destination is Posit Cloud:

  • Posit Cloud has a notion of an "application", but its ID is intentionally difficult for a user to track down because we prefer instead to
    expose related construct "content" to users. Work happening right now to support a history of prior revisions to some types of published content in cloud also makes the relationship between a Cloud "content" and a Cloud "application" not one-to-one -- each time you publish a new revision, you use the same content ID but get a new application ID. For all these reasons, to allow the user to publish to an existing output in Cloud without a .dcf deployment history, they should be providing the content ID, not the application ID.
  • As a result of the above, in the case where a deployment history is available, we're already written rsconnect code that switches to persisting the content ID rather than the application ID. I suggest this work merges before tackling this issue, and then we fix remaining problem(s) with manually passing a Cloud content ID:
  1. Manually passing a content ID (e.g. deployApp(appId='lucid:content:123', ...)) fails with
    Error in (function (..., row.names = NULL, check.rows = FALSE, check.names = TRUE,  : 
    arguments imply differing number of rows: 1, 0
    
  2. Perhaps we should always consider any value passed to appId to be a content ID if the server is posit.cloud, even if not made explicit with the lucid:content:?
@kippandrew
Copy link
Contributor

I'd like to propose a third option that is similar to open 2:

  1. Consider deprecating the appId parameter in favor of a new contentId parameter. I think this would work well for Posit Connect and Posit Cloud, both of which have the concept of contentId though there are differences (see my comment here: deploymentTarget doesn't work as intended with Posit Cloud #808 (comment)). However, shinyapps.io doesn't really have the same concept, but maybe it would be acceptable to have contentId mean appId when used with shinyapps.io.

@mslynch
Copy link
Contributor

mslynch commented Apr 27, 2023

We decided on passing the unqualified content id in appId (deployApp(appId='123', ...)) – this will work when we implement static publishing. We'll also ensure backwards compatibility with old dcf files using application id through #821.

@mslynch mslynch closed this as completed Apr 27, 2023
@mslynch mslynch closed this as not planned Won't fix, can't repro, duplicate, stale Apr 27, 2023
@mslynch
Copy link
Contributor

mslynch commented May 2, 2023

Reopening – the error arguments imply differing number of rows: 1, 0 is a regression on main, unrelated to any content id/app id changes. cc @hadley

@mslynch mslynch reopened this May 2, 2023
@hadley
Copy link
Member

hadley commented May 2, 2023

I'll take a look this afternoon.

hadley added a commit that referenced this issue May 2, 2023
Need to set `envVars` argument and cloud doesn't supply `owner_username`. This value is only used for display, and AFAIK is not important, so I just defaulted to the account user name.

Fixes #819
hadley added a commit that referenced this issue May 2, 2023
Need to set `envVars` argument and cloud doesn't supply `owner_username`. This value is only used for display, and AFAIK is not important, so I just defaulted to the account user name.

Fixes #819
hadley added a commit that referenced this issue May 2, 2023
Need to set `envVars` argument and cloud doesn't supply `owner_username`. This value is only used for display, and AFAIK is not important, so I just defaulted to the account user name.

Fixes #819
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants