generate title and name; name is normalized #1023

aronatkins · 2023-10-27T16:14:02Z

When deploying from a directory named "Incredible Shiny Application", the generated name for Posit Connect is Incredible_Shiny_Application and for shinyapps.io is incredible_shiny_application. Both services require a-zA-Z0-9-_ for names, but we lower-case only for shinyapps.io.

~~The name is derived from an incoming title, when available, and otherwise from the content path.~~

~~When deploying without a title, Connect uses the incoming name to seed the title. This behavior is not changing.~~

When deploying from a directory named "Incredible Shiny Application", a generated title mirrors the directory name and name is generated from the title and normalized like incredible_shiny_application.

A generated name is used only when one is not already provided.

Title changes on the server are now reflected in the deployment record.

fixes #1022
related to #1008

fixes #1022

aronatkins · 2023-10-27T16:23:25Z

CI failures due to r-project.org DNS trouble. Will re-run once that clears.

hadley

Should we do something to generateAppName() to make it less likely to make this mistake again?

aronatkins · 2023-10-27T17:59:37Z

Should we do something to generateAppName() to make it less likely to make this mistake again?

I considered modifying generateAppName() but did not want to change its implementation in a breaking fashion, since it is a public function. We disconnected generateAppName() during the build-up to 1.0.0 but didn't mark it as deprecated at that point in time.

Open to suggestions.

hadley · 2023-10-27T22:03:57Z

Maybe we should deprecate it. It is public, but I doubt many people are using it. There's no evidence of another CRAN package using it: https://github.com/search?q=org%3Acran%20generateAppName&type=code. But maybe it's used by the IDE?

hadley · 2023-10-27T22:04:32Z

Gah, yes: https://github.com/rstudio/rstudio/blob/ad0e5854c7a0abed5728c476ec3611f458e52d82/src/cpp/session/modules/SessionRSConnect.R#L592-L594

aronatkins · 2023-10-30T14:14:48Z

The IDE calls generateAppName() to create a name from an incoming title; that makes me want to shift back to using generateAppName() but remove its lower-casing... We could even pass in an (optional) incoming title and act more like the IDE when deployApp() is directly called.

aronatkins · 2023-10-30T17:30:49Z

@hadley Given #1022 (comment), how do we want deployApp() to behave?

generate lower-cased names, which leads to lower-case default titles. This is the behavior of rsconnect 0.8.29 and the current development version of rsconnect. Relies on generateAppName(), mirroring the IDE.
generate mixed-case names, which leads to mixed-case default titles. This is the behavior of rsconnect 1.0.0 through the current CRAN version.
generate lower-cased names and mixed-case titles. This leaves names compatible with rsconnect 0.8.29 (and preserves any existing look-ups), while letting rich file names be directly used as the content title.

We cannot modify generateAppName() because its generated (lower-cased) names are used by the IDE to detect a duplicate deployment.

The best answer would be to use the incoming filename to generate a title alongside the name, which needs less normalization, and then normalize only the name. I considered this option but abandoned it because it felt like too much of a change for what felt like a bug fix.

I'm leaning towards having deployApp() explicitly infer the title.

hadley · 2023-11-01T12:21:01Z

Yeah, letting deployApp() explicitly infer the title sounds good to me.

aronatkins · 2023-11-01T12:46:02Z

Yeah, letting deployApp() explicitly infer the title sounds good to me.

OK. I'll take a stab at that and update this PR.

aronatkins · 2023-11-01T13:45:14Z

R/deploymentTarget.R

@@ -91,7 +91,13 @@ findDeploymentTarget <- function(
  # Otherwise, identify a target account (given just one available or prompted
  # by the user), generate a name, and locate the deployment.
  accountDetails <- findAccountInfo(account, server, error_call = error_call)
-  appName <- generateAppName(appTitle, recordPath, accountDetails$name, unique = FALSE)
+  appTitle <- generateTitle(recordPath = recordPath, title = appTitle)


This has the side-effect of generating an appTitle for shinyapps.io content, which does not support title. Alternatively, we could generate a title only for Connect deployments.

The other drawback of this approach is that as soon as we have generated the title, we lose track of the fact that the title was not user-specified. This means that the generated title will always overwrite the deployment record title.

We may want to distinguish between "user provided title" and "generated title" when updating a discovered deployment record.

title <- userTitle %||% deployment$title %||% generatedTitle

After more testing, we probably want to avoid generating titles for shinyapps.io.

The IDE deployment pane forces a normalized "title", which becomes the title and name:

After manually normalizing the title with "incredible-shiny-application" and completing this workflow, the deployment record name and title both receive the same value (as forced by the IDE).

Using the CRAN version of rsconnect::deployApp() leaves a shinyapps.io deployment record with title=NULL. The IDE deployment workflow creates deployment records with title and name both with the same value.

The IDE allows mixed-case names(titles) for shinyapps.io, but mixed-case is not produced by generateAppName().

aronatkins · 2023-11-01T13:45:49Z

R/deployments.R

@@ -98,7 +98,7 @@ saveDeployment <- function(recordDir,
                           addToHistory = TRUE) {
  deployment <- deploymentRecord(
    name = deployment$name,
-    title = deployment$title,
+    title = application$title %||% deployment$title,


This helps make sure that a server-updated title is reflected on the client.

We may want to take this change even if we decide to abandon the other name-generation aspects of this PR.

How will automatically generating a title interact with this? Will auto-generated titles overwrite titles that have been changed on Connect?

toph-allen

I don't feel like I have a completely clear picture of some of the effects of this change. Open questions for me are around how these changes interact with titles that have been updated on Connect.

Prior to this update, the default title was an empty string. Now, if we aren't provided a title, we generate one, and use this to generate a name. Will this overwrite titles that have been changed on Connect?

How does this interact with deployment records? It looks like we get titles from deployment records (see here). Will this lead to a title that has been changed on Connect reverting to the previously-published title when it is updated? (Outside of the scope of these changes, but a question raised by them.)

The pre-1.0.0 behavior was to use a non-lowercased appName, right? Perhaps better to revert to that, and only use appTitle if provided.

toph-allen · 2023-11-08T20:37:07Z

R/deployments.R

@@ -98,7 +98,7 @@ saveDeployment <- function(recordDir,
                           addToHistory = TRUE) {
  deployment <- deploymentRecord(
    name = deployment$name,
-    title = deployment$title,
+    title = application$title %||% deployment$title,


How will automatically generating a title interact with this? Will auto-generated titles overwrite titles that have been changed on Connect?

normalize names for all servers, but lowercase for shinyapps.io

48c7970

fixes #1022

aronatkins requested review from hadley and toph-allen October 27, 2023 16:14

hadley approved these changes Oct 27, 2023

View reviewed changes

aronatkins force-pushed the aron-issue-1022 branch from f4986a9 to 019920f Compare November 1, 2023 13:37

generate title from path; update title from server

e2236c2

aronatkins force-pushed the aron-issue-1022 branch from 019920f to e2236c2 Compare November 1, 2023 13:38

aronatkins commented Nov 1, 2023

View reviewed changes

aronatkins changed the title ~~normalize names for all servers, but lowercase for shinyapps.io~~ generate title and name; name is normalized Nov 1, 2023

toph-allen reviewed Nov 8, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

generate title and name; name is normalized #1023

generate title and name; name is normalized #1023

aronatkins commented Oct 27, 2023 •

edited

Loading

aronatkins commented Oct 27, 2023

hadley left a comment

aronatkins commented Oct 27, 2023

hadley commented Oct 27, 2023

hadley commented Oct 27, 2023

aronatkins commented Oct 30, 2023

aronatkins commented Oct 30, 2023

hadley commented Nov 1, 2023

aronatkins commented Nov 1, 2023 •

edited

Loading

aronatkins Nov 1, 2023

aronatkins Nov 1, 2023

aronatkins Nov 1, 2023

aronatkins Nov 1, 2023

aronatkins Nov 1, 2023

aronatkins Nov 1, 2023

toph-allen Nov 8, 2023

toph-allen left a comment

toph-allen Nov 8, 2023

generate title and name; name is normalized #1023

Are you sure you want to change the base?

generate title and name; name is normalized #1023

Conversation

aronatkins commented Oct 27, 2023 • edited Loading

aronatkins commented Oct 27, 2023

hadley left a comment

Choose a reason for hiding this comment

aronatkins commented Oct 27, 2023

hadley commented Oct 27, 2023

hadley commented Oct 27, 2023

aronatkins commented Oct 30, 2023

aronatkins commented Oct 30, 2023

hadley commented Nov 1, 2023

aronatkins commented Nov 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toph-allen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aronatkins commented Oct 27, 2023 •

edited

Loading

aronatkins commented Nov 1, 2023 •

edited

Loading