Make publish_book() publish to connect.posit.cloud by default #1512
Conversation
|
@cderv Do you mind taking a look at the gh-pages deployment? bookdown/.github/workflows/Book.yaml Lines 100 to 111 in 86e3631 I don't remember when it stopped working, but I'm hoping to pull from gh-pages on connect.posit.cloud so we can still enjoy the automatic publishing from CI. Thanks! |
|
We just added So it currently only work on manual trigger, and to build book with Pandoc devel bookdown/.github/workflows/Book.yaml Lines 22 to 27 in 13bcda7 We just need to make it the default way to build book. I'll make a PR. |
|
I can release the new version of bookdown to CRAN at any time. @cderv Is there anything holding the release at the moment? |
nealrichardson
left a comment
nealrichardson
left a comment
There was a problem hiding this comment.
A few observations, I'm not sure this is quite right but maybe I'm misreading
| # if there are no Connect accounts setup on this machine, offer to add one | ||
| # for connect.posit.cloud | ||
| if (!'connect.posit.cloud' %in% accounts$server) { |
There was a problem hiding this comment.
Unless I misunderstand, this will always prompt to create an account (until you have one), even if you've specified account to be something else?
There was a problem hiding this comment.
Good point. Amended via 425b8fb. Thanks!
There was a problem hiding this comment.
This modification means that if another server is configured (like internal posit server), then connect to connect cloud won't be done by publish_book()
A user will require to use rsconnect::ConnectCloudUser() explicitly in that case to add the server. Then use publish_book(server = "connect.posit.cloud")).
I'll add that in the migration guide.
| publish_book = function(name = NULL, account = NULL, server = "connect.posit.cloud", ...) { | ||
| # delete local records of bookdown.org | ||
| accounts = rsconnect::accounts() | ||
| x1 = 'bookdown.org' %in% accounts$server |
There was a problem hiding this comment.
I wonder if it would make sense to encapsulate the bookdown server/deployment forgetting into a separate function? A warning message here could alert you to call that function, and then you wouldn't need the interactive readline() prompting, you could just do it because the user called the function.
There was a problem hiding this comment.
The interactive prompting will occur only if bookdown.org was found in the accounts. Since we absolutely want to disable publishing to bookdown.org in the near future, I feel it's more appropriate to do it just here. BTW, for non-interactive R sessions, this will have no effect (readline() returns an empty string, making the condition FALSE).
|
|
||
| # configure the account | ||
| rsconnect::connectUser(server = 'bookdown.org') | ||
| publish_book = function(name = NULL, account = NULL, server = "connect.posit.cloud", ...) { |
There was a problem hiding this comment.
I'm not an expert on how rsconnect works, but IIUC, the old code was relying on the NULL behavior to pick an account/credentials in the case where there is only one server configured in your environment. By making server non-NULL here, we would break the behavior for anyone who is publishing bookdown to a Posit Connect server like that.
There was a problem hiding this comment.
Another good point. Amended via e11cbea.
…and bookdown.org have been set up (e.g., when another Connect server has been connected before, don't ask to connect to Connect Cloud) #1512 (comment)
| if (x2) rsconnect::removeServer('bookdown.org') | ||
| } | ||
| if (readline('Do you want to delete local records of the Connect deployment? Your book will _not_ be deleted (y/n) ') == 'y') { | ||
| rsconnect::forgetDeployment() |
There was a problem hiding this comment.
@yihui while making so more testing for the migration guide, I am seeing that this will remove ALL deployments. So we can offer it, but it will prompt to remove all.
Example:
> rsconnect::forgetDeployment("docs")
Forget all deployment records for docs? [Y/n]
Y
There was a problem hiding this comment.
Amended via fa7ca78 (only removing the bookdown.org deployment). Thanks!
| # if there are no Connect accounts setup on this machine, offer to add one | ||
| # for connect.posit.cloud | ||
| if (!'connect.posit.cloud' %in% accounts$server) { |
There was a problem hiding this comment.
This modification means that if another server is configured (like internal posit server), then connect to connect cloud won't be done by publish_book()
A user will require to use rsconnect::ConnectCloudUser() explicitly in that case to add the server. Then use publish_book(server = "connect.posit.cloud")).
I'll add that in the migration guide.
close #1507