get_response -> AdapterResponse
#265
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
leahwicz
approved these changes
Dec 3, 2021
This was referenced Dec 3, 2021
leahwicz
pushed a commit
that referenced
this pull request
Dec 3, 2021
* Return AdapterResponse from get_response * fix flake
leahwicz
added a commit
that referenced
this pull request
Dec 3, 2021
* Refactor seed macros, clearer sql param logging (#250) * Try refactoring seed macros * Add changelog entry * 187: Adding apache hudi support to dbt (#210) * initial working version * Rebased and resolve all the merge conflicts. * Rebased and resolved merge conflicts. * Removed hudi dep jar and used the released version via packages option * Added insert overwrite unit tests for hudi * Used unique_key as default value for hudi primaryKey option * Updated changelog.md with this new update. * Final round of testing and few minor fixes * Fixed lint issues * Fixed the integration tests * Fixed the circle ci env to add hudi packages * Updated hudi spark bundle to use scala 2.11 * Fixed Hudi incremental strategy integration tests and other integration tests * Fixed the hudi hive sync hms integration test issues * Added sql HMS config to fix the integration tests. * Added hudi hive sync mode conf to CI * Set the hms schema verification to false * Removed the merge update columns hence its not supported. * Passed the correct hiveconf to the circle ci build script * Disabled few incremental tests for spark2 and reverted to spark2 config * Added hudi configs to the circle ci build script * Commented out the Hudi integration test until we have the hudi 0.10.0 version * Fixed the macro which checks the table type. * Disabled this model since hudi is not supported in databricks runtime, will be added later * Update profile_template.yml for v1 (#247) * Update profile_template.yml for v1 * PR feedback, fix indentation issues * It was my intention to remove the square brackets * Fixup changelog entry * Merge main, update changelog * Bump version to 1.0.0rc2 (#259) * bumpversion 1.0.0rc2 * Update changelog * Use pytest-dbt-adapter==0.6.0 * Corrected definition for set full_refresh_mode (#262) * Replaced definition for set full_refresh_mode * Updated changelog * Edit changelog Co-authored-by: Jeremy Cohen <[email protected]> * `get_response` -> `AdapterResponse` (#265) * Return AdapterResponse from get_response * fix flake Co-authored-by: Jeremy Cohen <[email protected]> Co-authored-by: Vinoth Govindarajan <[email protected]> Co-authored-by: Sindre Grindheim <[email protected]>
4 tasks
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.
In
core.dbt.adapters.sql.connection, we:get_responseis an abstract method which can returnUnion[AdapterResponse, str]AdapterResponse(an object with a_messageattribute) when we fireSQLQueryStatus, ever since dbt-labs/dbt-core@b3039fdBefore this change,
dbt-sparkimplementedget_responsein a super naive way (it just returns a string). This PR changes it to return anAdapterResponseinstead, with no functional change.In
dbt-core, we should either:get_response, then either use it_messageattribute (ifAdapterResponse) or itself (ifstr)get_responseabstract method must return anAdapterResponse— not a bad change, and it's effectively true, but we should think about waiting for a minor version release with clearer communication ahead of time