Skip to content

Commit 84cd7aa

Browse files
committed
fix(sdk): make the v0 deprecation markers actually fire
Three of them did not, and the CHANGELOG claimed things the diff does not do. Python read the public `use_sync_http` flag as "this instance is an internal transport, stay quiet". It is a documented option on `AsyncDstackClientV0`, so a caller who set it themselves was silently opted out of the one signal that says the surface is frozen. The sync wrappers now pass a private `_warn=False` instead, which is what they actually mean. Go's `ToEthereumAccount` and `ToSolanaKeypair` still carried their `// Deprecated:` inside the first comment paragraph, where neither gopls nor pkg.go.dev recognises it -- the same shape this branch repairs five times over in `client_v0.go`, left on the two functions whose own doc comments say they have security concerns. The JSDoc on the `DstackClient` alias told readers to import `DstackClientV0` from `./client-v0`. That text ships in `dist/index.d.ts` and the path does not resolve for a package consumer: `client-v0` is not a `tsup` entry and not in the `exports` map. Point at the package root. CHANGELOG corrections: Rust warns at every *mention of the type*, not at every call -- `#[deprecated]` on a struct does not propagate to its inherent methods, and a client received from a factory function warns nowhere. `TappdClient`'s `@deprecated` JSDoc is added here, not pre-existing. And "no behaviour changes" was wrong for Python: the warning fires at construction, so a downstream suite with `filterwarnings = error` goes red on upgrade. Say so where they will read it.
1 parent 88c75c6 commit 84cd7aa

6 files changed

Lines changed: 47 additions & 11 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
6262
- sdk: the JavaScript v1 `issueCert` response no longer carries a raw-bytes accessor. `asUint8Array()` is **removed rather than renamed**: it existed to feed the private key into the blockchain adapters, and v1 has no chain-flavoured surface. `IssueCert` returns TLS material, PEM is the form a TLS stack takes, and a caller who genuinely needs DER converts it with a standard library. The Rust, Python and Go v1 clients already returned the PEM string and the chain alone, so all four now agree. v0's `GetTlsKeyResponse.asUint8Array` is untouched — released API, and the viem and solana adapters depend on its truncating behaviour
6363
- sdk: the JavaScript v1 GPU evidence bundle's `asUint8Array()` is renamed `decodeEvidence()`, matching Python's and Rust's `decode_evidence` and Go, which hands back the decoded `Evidence` bytes directly. The name now says what the bytes are — the vendor's evidence, hex off the wire and decoded byte-exact, because sha256 over precisely those bytes is what the measured `gpu-attestation` event commits to
6464
- sdk: the v0 modules carry a `_v0` suffix, so the file a reader opens matches the client it holds. Rust's `dstack_sdk::dstack_client` becomes `dstack_sdk::dstack_client_v0` and `dstack_sdk_types::dstack` becomes `dstack_sdk_types::dstack_v0`; Python's `dstack_sdk.dstack_client` becomes `dstack_sdk.dstack_client_v0`; Go's `client.go`/`client_test.go` become `client_v0.go`/`client_v0_test.go`; and the JavaScript `index.ts`, which held both surfaces in one file, splits into `client-v0.ts`, `client-v1.ts` and a `shared.ts`, leaving `index.ts` as a barrel that re-exports exactly the names it always did. Until now the unsuffixed *file* meant v0 while the unsuffixed *class* meant v1, so a reader opening `dstack_client.rs` for the recommended client found the legacy one instead. **There are deliberately no backward-compat module aliases**: 0.6.0 is the loud-break release, and an import of an old module path fails at build time rather than silently binding the frozen surface under a name that now means something else. Package-level exports are untouched in every SDK — `dstack_sdk::DstackClient`, `from dstack_sdk import DstackClientV0` and `@phala/dstack-sdk`'s public surface are exactly what they were; only a deep import of the module path moves. In Go this is file naming alone, since it is all one `package dstack`
65-
- sdk: the v0 clients are deprecated in the way each language's tooling understands, not only in prose. Rust's `DstackClientV0` and `TappdClient` carry `#[deprecated(since = "0.6.0")]`, so a downstream build warns at the `use` and at every call; Python's `DstackClientV0` and `AsyncDstackClientV0` emit a `DeprecationWarning` on construction, through the same helper `TappdClient` already used, alongside the `.. deprecated:: 0.6.0` docstring note they already carried. Go's `// Deprecated:` markers and JavaScript's `@deprecated` JSDoc were already in place; a few Go ones sat mid-comment rather than as their own trailing paragraph, which is the only form the tooling recognises, and are repaired. Nothing is removed and no behaviour changes — the frozen surface stays reachable under its explicit name, it just says what it is at build time now
65+
- sdk: the v0 clients are deprecated in the way each language's tooling understands, not only in prose. Rust's `DstackClientV0` and `TappdClient` carry `#[deprecated(since = "0.6.0")]`, so a downstream build warns at every mention of the type — the `use`, the constructor, any signature naming it. Method calls on an already-built client stay silent, because Rust does not propagate the attribute to inherent methods. Python's `DstackClientV0` and `AsyncDstackClientV0` emit a `DeprecationWarning` on construction, through the same helper `TappdClient` already used, alongside the `.. deprecated:: 0.6.0` docstring note they already carried. JavaScript's `DstackClientV0` already had its `@deprecated` JSDoc and `TappdClient` gains one. Go's `// Deprecated:` markers were in place but seven sat mid-comment rather than as their own trailing paragraph, which is the only form gopls and pkg.go.dev recognise, and are repaired.
66+
67+
Nothing is removed and the wire behaviour is unchanged, but Python's marker is a runtime warning rather than a build-time one: a downstream test suite that turns `DeprecationWarning` into an error (`filterwarnings = error`, which is a common setting) will fail on `DstackClientV0()` until it adds a filter. The frozen surface stays reachable under its explicit name; it just says what it is now
6668

6769

6870
### Removed

sdk/go/dstack/ethereum.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ type EthereumAccount struct {
2323
}
2424

2525
// ToEthereumAccount creates an Ethereum account from GetKeyResponse or GetTlsKeyResponse (legacy method).
26+
//
2627
// Deprecated: Use ToEthereumAccountSecure instead. This method has security concerns.
2728
func ToEthereumAccount(keyResponse interface{}) (*EthereumAccount, error) {
2829
switch resp := keyResponse.(type) {

sdk/go/dstack/solana.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ type SolanaKeypair struct {
2020
}
2121

2222
// ToSolanaKeypair creates a Solana keypair from GetKeyResponse or GetTlsKeyResponse (legacy method).
23+
//
2324
// Deprecated: Use ToSolanaKeypairSecure instead. This method has security concerns.
2425
func ToSolanaKeypair(keyResponse interface{}) (*SolanaKeypair, error) {
2526
switch resp := keyResponse.(type) {

sdk/js/src/client-v1.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,8 @@ export class DstackClientV1 {
349349
* changing the name fails loudly rather than quietly deriving different keys:
350350
* the v1 signatures differ, and `getKey` requires `algorithm` explicitly, so a
351351
* v0 call site stops compiling (or throws) instead of returning wrong material.
352-
* To stay on the frozen surface, name `DstackClientV0` from `./client-v0`.
352+
* To stay on the frozen surface, import `DstackClientV0` by name from the
353+
* package root -- `client-v0` is an internal module, not an export path.
353354
*/
354355
export const DstackClient = DstackClientV1
355356
export type DstackClient = DstackClientV1

sdk/python/src/dstack_sdk/dstack_client_v0.py

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -419,13 +419,20 @@ def __init__(
419419
*,
420420
use_sync_http: bool = False,
421421
timeout: float = 3,
422+
_warn: bool = True,
422423
):
423-
"""Initialize the legacy async client, warning that v0 is deprecated."""
424-
# Only when this class is what the caller actually asked for. A
424+
"""Initialize the legacy async client, warning that v0 is deprecated.
425+
426+
``_warn`` is private: the sync wrappers build one of these as their own
427+
transport and have already warned, so they pass ``False``. It is a
428+
separate flag rather than a reading of ``use_sync_http`` because that
429+
one is public and documented -- a caller who sets it is still a caller
430+
who deserves the warning.
431+
"""
432+
# Only when this class is what the caller actually asked for: a
425433
# subclass (``AsyncTappdClient``) names its own surface in its own
426-
# warning, and ``use_sync_http`` means this instance is the transport
427-
# behind ``DstackClientV0``, which has already warned.
428-
if type(self) is AsyncDstackClientV0 and not use_sync_http:
434+
# warning.
435+
if _warn and type(self) is AsyncDstackClientV0:
429436
emit_deprecation_warning(
430437
"AsyncDstackClientV0 is deprecated: the v0 surface is frozen at "
431438
"dstack 0.5.11. Use AsyncDstackClient (AsyncDstackClientV1), "
@@ -660,7 +667,7 @@ def __init__(self, endpoint: str | None = None, *, timeout: float = 3):
660667
"derives different key material -- see docs/guest-api-v1.md"
661668
)
662669
self.async_client = AsyncDstackClientV0(
663-
endpoint, use_sync_http=True, timeout=timeout
670+
endpoint, use_sync_http=True, timeout=timeout, _warn=False
664671
)
665672

666673
@call_async
@@ -777,10 +784,11 @@ def __init__(
777784
*,
778785
use_sync_http: bool = False,
779786
timeout: float = 3,
787+
_warn: bool = True,
780788
):
781789
"""Initialize deprecated async tappd client wrapper."""
782-
if not use_sync_http:
783-
# Already warned in TappdClient.__init__
790+
if _warn:
791+
# ``TappdClient`` has already warned when it builds one of these.
784792
emit_deprecation_warning(
785793
"AsyncTappdClient is deprecated, please use AsyncDstackClientV0 instead"
786794
)
@@ -863,7 +871,7 @@ def __init__(self, endpoint: str | None = None, timeout: float = 3):
863871
)
864872
endpoint = get_tappd_endpoint(endpoint)
865873
self.async_client = AsyncTappdClient(
866-
endpoint, use_sync_http=True, timeout=timeout
874+
endpoint, use_sync_http=True, timeout=timeout, _warn=False
867875
)
868876

869877
@call_async

sdk/python/tests/test_client.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -697,3 +697,26 @@ async def fake_send(self, method, payload):
697697
client = AsyncDstackClientV0()
698698
with pytest.raises(RuntimeError, match="TLS key options"):
699699
await client.get_tls_key(with_app_info=False)
700+
701+
702+
def test_v0_warns_even_when_the_caller_asks_for_sync_http():
703+
"""``use_sync_http`` is a public transport option, not a warning switch.
704+
705+
The sync wrappers build their async twin with it, and used to suppress the
706+
deprecation warning by reading it -- so a user who set the documented flag
707+
themselves was silently opted out of the one signal telling them the surface
708+
is frozen.
709+
"""
710+
with pytest.warns(DeprecationWarning, match="AsyncDstackClientV0 is deprecated"):
711+
AsyncDstackClientV0(use_sync_http=True)
712+
713+
714+
def test_v0_sync_wrapper_warns_exactly_once():
715+
"""It builds an AsyncDstackClientV0 internally; that must not warn twice."""
716+
with warnings.catch_warnings(record=True) as caught:
717+
warnings.simplefilter("always")
718+
DstackClientV0()
719+
v0_warnings = [
720+
w for w in caught if "DstackClientV0 is deprecated" in str(w.message)
721+
]
722+
assert len(v0_warnings) == 1, [str(w.message) for w in caught]

0 commit comments

Comments
 (0)