Skip to content

Commit 478659d

Browse files
jirkasemmlerclaude
andcommitted
address review comments
- raise a clear ValueError when login_type is used but the backend cannot be resolved (the API rejects loginType without backend) - cache the resolved default backend on the instance to avoid repeated tokens/verify calls - update stale detail()/reset_password() docstrings for key-pair workspaces - derive keypair_create_response mock from create_response - add tests for non-snowflake default backend, verify caching and the unresolvable-backend error Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 1164636 commit 478659d

3 files changed

Lines changed: 103 additions & 25 deletions

File tree

kbcstorage/workspaces.py

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ def __init__(self, root_url, token):
7474
token (:obj:`str`): A storage API key.
7575
"""
7676
super().__init__(root_url, 'workspaces', token)
77+
self._default_backend = None
7778

7879
def list(self):
7980
"""
@@ -91,8 +92,9 @@ def detail(self, workspace_id):
9192
"""
9293
Retrieves information about a given workspace.
9394
94-
Note that the password to the workspace can only be retrieved when the
95-
workspace is created.
95+
Note that the workspace credentials (password or private key,
96+
depending on the login type) are only available when the workspace
97+
is created and cannot be retrieved later.
9698
9799
Args:
98100
workspace_id (int or str): The id of the workspace.
@@ -138,6 +140,11 @@ def create(self, backend=None, timeout=None, login_type=None, public_key=None, r
138140
"""
139141
private_key = None
140142
effective_backend = backend or self._get_default_backend()
143+
if effective_backend is None and login_type is not None:
144+
raise ValueError(
145+
"Cannot resolve the project's default backend from the token; "
146+
"pass backend explicitly when using login_type."
147+
)
141148
if effective_backend == BACKEND_SNOWFLAKE:
142149
if login_type in (None, LOGIN_TYPE_DEFAULT):
143150
login_type = LOGIN_TYPE_SNOWFLAKE_SERVICE_KEYPAIR
@@ -163,9 +170,14 @@ def create(self, backend=None, timeout=None, login_type=None, public_key=None, r
163170
def _get_default_backend(self):
164171
"""
165172
Resolve the project's default backend from the token detail.
173+
174+
The value is cached on the instance - a project's default backend is
175+
effectively immutable for the client's lifetime.
166176
"""
167-
token_info = Tokens(self.root_url, self.token).verify()
168-
return (token_info.get('owner') or {}).get('defaultBackend')
177+
if self._default_backend is None:
178+
token_info = Tokens(self.root_url, self.token).verify()
179+
self._default_backend = (token_info.get('owner') or {}).get('defaultBackend')
180+
return self._default_backend
169181

170182
def delete(self, workspace_id):
171183
"""
@@ -187,6 +199,11 @@ def reset_password(self, workspace_id):
187199
"""
188200
Generate a new password for the workspace.
189201
202+
Only supported for password-based login types (e.g. the deprecated
203+
'snowflake-legacy-service'). For key-pair workspaces rotate the
204+
credentials with set_public_key() using a freshly generated key pair
205+
instead.
206+
190207
Args:
191208
workspace_id (int or str): The id of the workspace for which the
192209
password should be reset.

tests/mocks/test_workspaces.py

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""
22
Asses basic functionality of the Workspace endpoint.
33
"""
4+
import copy
45
import unittest
56
from urllib.parse import parse_qs
67

@@ -179,6 +180,84 @@ def test_create_snowflake_explicit_legacy_login_type(self):
179180
assert 'publicKey' not in request_body
180181
assert created_detail['connection']['password'] == 'abc'
181182

183+
@responses.activate
184+
def test_create_non_snowflake_default_backend_unchanged(self):
185+
"""
186+
When the project default backend resolved from the token is not
187+
snowflake, no login type, public key or backend is sent.
188+
"""
189+
bigquery_verify_response = copy.deepcopy(verify_token_response)
190+
bigquery_verify_response['owner']['defaultBackend'] = 'bigquery'
191+
responses.add(
192+
responses.Response(
193+
method='GET',
194+
url='https://connection.keboola.com/v2/storage/tokens/verify',
195+
json=bigquery_verify_response
196+
)
197+
)
198+
responses.add(
199+
responses.Response(
200+
method='POST',
201+
url='https://connection.keboola.com/v2/storage/workspaces',
202+
json=create_response
203+
)
204+
)
205+
self.ws.create()
206+
request_body = parse_qs(responses.calls[1].request.body)
207+
assert 'backend' not in request_body
208+
assert 'loginType' not in request_body
209+
assert 'publicKey' not in request_body
210+
211+
@responses.activate
212+
def test_create_caches_default_backend(self):
213+
"""
214+
The default backend is resolved via the token verify call only once
215+
per endpoint instance.
216+
"""
217+
responses.add(
218+
responses.Response(
219+
method='GET',
220+
url='https://connection.keboola.com/v2/storage/tokens/verify',
221+
json=verify_token_response
222+
)
223+
)
224+
responses.add(
225+
responses.Response(
226+
method='POST',
227+
url='https://connection.keboola.com/v2/storage/workspaces',
228+
json=keypair_create_response
229+
)
230+
)
231+
responses.add(
232+
responses.Response(
233+
method='POST',
234+
url='https://connection.keboola.com/v2/storage/workspaces',
235+
json=keypair_create_response
236+
)
237+
)
238+
self.ws.create()
239+
self.ws.create()
240+
verify_calls = [c for c in responses.calls if c.request.method == 'GET']
241+
assert len(verify_calls) == 1
242+
243+
@responses.activate
244+
def test_create_login_type_without_resolvable_backend_raises(self):
245+
"""
246+
The API rejects loginType without an explicit backend, so the client
247+
raises a clear error when the default backend cannot be resolved.
248+
"""
249+
no_backend_verify_response = copy.deepcopy(verify_token_response)
250+
del no_backend_verify_response['owner']['defaultBackend']
251+
responses.add(
252+
responses.Response(
253+
method='GET',
254+
url='https://connection.keboola.com/v2/storage/tokens/verify',
255+
json=no_backend_verify_response
256+
)
257+
)
258+
with self.assertRaises(ValueError):
259+
self.ws.create(login_type='none')
260+
182261
@responses.activate
183262
def test_create_non_snowflake_backend_unchanged(self):
184263
"""

tests/mocks/workspace_responses.py

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -76,29 +76,11 @@
7676
}
7777

7878
keypair_create_response = {
79-
"id": 235,
80-
"name": "boring_wozniak",
81-
"type": "table",
82-
"component": "wr-db",
83-
"configurationId": "aws-1",
84-
"created": "2016-05-17T11:11:20+0200",
79+
**create_response,
8580
"connection": {
86-
"backend": "snowflake",
87-
"host": "keboola.snowflakecomputing.com",
88-
"database": "keboola_123",
89-
"schema": "boring_wozniak",
90-
"warehouse": "SAPI_PROD",
91-
"user": "xzy",
92-
"loginType": "snowflake-service-keypair"
93-
},
94-
"creatorToken": {
95-
"id": 234,
96-
"description": "martin@keboola.com"
81+
**{k: v for k, v in create_response["connection"].items() if k != "password"},
82+
"loginType": "snowflake-service-keypair",
9783
},
98-
"creatorUser": {
99-
"id": 234,
100-
"name": "Martin"
101-
}
10284
}
10385

10486
load_tables_response = {

0 commit comments

Comments
 (0)