Add support for deploying quarto files/projects to posit.cloud #444
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
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
mslynch
approved these changes
Jul 6, 2023
aronatkins
reviewed
Jul 6, 2023
Collaborator
aronatkins
left a comment
aronatkins
left a comment
There was a problem hiding this comment.
Could you explain why changes were necessary to how we derive some of the paths and the environment?
I'll wait for @mmarchetti to offer feedback as well.
CHANGELOG.md
Outdated
| ## Unreleased | ||
|
|
||
| ### Added | ||
| - Deploys for Posit Cloud now support quarto source files or projects with `markdown` or `jupyter` engines. |
omar-rs
commented
Jul 6, 2023
| base_dir = file_or_directory | ||
| if not isdir(file_or_directory): | ||
| base_dir = basename(file_or_directory) | ||
| base_dir = dirname(file_or_directory) |
Contributor
Author
There was a problem hiding this comment.
Without this, the bundle would be created with files that have incorrect paths in the case where the deploy command is called with a path or file path argument:
rsconnect deploy quarto <path/to/quarto.qmd>
The resulting bundle would try to add files <path/to/quarto.qmd> instead of quarto.qmd.
omar-rs
commented
Jul 6, 2023
rsconnect/bundle.py
Outdated
| with tarfile.open(mode="w:gz", fileobj=bundle_file) as bundle: | ||
| bundle_add_buffer(bundle, "manifest.json", json.dumps(manifest, indent=2)) | ||
| if environment: | ||
| if environment and environment.source != 'file': |
Contributor
Author
There was a problem hiding this comment.
This is extra protection for the bundle_add_buffer function to be called on in-memory requirements.txt file, not an actual file on disk.
Contributor
There was a problem hiding this comment.
Regardless of whether the package requirements were sourced from a file or from pip freeze, the environment object should contain the correct set. We need to guarantee that the bundle and manifest end up with a requirements.txt file in either case, and with this change, the other case will have to be handled in a code path somewhere else.
Contributor
Author
There was a problem hiding this comment.
@mmarchetti thanks for the suggestion.
I changed the handling of the package requirements to be independent of how they are sourced. One area that was a bit tricky is that the relevant_files used to include the requirements.txt file, which would cause the bundle to include it twice (once from buffer through environment.contents, and another time from relevant_files). With this change, the requirements.txt is only added from environment.contents.
omar-rs
commented
Jul 6, 2023
rsconnect/bundle.py
Outdated
| if environment: | ||
| extra_files = list(extra_files or []) + [environment.filename] | ||
| extra_files = list(extra_files or []) | ||
| if environment.source == 'file': |
Contributor
Author
There was a problem hiding this comment.
If the environment filename requirements.txt was generated in memory, adding it to the extra_files would result in a file not found exception later on when the bundle is created.
Contributor
There was a problem hiding this comment.
I guess this is the other code path. I would suggest always relying on the environment object and using add_buffer rather than splitting the functionality.
omar-rs
commented
Jul 6, 2023
| base_dir = dirname(file_or_directory) | ||
| relevant_files = [file_or_directory] + extra_files | ||
| file_name = basename(file_or_directory) | ||
| relevant_files = [file_name] + extra_files |
Contributor
Author
There was a problem hiding this comment.
For standalone quarto files deployed by specifying a file path, the manifest should include the base name, not he full path filename.
Contributor
Author
I added comments to |
omar-rs
force-pushed
the
quarto_posit_cloud
branch
2 times, most recently
from
35ba66c to
af7f714
Compare
mmarchetti
approved these changes
Jul 11, 2023
Contributor
mmarchetti
left a comment
mmarchetti
left a comment
There was a problem hiding this comment.
The changes look OK. I would like to be sure that we run some QA on this, beyond the automated tests. To include various content types (quarto, notebooks, and app/api) and target servers.
Contributor
Author
Sounds good. Do I need to ping someone specific to help with QA? I'm happy to do some more testing myself, but might be better to have new eyes. Also, I'm hoping we can get this branch released around August 1 (cloud publishing). Is that doable? |
omar-rs
force-pushed
the
quarto_posit_cloud
branch
from
af7f714 to
bf96a55
Compare
omar-rs
force-pushed
the
quarto_posit_cloud
branch
from
bf96a55 to
770c774
Compare
|
Tested: ✅ A. deploy static html (client-rendered content) to posit.cloud ❌ B. deploy standalone quarto file with markdown, server-side Failed with I don't have the option to use the ❌ C. deploy python-based quarto project, server-side This failed with this as the output: ❌ D. deploy quarto site, server-rendered Failed with ❌ The same tests should be runnable against Posit Connect Scenario C had a different failure message: |
|
Latest test results: ✅ Scenario A. deploy static html (client-rendered content) to posit.cloud ✅ : Scenario B. deploy standalone quarto file with markdown, server-side ✅ : Scenario C. deploy python-based quarto project, server-side ✅ Scenario D. deploy quarto site, server-rendered The same tests should be runnable against Posit Connect: |
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.
This change is to enable publishing of
quartofiles or projects toposit.cloudin anticipation of the publishing release in the beginning of August.Intent
With this change, one can use
rsconnect-pythonto publishquartofiles or projects toposit.cloudusing:similarly to publishing to
Posit Connect. The supportedenginevalues aremarkdownandjupyter.Resolves #436
Type of Change
Approach
We have previously added support for publishing quarto outputs to
posit.cloudas Connect applications. With the support of quarto publishing inposit.cloudas static, server-rendered output, this change modifies the output creation API call to pass an additionalrender_by=serverparameter and setapplication_type=static.Automated Tests
Added/updated tests in the following areas:
Directions for Reviewers
The following deployment of quarto is now supported:
where
some_file.qmdis a quarto doc withmarkdownorjupyterengine. Similarly, a quarto project could be supplied instead of a file.Checklist