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 rsconnect json files and folium tests #928

Merged
merged 8 commits into from
Jan 4, 2024
Merged

Conversation

karangattu
Copy link
Collaborator

@karangattu karangattu commented Dec 19, 2023

Add tests for render.display for shiny express apps.
Also, enable deploy tests to shinyapps.io in addition to connect

@karangattu karangattu added this to the v0.7.0 milestone Dec 19, 2023
@karangattu karangattu marked this pull request as ready for review December 19, 2023 20:07
Copy link
Collaborator

@jcheng5 jcheng5 left a comment

Choose a reason for hiding this comment

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

This looks great! It will need to be refactored slightly after @wch merges #904 and #893 (please don't merge ahead of those, they should get merged today).

@karangattu
Copy link
Collaborator Author

karangattu commented Dec 31, 2023

@wch The deploy of express apps to Connect fails to start with the logs in Connect stating the following

2023/12/31 12:40:58 PM: Loading code from "shiny.express.app:app_2e_py"
2023/12/31 12:40:59 PM: Detected Shiny Express app. please note that Shiny Express is still in development and the API is subject to change!
2023/12/31 12:40:59 PM: Unexpected error while running Python API: cannot import name 'ui' from 'shiny.express' (/opt/rstudio-connect/mnt/app/python/env/lib/python3.10/site-packages/shiny/express/__init__.py)
2023/12/31 12:40:59 PM: Traceback (most recent call last):
2023/12/31 12:40:59 PM:   File "/opt/rstudio-connect/python/connect_fastapi_runtime.py", line 489, in 
2023/12/31 12:40:59 PM:     main(sys.argv)
2023/12/31 12:40:59 PM:   File "/opt/rstudio-connect/python/connect_fastapi_runtime.py", line 418, in main
2023/12/31 12:40:59 PM:     user_app = import_app(entrypoint, app_mode)
2023/12/31 12:40:59 PM:   File "/opt/rstudio-connect/python/connect_common_runtime.py", line 167, in import_app
2023/12/31 12:40:59 PM:     app = getattr(app, name_part)
2023/12/31 12:40:59 PM:   File "/opt/rstudio-connect/mnt/app/python/env/lib/python3.10/site-packages/shiny/express/app.py", line 13, in __getattr__
2023/12/31 12:40:59 PM:     return wrap_express_app(Path(name))
2023/12/31 12:40:59 PM:   File "/opt/rstudio-connect/mnt/app/python/env/lib/python3.10/site-packages/shiny/express/_run.py", line 44, in wrap_express_app
2023/12/31 12:40:59 PM:     app_ui = run_express(file)
2023/12/31 12:40:59 PM:   File "/opt/rstudio-connect/mnt/app/python/env/lib/python3.10/site-packages/shiny/express/_run.py", line 98, in run_express
2023/12/31 12:40:59 PM:     exec(
2023/12/31 12:40:59 PM:   File "/opt/rstudio-connect/mnt/app/app.py", line 2, in 
2023/12/31 12:40:59 PM:     from shiny.express import input, ui
2023/12/31 12:40:59 PM: ImportError: cannot import name 'ui' from 'shiny.express' (/opt/rstudio-connect/mnt/app/python/env/lib/python3.10/site-packages/shiny/express/__init__.py)

Shinyapps.io deploys seem to be unaffected.
shiny core apps deployed to connect also seem to be unaffected.

@wch
Copy link
Collaborator

wch commented Jan 3, 2024

@karangattu Do you know how how the deployment script determines which version of Shiny to use? When I deploy locally, I'm able to reproduce the problem if I have this as my requirements.txt:

shiny

This will install Shiny from PyPI, and that version of Shiny does not have a shiny.express.ui module (it only has shiny.express.layout).

However, if I change the requirements.txt to this, the app deploys and runs just fine:

git+https://github.com/posit-dev/py-shiny

This installs Shiny from GitHub, and that version of Shiny does have shiny.express.ui.

In the Connect logs, if you look at "Python environment restore" and search for shiny, what version do you see? If you have shiny==0.6.1.9000, then it's using the GitHub version; if you see something else, then it's not using the correct version.

(One more note: even if it has 0.6.1.9000, it is possible that it's using an older commit, before the shiny.express.ui stuff was added. If that's the case, then I think it's likely that Connect's package caching scheme is using just the version number, and not looking at the commit hash.)

I don't know why this would be different between shinyapps.io and a Connect server, though.

@karangattu
Copy link
Collaborator Author

@karangattu Do you know how how the deployment script determines which version of Shiny to use? When I deploy locally, I'm able to reproduce the problem if I have this as my requirements.txt:

shiny

This will install Shiny from PyPI, and that version of Shiny does not have a shiny.express.ui module (it only has shiny.express.layout).

However, if I change the requirements.txt to this, the app deploys and runs just fine:

git+https://github.com/posit-dev/py-shiny

This installs Shiny from GitHub, and that version of Shiny does have shiny.express.ui.

In the Connect logs, if you look at "Python environment restore" and search for shiny, what version do you see? If you have shiny==0.6.1.9000, then it's using the GitHub version; if you see something else, then it's not using the correct version.

(One more note: even if it has 0.6.1.9000, it is possible that it's using an older commit, before the shiny.express.ui stuff was added. If that's the case, then I think it's likely that Connect's package caching scheme is using just the version number, and not looking at the commit hash.)

I don't know why this would be different between shinyapps.io and a Connect server, though.

The requirements.txt contains the following for each express app

git+https://github.com/posit-dev/py-shiny.git#egg=shiny
git+https://github.com/posit-dev/py-htmltools.git#egg=htmltools

As for the version the deploy script uses, it runs the setup on the dev version, as show in this GitHub actions workflow step so I assume it is using the develop version.
I ran pip install -e ".[dev,test]" on my local and the deploys still don't go through. :(
I suspect shinyapps.io might be caching and the deploys might not be happening and the tests are reading the older version of the app. I have yet to test that theory though.

@karangattu
Copy link
Collaborator Author

The python environment restore in connect logs show the following:

2024/01/03 10:05:36 PM: Packages in the environment: aiofiles==23.2.1, annotated-types==0.6.0, anyio==3.7.1, appdirs==1.4.4, asgiref==3.7.2, click==8.1.7, exceptiongroup==1.2.0, fastapi==0.105.0, h11==0.14.0, htmltools @ git+https://github.com/posit-dev/py-htmltools.git@3b6fa5e4d0fab58b6bd79deae09367ec0c1a69bf, idna==3.6, linkify-it-py==2.0.2, markdown-it-py==3.0.0, mdit-py-plugins==0.4.0, mdurl==0.1.2, packaging==23.2, prompt-toolkit==3.0.36, pydantic==2.5.2, pydantic_core==2.14.5, python-multipart==0.0.6, questionary==2.0.1, shiny @ git+https://github.com/posit-dev/py-shiny.git@ad20db166b220c0866488ddf72706ffabf2e1362, sniffio==1.3.0, starlette==0.27.0, typing_extensions==4.9.0, uc-micro-py==1.0.2, uvicorn==0.24.0.post1, watchfiles==0.21.0, wcwidth==0.2.12, websockets==12.0, wsproto==1.2.0, 

@karangattu
Copy link
Collaborator Author

karangattu commented Jan 4, 2024

Looking at the SHA, the connect env had the shiny version cached from 13 days ago.. I don't know why it is not picking up the latest version

@karangattu
Copy link
Collaborator Author

Fixed it by removing .git#egg=shiny from git+https://github.com/posit-dev/py-shiny.git#egg=shiny listed in the requirements.txt in the directory for each express app that we deploy to both connect and Shinyapps.io

@karangattu karangattu merged commit 3e587ab into main Jan 4, 2024
26 checks passed
@karangattu karangattu deleted the deploy-folium-tests branch January 4, 2024 06:42
schloerke added a commit that referenced this pull request Jan 8, 2024
* main: (24 commits)
  Use dynamic version of py-shiny for deploy tests (#970)
  Add underscores to hide some imports (#978)
  Add rsconnect json files(shinyapps.io tests) and folium tests (#928)
  Express' `value_box()` no longer includes named positional args (#966)
  Include `tooltip()` and `popover()` in express (#949)
  Remove extra call to run_express()
  Call `tagify()` early to intercept `AttributeErrors` (#941)
  Don't pass sidebar twice to navbar_page
  Update changelog
  Update changelog
  Switch from `requests` to `urllib` (#940)
  Bump version to 0.6.1.1
  Fix docstring for page_opts
  Fix API doc sections for Express
  Smarter, lazier, and more complete page default/api for express (#893)
  Change `express.layout` to `express.ui` (#904)
  Remove `@output` from examples (#790)
  feat: Allow for `App` `server=` to take `input` only (#920)
  Add fixes for type stub generation (#828)
  Move quarto express docs to bottom
  ...
schloerke added a commit that referenced this pull request Jan 9, 2024
* main:
  Use dynamic version of py-shiny for deploy tests (#970)
  Add underscores to hide some imports (#978)
  Add rsconnect json files(shinyapps.io tests) and folium tests (#928)
  Express' `value_box()` no longer includes named positional args (#966)
  Include `tooltip()` and `popover()` in express (#949)
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

3 participants