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

Fix #294 - Reliable indication of Azure #312

Merged
merged 2 commits into from
Sep 10, 2020
Merged

Fix #294 - Reliable indication of Azure #312

merged 2 commits into from
Sep 10, 2020

Conversation

nfx
Copy link
Contributor

@nfx nfx commented Sep 10, 2020

  • DatabricksClient.IsAzure is checking for both resource id and workspace hostname
  • Added more unit test coverage for ClustersAPI
  • Fixed version linking

* DatabricksClient.IsAzure is checking for both resource id and workspace hostname
* Added more unit test coverage for ClustersAPI
* FIxed version linking
@TravisBuddy
Copy link

Hey @nfx,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 3715ae80-f381-11ea-addc-35ae975c2fab

@codecov-commenter
Copy link

codecov-commenter commented Sep 10, 2020

Codecov Report

Merging #312 into master will increase coverage by 0.71%.
The diff coverage is 38.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #312      +/-   ##
==========================================
+ Coverage   64.18%   64.90%   +0.71%     
==========================================
  Files          55       55              
  Lines        6741     6744       +3     
==========================================
+ Hits         4327     4377      +50     
+ Misses       2044     1996      -48     
- Partials      370      371       +1     
Impacted Files Coverage Δ
common/client.go 73.22% <0.00%> (ø)
compute/model.go 95.65% <ø> (ø)
internal/qa/testing.go 48.93% <20.00%> (-0.71%) ⬇️
compute/clusters.go 81.50% <66.66%> (+20.60%) ⬆️
compute/common_instances.go 73.33% <100.00%> (ø)
provider/provider.go 91.85% <100.00%> (+2.55%) ⬆️

@nfx nfx merged commit aafa7d5 into master Sep 10, 2020
@nfx nfx deleted the fix/294 branch September 10, 2020 16:44
@nfx nfx mentioned this pull request Sep 10, 2020
@sdebruyn
Copy link
Contributor

This will cover 99% of the cases, however, users can use custom hostnames. Maybe an optional string in the provider to explicitly set it to AWS or Azure can help us? Or an API endpoint just letting us know which cloud we're working with.

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.

[ISSUE] Error when omitting cluster_id when creating a databricks_azure_adls_gen2_mount resource
4 participants