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

Upgrade to 0.17.0rc2 #83

Merged
merged 6 commits into from
May 22, 2020
Merged

Upgrade to 0.17.0rc2 #83

merged 6 commits into from
May 22, 2020

Conversation

beckjake
Copy link
Contributor

@beckjake beckjake commented May 14, 2020

Fixes #78
Fixes #81 (sort of...)

Prevent users from setting database just about anywhere. It would have been nice to make database and schema equivalent, like in real spark, but that requires some ugly hacks on the dbt-core end.

In the longer term, it would be nice to expose more things about config to plugins. Adapter specific configs can add new fields, but they can't remove them, and they can't even really check on them to see if they make sense because the builtin fields are examined before the adapter ones are considered. I'm not sure how exactly we could support such behavior in the dataclasses/hologram paradigm, but we can think about it more. Instead, the spark adapter now checks when relations are built, and errors out then. I don't think it's ideal, but it happens before dbt starts really running (when it's examining the manifest for what schemas to create) so at least there's that.

This also upgrades the plugin to 0.17.0rc1 0.17.0rc2 in anticipation of the dbt 0.17.0 release - 0.17.0 support should just be a bumpversion away.

Jacob Beck added 2 commits May 13, 2020 10:07
Database is not allowed to be set basically anywhere
@beckjake beckjake requested a review from jtcohen6 May 14, 2020 14:41
Copy link
Contributor

@jtcohen6 jtcohen6 left a comment

Choose a reason for hiding this comment

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

I took for a spin locally with containerized Spark and all worked as expected. I tested with overriding generate_schema_name (as expected) and generate_database_name (no effect). I left a few comments around the specific error messages to return.

Weirdly, when I try to test against Databricks, I'm seeing only this error:

Encountered an error:
Runtime Error
  Runtime Error
    Database Error

This is before it even tries to connect to the cluster. Here are the last few lines from the traceback:

File "/Users/jerco/dev/product/spark/env/lib/python3.7/site-packages/dbt_core-0.17.0rc1-py3.7.egg/dbt/task/runnable.py", line 473, in list_schemas
    for s in adapter.list_schemas(database_quoted)
  File "/Users/jerco/dev/product/spark/env/lib/python3.7/site-packages/dbt_core-0.17.0rc1-py3.7.egg/dbt/adapters/sql/impl.py", line 233, in list_schemas
    kwargs={'database': database}
  File "/Users/jerco/dev/product/spark/env/lib/python3.7/site-packages/dbt_core-0.17.0rc1-py3.7.egg/dbt/adapters/base/impl.py", line 981, in execute_macro
    self.release_connection()
  File "/usr/local/Cellar/python/3.7.7/Frameworks/Python.framework/Versions/3.7/lib/python3.7/contextlib.py", line 130, in __exit__
    self.gen.throw(type, value, traceback)
  File "/Users/jerco/dev/product/spark/dbt-spark/dbt/adapters/spark/connections.py", line 201, in exception_handler
    raise dbt.exceptions.RuntimeException(str(exc))
dbt.exceptions.RuntimeException: Runtime Error
  Runtime Error
    Database Error

dbt/adapters/spark/connections.py Outdated Show resolved Hide resolved
dbt/adapters/spark/relation.py Outdated Show resolved Hide resolved
beckjake and others added 2 commits May 14, 2020 09:36
Co-authored-by: Jeremy Cohen <jeremy@fishtownanalytics.com>
Co-authored-by: Jeremy Cohen <jeremy@fishtownanalytics.com>
@jtcohen6
Copy link
Contributor

jtcohen6 commented May 14, 2020

For posterity: the issue above was actually a matter of my Databricks token having expired. (My b!) We're adding a more descriptive error (and helpful reminder) for the next time this happens.

Handle EOFError as a fatal error, and point out token expiration if there is a token
@beckjake
Copy link
Contributor Author

I'm going to hold off on merging this until dbt 0.17.0 is officially out - once that happens I'll run the appropriate bumpversion commands and push to this branch.

@beckjake beckjake changed the title Upgrade to 0.17.0rc1 Upgrade to 0.17.0rc2 May 22, 2020
@jtcohen6 jtcohen6 merged commit a4d6874 into master May 22, 2020
@jtcohen6 jtcohen6 deleted the feature/0.17.0 branch May 22, 2020 20:55
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.

Support dbt v0.17.0 Reconcile schema/database as profile + model configs
2 participants