Skip to content

Conversation

@mmarchetti
Copy link
Contributor

This PR ensures that manifest file lists always contain posix paths, and not Windows paths (containing \). shinyapps.io does not accept Windows paths in the manifest.json.

Intent

Fixes #373

Type of Change

  • Bug Fix
  • New Feature
  • Breaking Change

Approach

Using pathlib.Path.as_posix() to convert to Posix paths.

Automated Tests

I removed all of the special case unit tests for Windows, and make all of the tests run on all platforms.

Directions for Reviewers

Will need to test deployments on posix and Windows clients, deploying to Connect and to shinyapps.io.

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 May 19, 2023

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
4197 2654 63% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
rsconnect/bundle.py 77% 🟢
TOTAL 77% 🟢

updated for commit: 8774a3d by action🐍

@mmarchetti mmarchetti requested a review from bcwu May 19, 2023 19:06
@jcheng5
Copy link
Contributor

jcheng5 commented May 24, 2023

Hi @mmarchetti, gentle nudge. Anything standing in the way of this getting merged and released? Let me know if I can help.

@kgartland-rstudio
Copy link
Contributor

kgartland-rstudio commented May 24, 2023

Hi @mmarchetti, gentle nudge. Anything standing in the way of this getting merged and released? Let me know if I can help.

I'm testing this now and the manifests created in Windows now look to be using the proper filepaths, but is there a live shinyapps.io instance where I can deploy to ensure they're working properly?

EDIT:
@dskard helped me out. I'm testing deploys now.

@kgartland-rstudio
Copy link
Contributor

Test app with nested directories:
https://huggingface.co/spaces/posit/shiny-for-python-template/tree/main

shinyapps.io

  • write-manifest and deploy from windows
  • write-manifest and deploy from macOS
  • write-manifest and deploy from linux

connect

  • write-manifest and deploy from windows
  • write-manifest and deploy from macOS
  • write-manifest and deploy from linux

Also deployed other manifest content and ran rsconnect-python tests in Connect against this branch. All passed.

Good to merge!

@mmarchetti mmarchetti merged commit 6a36b39 into master May 25, 2023
@mmarchetti mmarchetti deleted the mm-posix-paths branch May 25, 2023 12:16
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.

Shiny app deployment fails when static content is present in the app

5 participants