Switch image parameters to be optional at public api layer #251
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…' into sagerb-switch-image-to-optional-at-public-api-layer
mmarchetti
approved these changes
May 27, 2022
|
Regression testing:
Validation complete, ready to merge. |
mmarchetti
deleted the
sagerb-switch-image-to-optional-at-public-api-layer
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
NOTE: This PR has Bincheng's branch merged in, so that I could holistically search and refactor all image parameter usages.
Description
To maintain compatibility at the public api layer of rsconnect-python, we have made the decision to make the
imageparameter be optional and default toNone.This impacts the following CLI commands which support the image parameter (
--imageor-I).rsconnect 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 htmlImplementation Notes
RSConnect-Python's public API library, uses functions which are not prefixed with an underscore (
_) to indicate internal functions. These functions will use fixed arguments to leverage tooling as much as possible and therefore are not effected by this change.Testing Notes / Validation Steps
Validation follows the approach outlined within #245, where image support was first introduced. These steps should give a good validation for direct usage of the rsconnect-python CLI. It does not include, however, integration testing between
rsconnect-pythonandrsconnect-jupyter(which builds upon thersconnect-pythoncodebase). That will need to be validated in a separate way.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
confirm 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:
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 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:
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 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:
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 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:
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 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:
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 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: