Add image to manifests and CLIs #245
Conversation
…to-specify-target-image
tests/test_metadata.py
Outdated
|
|
||
| def tearDown(self): | ||
| # clean up our temporary store | ||
| remove(self.server_store_path) |
There was a problem hiding this comment.
os.remove doesn't handle directories. shutil.rmtree will handle non-empty directories.
dbkegley
left a comment
dbkegley
left a comment
There was a problem hiding this comment.
I'm excited to see the type hints in here as well, I think it's really helpful to increase our confidence.
Do we need to document in the release notes that some of our interfaces are changing since we are removing some of the default parameters? I know that technically the CLI is supposed to be the only public interface, but the API docs page https://docs.rstudio.com/rsconnect-python does list the python APIs as well. Changing these signatures could break existing callers.
More generally, I'm not sure how I feel about adding a shorthand -m flag. I like to reserve shorthand flags for things that are extremely common, but that's just personal preference. Not sure how others feel about that.
tests/target_image/test.sh
Outdated
| "rsconnect write-manifest streamlit --help" | ||
| "rsconnect write-manifest fastapi --help" | ||
| "rsconnect deploy streamlit --help" | ||
| "rsconnect deploy fastapi --help" |
There was a problem hiding this comment.
Should this be an exhaustive list of commands that support --image?
There was a problem hiding this comment.
ended up removing this script.. it wasn't complete and was only a start. I'm going to keep what I created locally, but I'll be working with Kevin to get integration tests which utilize a kubernetes launcher configuration.
tests/target_image/test.sh
Outdated
| ConfirmManifestDiff | ||
|
|
||
|
|
||
| # ConfirmHelp |
There was a problem hiding this comment.
There's a bit of commented code in here, can it be removed?
|
We've removed the API docs for now since they suggested that every function in the package was part of the API - it's currently still up on docs.rstudio.com because we haven't released since then. We still need to create a new "how to use the Python API" type of doc. |
CHANGELOG.md
Outdated
| optional value is only recognized by the RStudio Connect server it it is configured | ||
| to use off-host execution). This is only supported for the write-manifest commands |
There was a problem hiding this comment.
"it it is configured...". Also I think this is missing a leading parenthesis.
So that we're a little more explicit about when this image is used by Connect, how about:
... command line option to specify the target image to be used on the RStudio Connect server during content execution.
There was a problem hiding this comment.
OK, I’m combining the two changelog entries into one:
- An `image` command line option has been added to the `write-manifest` and
`deploy` commands to specify the target image to be used on the RStudio Connect
server during content execution. This is only supported for the `api`, `bokeh`, `dash`,
`fastapi`, `notebook`, `quarto` and `streamlit` sub-commands. It is only
applicable if the RStudio Connect server is configured to use off-host execution.
and then all image helps will be:
Target image to be used during content execution (only applicable if the RStudio Connect server is configured to use off-host execution)
I've also switched the short help string to a capital I (-I) instead of the lowercase -m.
Description
These commands now accept the
--imageor-mparameter, to specify the content imagersconnect write-manifest api:rsconnect write-manifest bokeh:rsconnect write-manifest dash:rsconnect write-manifest fastapi:rsconnect write-manifest notebook:rsconnect write-manifest quarto:rsconnect write-manifest streamlit:rsconnect deploy apirsconnect deploy bokehrsconnect deploy dashrsconnect deploy fastapirsconnect deploy notebookrsconnect deploy quartorsconnect deploy streamlitNot impacted (but related) and unaffected (do not accept the --image parameter)
rsconnect deploy manifestrsconnect deploy other-contentrsconnect deploy htmlFixes rstudio/connect#21381
Implementation Notes
The objective of this PR was to add a single command line option to multiple commands. Unfortunately, the refactoring involved encountered a number of early errors which highlighted the need to enable and utilize better development tooling as part of the refactoring process. After discussions with multiple folks who were focused on the rsconnect-python codebase, we made the decision to gain confidence by making two major changes in the areas where the code was being impacted:
AnyIn addition, these changes allowed us to turn on linting for our test code, which was also calling these refactored functions.
After these changes were made, our confidence of being able to identify issues with the refactoring
staticallyrather thanfunctionalitywas greatly improved. Reproduction of the manually encountered errors within the new codebase were successfully identified by themake lintbuild step and did not require actual execution of the code path having the problem.The changes, therefore, within this PR are a bit larger than one would think for simply adding an option or two to the commands listed above. They were, however, critical to improving our confidence in making this change (where in the past, this was a recognized fragile effort).
I have also added notes into the
CONTRIBUTE.mdfile which should help bootstrap development efforts for this repo. These were previously missing and did represent an initial barrier to contribution.Testing Notes / Validation Steps
This feature does not make any changes to the APIs being used, so it should not have introduced any new compatibility requirements w/ a Connect version. The new options, present on the CLI, will happily be accepted by rsconnect-python regardless how the Connect server is configured. The impact, however, of how they are used by the server, differs by the Connect release and configuration. Starting with the May 2022 release, the Connect server when configured with
Launcher.Enabled = trueandLauncher.Kubernetes = true, will attempt to use theenvironment.imagesetting within the content bundle to match the target content image. If this configuration is not active or if it is a previous version of Connect, that field will be ignored and the legacy behavior will not be impacted. Because of this.. it is prudent to run a few of these commands against an older version or a non-kubernetes configuration, just to make sure there is no compatibility issue. Those steps are NOT outlined below.Approach taken
By using a kubernetes configuration for Connect, which utilizes two duplicate images, we are able to verify the functionality of which image is used by the server when the image flag is specified and when it is not.
Verification for each of the impacted commands is grouped by target, allowing us to focus on a target at a time, but executing fairly similar steps for each target:
Validate write-manifest
rsconnect write-manifest -image rstudio/dev-connect-duplicate <target>to update the manifest.json to include theenvironment.imagesettingValidate deploy manifest for the manifest.json with the image setting
rsconnect deploy manifest <manifest file>to confirm proper deploymentValidate deploy with and without image
rsconnect deploy <target>to validate that target is able to be deployed to server and that the server selects the content image target by using the built-in algorithm (will selectrstudio/dev-connect).rsconnect deploy <target> --image rstudio/dev-connect-duplicateto validate that target is able to be deployed to server and that the server uses the image option to determine the target image (will berstudio/dev-connect-duplicate).Testing details
Need to be setup specifically with Python 3.8.x to work with the test data / content outlined below.
Setup virtual environment to use branch rsconnect within the base directory of rsconnect-python repo:
For Connect, I recommend using the new
dev-connectdocker file workflow, which will be available frommainbefore this PR goes to testing, as will David's supporting server branch which implements the image logic needed on the server.To start the server, you'll need to:
just ui/dashboard/cleanjust bootstrap buildyeti/docker/built-in/connect-shared-files/rsc-launcher-runtime.ymlto have the following two additional image specifications, to be placed at top of image list (on line 3, after the lineimages:). This should move all of the other image entries down in the file / list.imagePullPolicy=Neverto the pods volume definition.packaging/launcher/connect-k8s-templates/job.tpltoconfig/launcher/k8s-custom-templatesconfig/launcher/k8s-custom-templates/job.tpland add the following line after line 83 (image: {{ toYaml .Job.container.image }}) - should be indented same level as the image line.justfile):docker volume rm dev-connect-volumekubectl -n rsc-dev delete jobs --allkubectl -n rsc-dev delete svc --allUSE_KUBERNETES=yes just start-dev-connectValidate changes for target: API
Verify write-manifest api
WITH the venv active.. cd into your rstudio/connect-content repo.
From: rstudio/connect-content
and confirm that
--imageand-mis listed in the helpcd bundles/python-flaskapi rsconnect write-manifest api --helpConfirm that
--imageand-mis listed in the helpconfirm that you see the environment/image path added in the new manifest. This will look something like:
where the first section is the important verification (and the second is just an artifact that we added a file).
Verify deploy manifest
Verify output is similar to:
Verify deploy api
WITH your terminal working directory same as previously (bundles/python-flaskapi)
rsconnect deploy api -s http://localhost:3939 -k c7achTdggWVGtr851azThcyYDwH5JRgf --new .Confirm that the automatic image was selected, by verifying the output is similar to:
rsconnect deploy api -s http://localhost:3939 -k c7achTdggWVGtr851azThcyYDwH5JRgf --image rstudio/dev-connect-duplicate --new .Verify the output is similar to the following:
Validate changes for target: BOKEH
Verify write-manifest bokeh
WITH the venv active.. cd into your rstudio/connect-content repo.
From: rstudio/connect-content:
Confirm that you see the environment/image path added in the new manifest. This will look something like the following
Where the first section is the important verification (and the second is just an artifact that we added a file).
Verify deploy manifest
Verify output is similar to:
Verify deploy bokeh
WITH your terminal working directory same as previously (bundles/python-bokeh)
rsconnect deploy bokeh -s http://localhost:3939 -k c7achTdggWVGtr851azThcyYDwH5JRgf --new .Confirm that the automatic image was selected, by verifying the output is similar to:
rsconnect deploy bokeh -s http://localhost:3939 -k c7achTdggWVGtr851azThcyYDwH5JRgf --image rstudio/dev-connect-duplicate --new .Verify the output is similar to the following:
Validate changes for target: DASH
Verify write-manifest dash
WITH the venv active.. cd into your rstudio/connect-content repo.
From: rstudio/connect-content:
cd bundles/python-dash rsconnect write-manifest dash --helpConfirm that
--imageand-mis listed in the helpConfirm that you see the environment/image path added in the new manifest. This will look something like the following
Where the first section is the important verification (and the second is just an artifact that we added a file).
Verify deploy manifest
Verify output is similar to:
Verify deploy dash
WITH your terminal working directory same as previously (bundles/python-dash)
rsconnect deploy dash -s http://localhost:3939 -k c7achTdggWVGtr851azThcyYDwH5JRgf --new .Confirm that the automatic image was selected, by verifying the output is similar to:
rsconnect deploy dash -s http://localhost:3939 -k c7achTdggWVGtr851azThcyYDwH5JRgf --image rstudio/dev-connect-duplicate --new .Verify the output is similar to the following:
Validate changes for target: FASTAPI
Verify write-manifest fastapi
WITH the venv active.. cd into your rstudio/connect-content repo.
From: rstudio/connect-content:
cd bundles/fastapi-simple rsconnect write-manifest fastapi --helpConfirm that
--imageand-mis listed in the helpConfirm that you see the environment/image path added in the new manifest. This will look something like the following
Where the first section is the important verification (and the second is just an artifact that we added a file).
Verify deploy manifest
Verify output is similar to:
Verify deploy fastapi
WITH your terminal working directory same as previously (bundles/fastapi-simple)
rsconnect deploy fastapi -s http://localhost:3939 -k c7achTdggWVGtr851azThcyYDwH5JRgf --new .Confirm that the automatic image was selected, by verifying the output is similar to:
rsconnect deploy fastapi -s http://localhost:3939 -k c7achTdggWVGtr851azThcyYDwH5JRgf --image rstudio/dev-connect-duplicate --new .Verify the output is similar to the following:
Validate changes for target: NOTEBOOK
Verify write-manifest notebook
WITH the venv active.. cd into your rstudio/connect-content repo.
From: rstudio/connect-content:
cd bundles/stock-report-jupyter rsconnect write-manifest notebook --helpConfirm that
--imageand-mis listed in the help# baseline the manifest w/ your virtual environment basics rsconnect write-manifest notebook --overwrite stock-report-jupyter.ipynb quandl-wiki-tsla.json.gz thumbnail.jpg mv manifest.json manifest-original.json rsconnect write-manifest notebook --image rstudio/dev-connect-duplicate stock-report-jupyter.ipynb quandl-wiki-tsla.json.gz thumbnail.jpg diff manifest.json manifest-original.jsonConfirm that you see the environment/image path added in the new manifest. This will look something like the following
Where the first section is the important verification (and the second is just an artifact that we added a file).
Verify deploy manifest
Verify output is similar to:
Verify deploy notebook
WITH your terminal working directory same as previously (bundles/stock-report-jupyter)
Confirm that the automatic image was selected, by verifying the output is similar to:
Verify the output is similar to the following:
Validate changes for target: QUARTO
Verify write-manifest quarto
WITH the venv active.. cd into your rstudio/connect-content repo.
From: rstudio/connect-content:
cd bundles/quarto-proj-py rsconnect write-manifest quarto --helpConfirm that
--imageand-mis listed in the helpConfirm that you see the environment/image path added in the new manifest. This will look something like the following
Where the first section is the important verification (and the second is just an artifact that we added a file).
Verify deploy manifest
Verify output is similar to:
Verify deploy quarto
WITH your terminal working directory same as previously (bundles/quarto-project-py)
rsconnect deploy quarto -s http://localhost:3939 -k c7achTdggWVGtr851azThcyYDwH5JRgf --new .Confirm that the automatic image was selected, by verifying the output is similar to:
rsconnect deploy quarto -s http://localhost:3939 -k c7achTdggWVGtr851azThcyYDwH5JRgf --image rstudio/dev-connect-duplicate --new .Verify the output is similar to the following:
Validate changes for target: STREAMLIT
Verify write-manifest streamlit
WITH the venv active.. cd into your rstudio/connect-content repo.
From: rstudio/connect-content:
cd bundles/python-streamlit rsconnect write-manifest streamlit --helpConfirm that
--imageand-mis listed in the helpConfirm that you see the environment/image path added in the new manifest. This will look something like the following
Where the first section is the important verification (and the second is just an artifact that we added a file).
Verify deploy manifest
Verify output is similar to:
Verify deploy streamlit
WITH your terminal working directory same as previously (bundles/python-streamlit)
rsconnect deploy streamlit -s http://localhost:3939 -k c7achTdggWVGtr851azThcyYDwH5JRgf --new .Confirm that the automatic image was selected, by verifying the output is similar to:
rsconnect deploy streamlit -s http://localhost:3939 -k c7achTdggWVGtr851azThcyYDwH5JRgf --image rstudio/dev-connect-duplicate --new .Verify the output is similar to the following:
Confirm other commands do not accept --image param
deploy manifest
from the bundles/python-streamlit subdirectory
Confirm you see an error, similar to:
Confirm help doesn't include image
It should not include a --image option
deploy other-content
from the bundles/python-streamlit subdirectory
Confirm you see an error, similar to:
Confirm help doesn't include image
It should not include a --image option