-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat: Bump version number and update adapter to work with dbt v1.0.0 #38
Conversation
…ction. Minor change to field values in profiles.yml to match internal nomenclature: host -> api_endpoint.
… working. I think that's actually an issue in main.
…ses of jaydebeapi, Java, and the JDBC.
…to move_to_our_db_api
…bt-firebolt into move_to_our_db_api
tests/integration/firebolt.dbtspec
Outdated
@@ -6,8 +6,10 @@ target: | |||
engine_name: "{{ env_var('ENGINE_NAME') }}" | |||
user: "{{ env_var('USER_NAME') }}" | |||
password: "{{ env_var('PASSWORD') }}" | |||
schema: "{{ env_var('SCHEMA') }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we just hardcode this value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this is either a by-team or by-user setting, so it needs to be set locally. Since we don't have schemas, this string just gets appended to each table name, so people aren't overwriting each others' tables. As an example, my "schema" is "emf," so all my table names are prefixed with "emf_"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ach, sorry. Yes, it should be hard-coded. I was thinking of profiles.yml
, not this integration test file.
dbt/adapters/firebolt/connections.py
Outdated
@@ -100,9 +100,9 @@ def open(cls, connection): | |||
engine_name = credentials.engine_name | |||
error_msg_append = '' | |||
raise EngineOfflineException( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no need for this exception since SDK checks running engine by itself and raises an appropriate error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kk
Kudos, SonarCloud Quality Gate passed! |
Resolves FIR-10759
Unblocks FIR-10765
Checklist
CHANGELOG.md
and added information about my change.Note: This PR is a branch of #10 , so is currently blocked until that is merged.