Skip to content

Migrate client to github.com/databricks/databricks-sdk-go#1848

Merged
nfx merged 27 commits intomasterfrom
migrate/sdk
Feb 20, 2023
Merged

Migrate client to github.com/databricks/databricks-sdk-go#1848
nfx merged 27 commits intomasterfrom
migrate/sdk

Conversation

@nfx
Copy link
Copy Markdown
Contributor

@nfx nfx commented Dec 14, 2022

This PR starts migration to https://github.com/databricks/databricks-sdk-go by replacing the configuration and HTTP client layer with the one defined in Go SDK. SDK inherits a lot of original terraform client implementations.

@nfx nfx marked this pull request as ready for review February 16, 2023 12:12
@nfx
Copy link
Copy Markdown
Contributor Author

nfx commented Feb 16, 2023

current status:

DONE 1239 tests, 79 skipped, 15 failures in 20.123s

@nfx
Copy link
Copy Markdown
Contributor Author

nfx commented Feb 16, 2023

current status:

DONE 1238 tests, 79 skipped, 4 failures in 18.152s

@nfx
Copy link
Copy Markdown
Contributor Author

nfx commented Feb 17, 2023

current status (with go sdk v0.3.1 upstream)

DONE 1238 tests, 79 skipped in 16.687s

nfx added a commit to databricks/databricks-sdk-go that referenced this pull request Feb 17, 2023
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 17, 2023

Codecov Report

Merging #1848 (22b9012) into master (6c943f0) will decrease coverage by 0.82%.
The diff coverage is 58.47%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1848      +/-   ##
==========================================
- Coverage   90.33%   89.51%   -0.82%     
==========================================
  Files         146      140       -6     
  Lines       11911    11038     -873     
==========================================
- Hits        10760     9881     -879     
- Misses        743      767      +24     
+ Partials      408      390      -18     
Impacted Files Coverage Δ
catalog/resource_grants.go 96.22% <0.00%> (ø)
clusters/resource_library.go 84.84% <0.00%> (ø)
common/env.go 100.00% <ø> (+4.34%) ⬆️
exporter/context.go 86.58% <ø> (-0.04%) ⬇️
internal/acceptance/acceptance.go 0.00% <0.00%> (ø)
sql/resource_sql_global_config.go 85.18% <0.00%> (ø)
common/client.go 42.35% <39.47%> (-55.31%) ⬇️
access/resource_sql_permissions.go 86.11% <50.00%> (ø)
mws/data_mws_workspaces.go 83.33% <50.00%> (ø)
storage/adls_gen1_mount.go 97.01% <50.00%> (-2.99%) ⬇️
... and 38 more

Copy link
Copy Markdown
Contributor

@alexott alexott left a comment

Choose a reason for hiding this comment

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

lgtm

@alexott
Copy link
Copy Markdown
Contributor

alexott commented Feb 17, 2023

Need to fix test although

Copy link
Copy Markdown
Contributor

@pietern pietern left a comment

Choose a reason for hiding this comment

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

This removes a few acceptance tests that look legit, most notably the storage ones.

The change in error message on invalid authentication is not optimal. The original message hinted at the provider configuration where the new one doesn't. I think it's worth intercepting and wrapping it to stick to the original message.

@nfx nfx changed the title Migrate to Go SDK client Migrate client to github.com/databricks/databricks-sdk-go Feb 20, 2023
@nfx nfx merged commit 1c18f51 into master Feb 20, 2023
@nfx nfx deleted the migrate/sdk branch February 20, 2023 11:57
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.

4 participants