-
Notifications
You must be signed in to change notification settings - Fork 409
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
Verify workspace reachability through HTTP client #882
Conversation
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.
LGTM from a cursory look. I only touched the parts dealing with hostname synthesis based off the account console hostname.
Codecov Report
@@ Coverage Diff @@
## master #882 +/- ##
==========================================
- Coverage 83.80% 83.65% -0.16%
==========================================
Files 92 92
Lines 8233 8258 +25
==========================================
+ Hits 6900 6908 +8
- Misses 842 859 +17
Partials 491 491
|
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.
lgtm
* `WorkspacesAPI.WaitForRunning` used to check if workspace is reachable through `net.Dial` on TCP level, though this scenario ignored HTTPS proxies, which are common for enterprise setup. * `mws.Workspace` now gets `workspace_url` computed in accordance to Accounts API endpoint used from a single place in `WorkspacesAPI.Read`, if there's no `workspace_url` returned in the response, which allows fully unit testing the verification flow. * added `DatabricksClient.ClientForHost` to talk to just created workspace with the same authentication parameters. This fixes #874
Also made test surface smaller
bd7df75
to
a6effac
Compare
WorkspacesAPI.WaitForRunning
used to check if workspace is reachable throughnet.Dial
on TCP level, though this scenario ignored HTTPS proxies, which are common for enterprise setup.mws.Workspace
now getsworkspace_url
computed in accordance to Accounts API endpoint used from a single place inWorkspacesAPI.Read
, if there's noworkspace_url
returned in the response, which allows fully unit testing the verification flow.DatabricksClient.ClientForHost
to talk to just created workspace with the same authentication parameters.This fixes #874