-
Notifications
You must be signed in to change notification settings - Fork 4
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
docs: update streamlit example and readmes #78
Conversation
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
certifi==2024.2.2 | ||
charset-normalizer==3.3.2 | ||
click==8.1.7 | ||
databricks-sql-connector==3.0.1 |
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.
This is more a question than an issue or suggestion: what happened with all of these dependencies? Were they old copy 🍝 or something?
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 did a pip freeze initially on this example but I like the approach that @plascaray used in his examples better where we only capture the dependencies that we really need, and then let pip handle transitive deps. No need for us to record them here IMO so I was cleaning this up so that it's consistent with the other examples
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.
💯 Absolutely
Thanks!
cfg = Config(host=DB_HOST_URL, credentials_provider=credentials_provider) | ||
# cfg = Config(host=DB_HOST_URL, token=DB_PAT) |
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.
❤️ 🧹 #️⃣
Thank for cleaning this up
This PR makes the streamlit example consistent with our other examples
I'm also going to update the other PRs to talk about the required environment vars before running the apps locally