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

Upgrades to streamlit app #1036

Closed
wants to merge 5 commits into from
Closed

Upgrades to streamlit app #1036

wants to merge 5 commits into from

Conversation

sultaniman
Copy link
Contributor

This PR upgrades our streamlit app #459 which is launched when dlt pipeline PIPELINE_NAME show command is called.
It also includes better prompt to detect and install streamlit and additional packages if it is missing.

image

image

@sultaniman sultaniman self-assigned this Feb 29, 2024
Copy link

netlify bot commented Feb 29, 2024

Deploy Preview for dlt-hub-docs canceled.

Name Link
🔨 Latest commit 7c7f017
🔍 Latest deploy log https://app.netlify.com/sites/dlt-hub-docs/deploys/65e0b52c24d16d0008de5179

)


maybe_install_streamlit()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will remove this and will call it in streamlit_app/init.py

Copy link
Collaborator

@rudolfix rudolfix left a comment

Choose a reason for hiding this comment

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

Let's review UI together! First take:

  • the sidebar should contain the most important information like pipeline name, destination etc. maybe last load if trace is present
  • I do not want to distribute images with the library
  • OR we should distribute the streamlit app as dlt extra: we extract this app as seprate dependency

otherwise I'm so happy we are pushing that forward

globals()[package] = importlib.import_module(package)


def query_yes_no(question, default="yes"):
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's wrong with current echo.py and clikck library we use. this should IMO work

Copy link
Contributor Author

@sultaniman sultaniman Mar 6, 2024

Choose a reason for hiding this comment

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

@rudolfix I will use dlt.echo 🙂 thanks for reference

@sultaniman
Copy link
Contributor Author

Closing this in favor of #1060 because I decided to split automatic dependency installation utilities in the separate PR.

@sultaniman sultaniman closed this Mar 6, 2024
@sultaniman sultaniman deleted the issue-459 branch March 6, 2024 12:19
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.

2 participants