-
Notifications
You must be signed in to change notification settings - Fork 3
Add shiny for python dashboard tips example #230
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
base: main
Are you sure you want to change the base?
Conversation
"category": "example", | ||
"tags": ["python", "shiny"], | ||
"minimumConnectVersion": "2025.04.0", | ||
"version": "0.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can leave this 0.0.0 until merged and then the connect team can take this over to ensure it works as expected and run it through cicd
}, | ||
"environment": { | ||
"python": { | ||
"requires": "~=3.11.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this a hard requirement or can this support back to 3.9?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a similar question to @mconflitti-pbc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the version is set to 0.0.0, we can merge this and then test it and bump it. Thanks for the contribution. Sound good @dotNomad ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just had one question about the inclusion of the "Deploy to Connect Cloud" button. I'm good with merging this and incrementing the version once merged. 👍
We can adjust the README once we get the UI in place in Connect too.
@@ -78,7 +78,7 @@ required to group content in the Gallery. | |||
{ | |||
... | |||
"extension": { | |||
"category": "extension, | |||
"category": "extension", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch thank you!
@@ -0,0 +1,7 @@ | |||
## Dashboard tips app | |||
|
|||
<a href='https://connect.posit.cloud/publish?framework=shiny&sourceRepositoryURL=https%3A%2F%2Fgithub.com%2Fposit-dev%2Fpy-shiny-templates&sourceRef=main&sourceRefType=branch&primaryFile=dashboard-tips%2Fapp-express.py&pythonVersion=3.11'><img src='https://cdn.connect.posit.cloud/assets/deploy-to-connect-blue.svg' align="right" /></a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels a bit strange to have a "Deploy to Connect Cloud" link here if someone is viewing the README from the Gallery. We also plan to expose these READMEs in the Connect UI for the Gallery which presents weird questions about showing this. Any thoughts on removing this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh interesting! Yes I agree this shouldnt be in the connect on-prem gallery as-is. Though, I really like that as a feature for one click installs!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could swap this out to a link to the posit-dev/py-shiny-templates repo?
That README has the link https://github.com/posit-dev/py-shiny-templates/tree/main/dashboard-tips and it makes more sense there.
}, | ||
"environment": { | ||
"python": { | ||
"requires": "~=3.11.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a similar question to @mconflitti-pbc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops meant to select "Approve"
Adds this Shiny template as an example.
Not sure why the integration tests are failing, but looks as though it's due to a missing secret (since I don't have write access)?