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 error message for validate connections #1554

Merged
merged 4 commits into from
Jan 10, 2023
Merged

Conversation

feluelle
Copy link
Member

@feluelle feluelle commented Jan 9, 2023

Description

What is the current behavior?

Currently, we are not giving any hints to the user about an invalid/failed connection.

closes: #1535

What is the new behavior?

Print an error message when validating a connection.

Does this introduce a breaking change?

Yes, kind of. We are not erroring out anymore when the file path internally could not get resolved in case of sqlite connections. This is so that an invalid sqlite connection does not block you from running any generate/validate/run commands. Previously it was raising an exception which led to an immediate exit.

Checklist

  • Created tests which fail without the change (if possible)
  • Extended the README / documentation, if necessary

@codecov
Copy link

codecov bot commented Jan 9, 2023

Codecov Report

Base: 94.01% // Head: 97.71% // Increases project coverage by +3.69% 🎉

Coverage data is based on head (a46f78c) compared to base (38809fa).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1554      +/-   ##
==========================================
+ Coverage   94.01%   97.71%   +3.69%     
==========================================
  Files          89       22      -67     
  Lines        4347      787    -3560     
  Branches      428        0     -428     
==========================================
- Hits         4087      769    -3318     
+ Misses        178       18     -160     
+ Partials       82        0      -82     
Impacted Files Coverage Δ
sql-cli/sql_cli/__main__.py 100.00% <100.00%> (ø)
sql-cli/sql_cli/connections.py 100.00% <100.00%> (+2.77%) ⬆️
python-sdk/src/astro/utils/path.py
python-sdk/src/astro/lineage/__init__.py
python-sdk/src/astro/files/types/__init__.py
python-sdk/src/astro/utils/table.py
python-sdk/src/astro/sql/table.py
python-sdk/src/astro/databricks/delta.py
python-sdk/src/astro/sql/operators/load_file.py
python-sdk/src/astro/databricks/api_utils.py
... and 60 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

feluelle and others added 2 commits January 10, 2023 13:53
Co-authored-by: Tatiana Al-Chueyr <tatiana.alchueyr@gmail.com>
@feluelle feluelle merged commit 3085dbd into main Jan 10, 2023
@feluelle feluelle deleted the feature/validation-message branch January 10, 2023 13:51
utkarsharma2 pushed a commit that referenced this pull request Jan 17, 2023
# Description

## What is the current behavior?

Currently, we are not giving any hints to the user about an
invalid/failed connection.

closes: #1535

## What is the new behavior?

Print an error message when validating a connection.

## Does this introduce a breaking change?

Yes, kind of. We are not erroring out anymore when the file path
internally could not get resolved in case of sqlite connections. This is
so that an invalid sqlite connection does not block you from running any
generate/validate/run commands. Previously it was raising an exception
which led to an immediate exit.

### Checklist
- [x] Created tests which fail without the change (if possible)
- [ ] Extended the README / documentation, if necessary

Co-authored-by: Tatiana Al-Chueyr <tatiana.alchueyr@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@pankajkoti pankajkoti added this to the sql-cli/0.3.0 milestone Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Print error message in validate command
3 participants