Skip to content

Commit

Permalink
Capture stdout and stderr separately when calling Azure CLI (#460)
Browse files Browse the repository at this point in the history
## Changes

They were captured together such that any warnings emitted to stderr
would turn the output of the command into malformed JSON and raise an
error.

Traces all the way back to #30.

## Tests

- [x] `make test` run locally
- [x] `make fmt` applied
- [ ] relevant integration tests applied
  • Loading branch information
pietern authored Nov 28, 2023
1 parent 61dedbc commit d689d62
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 7 deletions.
8 changes: 5 additions & 3 deletions databricks/sdk/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -359,16 +359,18 @@ def refresh(self) -> Token:
is_windows = sys.platform.startswith('win')
# windows requires shell=True to be able to execute 'az login' or other commands
# cannot use shell=True all the time, as it breaks macOS
out = subprocess.check_output(self._cmd, stderr=subprocess.STDOUT, shell=is_windows)
it = json.loads(out.decode())
out = subprocess.run(self._cmd, capture_output=True, check=True, shell=is_windows)
it = json.loads(out.stdout.decode())
expires_on = self._parse_expiry(it[self._expiry_field])
return Token(access_token=it[self._access_token_field],
token_type=it[self._token_type_field],
expiry=expires_on)
except ValueError as e:
raise ValueError(f"cannot unmarshal CLI result: {e}")
except subprocess.CalledProcessError as e:
message = e.output.decode().strip()
stdout = e.stdout.decode().strip()
stderr = e.stderr.decode().strip()
message = stdout or stderr
raise IOError(f'cannot get access token: {message}') from e


Expand Down
11 changes: 10 additions & 1 deletion tests/test_auth_manual_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,13 @@ def test_azure_cli_fallback(monkeypatch):
monkeypatch.setenv('FAIL_IF', 'subscription')
resource_id = '/subscriptions/123/resourceGroups/abc/providers/Microsoft.Databricks/workspaces/abc123'
cfg = Config(auth_type='azure-cli', host='x', azure_workspace_resource_id=resource_id)
assert 'X-Databricks-Azure-SP-Management-Token' in cfg.authenticate()
assert 'X-Databricks-Azure-SP-Management-Token' in cfg.authenticate()


def test_azure_cli_with_warning_on_stderr(monkeypatch):
monkeypatch.setenv('HOME', __tests__ + '/testdata/azure')
monkeypatch.setenv('PATH', __tests__ + '/testdata:/bin')
monkeypatch.setenv('WARN', 'this is a warning')
resource_id = '/subscriptions/123/resourceGroups/abc/providers/Microsoft.Databricks/workspaces/abc123'
cfg = Config(auth_type='azure-cli', host='x', azure_workspace_resource_id=resource_id)
assert 'X-Databricks-Azure-SP-Management-Token' in cfg.authenticate()
10 changes: 7 additions & 3 deletions tests/testdata/az
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
#!/bin/bash

if [ "yes" == "$FAIL" ]; then
if [ -n "$WARN" ]; then
>&2 /bin/echo "WARNING: ${WARN}"
fi

if [ "yes" == "$FAIL" ]; then
>&2 /bin/echo "This is just a failing script."
exit 1
fi

if [ "logout" == "$FAIL" ]; then
if [ "logout" == "$FAIL" ]; then
>&2 /bin/echo "No subscription found. Run 'az account set' to select a subscription."
exit 1
fi

if [ "corrupt" == "$FAIL" ]; then
if [ "corrupt" == "$FAIL" ]; then
/bin/echo "{accessToken: ..corrupt"
exit
fi
Expand Down

0 comments on commit d689d62

Please sign in to comment.