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

Display exception message on deploy command failure #2204

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

jonbiemond
Copy link

Description

When the deploy command fails due to a missing import, output the exception message to the terminal. This way the user has the necessary information to resolve the error.

Previous output:

WARNING: Please install additional command line dependencies to use deploy command:
pip install "dlt[cli]"
We ask you to install those dependencies separately to keep our core library small and make it work everywhere.
NOTE: Please refer to our docs at 'https://dlthub.com/docs/walkthroughs/deploy-a-pipeline' for further assistance.

After fix:

ERROR: No module named 'pipdeptree'
WARNING: Please install additional command line dependencies to use deploy command:
pip install "dlt[cli]"
We ask you to install those dependencies separately to keep our core library small and make it work everywhere.
NOTE: Please refer to our docs at 'https://dlthub.com/docs/walkthroughs/deploy-a-pipeline' for further assistance.

Related Issues

Fixes (#2203)

Additional Context

Include the exception message
when the deploy command fails due to a missing import.
This way the user has the necessary
information to resolve the error.
Copy link

netlify bot commented Jan 12, 2025

Deploy Preview for dlt-hub-docs canceled.

Name Link
🔨 Latest commit 54ee623
🔍 Latest deploy log https://app.netlify.com/sites/dlt-hub-docs/deploys/6783a56b22fe1f000897ae02

@sh-rp
Copy link
Collaborator

sh-rp commented Jan 22, 2025

@jonbiemond thanks for your PR. Does installing the "cli" extra not resolve this error? In my view the error message that is there now is correct.

@jonbiemond
Copy link
Author

@sh-rp It did not for me. You can see the details in the linked issue #2203
Once #2207 is merged that should also be resolved.

The problem with the current error message is that it hides the underlying exception. This makes it very difficult to debug and resolve.

@sh-rp
Copy link
Collaborator

sh-rp commented Jan 22, 2025

Ok, I see. This is rather a bug in the extras (that you thankfully fixed!) than a bug here. I think if we merge your other PR, we do not need this change anymore. Alternatively you could write something like:

raise CliCommandException() from deploy_command_import_exception

And not print out this exception. Then the full trace will be visible when running the cli with the --debug flag.

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.

3 participants