making error msg for malformed packages.yml better #2078
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
drewbanin
requested changes
Jan 30, 2020
adding actual error text into wrapper Co-Authored-By: Drew Banin <drew@fishtownanalytics.com>
drewbanin
requested changes
Jan 31, 2020
core/dbt/config/project.py
Outdated
| @@ -135,7 +142,7 @@ def package_config_from_data(packages_data): | |||
| packages = PackageConfig.from_dict(packages_data) | |||
| except ValidationError as e: | |||
| raise DbtProjectError( | |||
| MALFORMED_PACKAGE_ERROR | |||
| MALFORMED_PACKAGE_ERROR.format(error=str(e)) | |||
There was a problem hiding this comment.
Ooops - I think I led you astray here - we're actually going to want str(e.message)
Suggested change
| MALFORMED_PACKAGE_ERROR.format(error=str(e)) | |
| MALFORMED_PACKAGE_ERROR.format(error=str(e.msg)) |
In practice, this should result in output like:
Which i think is pretty good all things considered!
Note: we'll also want to remove the duplicated link on line 71!
There was a problem hiding this comment.
Thanks for the review!
drewbanin
approved these changes
Jan 31, 2020
drewbanin
reviewed
Feb 4, 2020
core/dbt/config/project.py
Outdated
| @@ -59,6 +59,17 @@ | |||
| --no-version-check | |||
| """ | |||
|
|
|||
| MALFORMED_PACKAGE_ERROR = """\ | |||
| The packages.yml file in this project is malformed. Please double check the contents | |||
There was a problem hiding this comment.
I see that our style guide checker (flake8) failed with this error:
core/dbt/config/project.py:63:80: E501 line too long (84 > 79 characters)
Can you just split this onto two lines to appease flake8? Thanks!
drewbanin
requested changes
Feb 6, 2020
core/dbt/config/project.py
Outdated
| @@ -59,6 +59,17 @@ | |||
| --no-version-check | |||
| """ | |||
|
|
|||
| MALFORMED_PACKAGE_ERROR = """\ | |||
| The packages.yml file in this project is malformed. Please double check | |||
There was a problem hiding this comment.
flake8 run-test: commands[0] | /bin/bash -c '$(which flake8) --select=E,W,F --ignore=W504 core/dbt plugins/*/dbt'
core/dbt/config/project.py:63:72: W291 trailing whitespace
Suggested change
| The packages.yml file in this project is malformed. Please double check | |
| The packages.yml file in this project is malformed. Please double check |
drewbanin
approved these changes
Feb 10, 2020
|
Thanks for sticking with this one @sonac! Merging this for our 0.16.0 release :D |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I messed up the previous PR during rebase and it was automatically closed. Here's new and clean one.
Fixes #2017