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

Add support for git deployments #501

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

Add support for git deployments #501

wants to merge 9 commits into from

Conversation

mmarchetti
Copy link
Collaborator

This PR adds support for deploying a new git-backed content item using rsconnect-python.

Intent

This is based on @colearendt 's PR. I moved it to a local branch (for CI), updated it to work the the executor, added title/env var support, and added documentation.

Type of Change

  • Bug Fix
  • New Feature
  • Breaking Change

Automated Tests

Needs tests.

Directions for Reviewers

Run something like

rsconnect deploy git -r https://github.com/rstudio/rsc-hello-world-content -d square-plot

See a new git-backed content item in Connect. You can also use --title, and -E to specify environment variables as for other content types.

Checklist

  • I have updated CHANGELOG.md to cover notable changes.
  • I have updated all related GitHub issues to reflect their current state.

@github-actions
Copy link

github-actions bot commented Oct 17, 2023

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
4273 2788 65% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
rsconnect/api.py 67% 🟢
rsconnect/main.py 56% 🟢
TOTAL 61% 🟢

updated for commit: 85d17b4 by action🐍

Copy link
Contributor

@sagerb sagerb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exciting! Thanks for doing this.

Looks like the formatting is off? Is this perhaps the same thing as we saw with the conda removal PR?

@mmarchetti mmarchetti marked this pull request as ready for review October 19, 2023 14:08
@mmarchetti
Copy link
Collaborator Author

Looks like the formatting is off? Is this perhaps the same thing as we saw with the conda removal PR?

It seems better now that your PR has merged.

Copy link
Contributor

@sagerb sagerb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good. I did not verify the functionality.

@kgartland-rstudio
Copy link
Collaborator

rsconnect deploy git --help:

Can we get some more context around these new flags:

  -r, --repository TEXT    [required]
  -b, --branch TEXT
  -d, --subdirectory TEXT

Something like...:

  -r, --repository TEXT             [required] URL to the root of the repository being deployed
  -b, --branch TEXT                 name of the branch being deployed
  -d, --subdirectory TEXT           subdirectory of the content being deployed

@kgartland-rstudio
Copy link
Collaborator

kgartland-rstudio commented Oct 20, 2023

If a -t isn't included, is there a way we can imply the title based on the directory?

In Connect the title is just Unknown. It's doubly bad because the metrics page then just has a blank title for the app:
Screenshot 2023-10-20 at 2 56 41 PM

I beleive the Metrics page uses the name field from the API but most (all?) other deploy methods copy the title into the name field so it's never null.

@kgartland-rstudio
Copy link
Collaborator

kgartland-rstudio commented Oct 20, 2023

If you try to rsconnect deploy git to shinyapps.io you get an internal error:

-> rsconnect deploy git -r https://github.com/rstudio/rsc-hello-world-content -d python3/python-shiny -b master -n pxb42b-kevin-gartland
Validating server... 	[OK]
Deploying git repository ... 	[ERROR]: 'PositClient' object has no attribute 'deploy_git'
Traceback (most recent call last):
  File "/Users/kgartland/.pyenv/versions/3.11.6/envs/fastapi/lib/python3.11/site-packages/rsconnect/main.py", line 94, in wrapper
    result = func(*args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^
  File "/Users/kgartland/.pyenv/versions/3.11.6/envs/fastapi/lib/python3.11/site-packages/rsconnect/main.py", line 1444, in deploy_git
    ce.validate_server().deploy_git().emit_task_log()
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/kgartland/.pyenv/versions/3.11.6/envs/fastapi/lib/python3.11/site-packages/rsconnect/log.py", line 189, in wrapper
    result = method(self, *args, **kw)
             ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/kgartland/.pyenv/versions/3.11.6/envs/fastapi/lib/python3.11/site-packages/rsconnect/api.py", line 790, in deploy_git
    result = self.client.deploy_git(app_name, repository, branch, subdirectory, title, env_vars)
             ^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'PositClient' object has no attribute 'deploy_git'
Internal error: 'PositClient' object has no attribute 'deploy_git'

Should we add support for shinyapps.io here, or just catch the error and display a helpful message?

@kgartland-rstudio
Copy link
Collaborator

If you try to deploy to Connect where Git is disabled, this error is thrown:

(fastapi) [03:13:39 kgartland ~/work/rsconnect-python] (add-git)-> rsconnect deploy git -r https://github.com/rstudio/rsc-hello-world-content -d python3/python-bokeh -b kg-force-updates -n ldap
Validating server... 	[OK]
Deploying git repository ... 	[ERROR]: Received an unexpected response from Posit Connect (calling /rsc/dev-ldap/__api__/applications/b4c58613-e221-471d-ba3d-ffd4cb443262/repo): 404 Not Found
Error: Received an unexpected response from Posit Connect (calling /rsc/dev-ldap/__api__/applications/b4c58613-e221-471d-ba3d-ffd4cb443262/repo): 404 Not Found

Can we we detect if Git is enabled/installed on Connect and throw a friendly error here?

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

Successfully merging this pull request may close these issues.

None yet

4 participants