Skip to content

Commit 93b43f7

Browse files
djwaldoclaude
andcommitted
feat(thoughtspot): preserve obj_id, ThoughtSpot's portable object handle
`guid`, `fqn` and `obj_id` were grouped together as "instance-local identity never travels in a portable document". They are not the same kind of thing. `guid` is a raw cluster UUID. `fqn` is a reference to one -- and this repo's own schema reference already records that a viz-level `fqn` is DROPPED on import, leaving the object with no data source. Both are correctly refused. `obj_id` is the opposite: ThoughtSpot introduced it precisely so objects can be referenced across environments, and it survives import. It is a readable handle -- `SampleRetail-Apparel-LH-58435d2b`, the display name plus the GUID's first segment -- not a bare identifier. Discarding it meant a converted model re-imported as a NEW object beside the one it came from, rather than updating it, which breaks the promote-between-environments workflow this converter exists to serve. It is now stashed under its own payload key and restored at the document root. The forbidden-key scan still names `obj_id`, deliberately: that scan stops one riding along unnoticed inside a block copied wholesale from source TML, while the stash preserves it on purpose, under a distinct key. The two are not in conflict and the comment now says so. `guid` is still stripped at every depth, and the test asserts that. Observed on a real export while investigating why a converted model could not be re-imported: the source carried `obj_id: SampleRetail-Apparel-LH-58435d2b` and the converted document carried nothing. Mutation-checked. 928 tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent c070832 commit 93b43f7

7 files changed

Lines changed: 89 additions & 3 deletions

File tree

‎converters/thoughtspot/docs/vendor-payload.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ Every `custom_extensions` entry this converter writes uses `vendor_name` `THOUGH
3838
| Key | Scope | Classification | Treatment on the return trip |
3939
|---|---|---|---|
4040
| `tml_name` | Shared | shadows_derivable | Restored only if reconstructing it from the live document still agrees with the stashed value (self-verifying — no separate witness key); disagreement re-derives instead. |
41+
| `tml_obj_id` | Model | information_only | Restored as-is whenever present — nothing on the Ossie side could have diverged from it. |
4142
| `formula_id` | Field | information_only | Restored as-is whenever present — nothing on the Ossie side could have diverged from it. |
4243
| `db_column_name` | Field | shadows_derivable | Restored only if its witness companion key still matches the live document's current value; a mismatch means the document changed since the stash was written, so the value is re-derived instead. |
4344
| `data_type` | Field | shadows_derivable | Restored only if its witness companion key still matches the live document's current value; a mismatch means the document changed since the stash was written, so the value is re-derived instead. |

‎converters/thoughtspot/src/ossie_thoughtspot/constants.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,22 @@
9999
#: must agree on the exact spelling and nothing else enforces that.
100100
FIELD_STASH_DB_COLUMN_NAME = "db_column_name"
101101

102+
#: The source TML `obj_id` -- ThoughtSpot's own PORTABLE object handle, e.g.
103+
#: `SampleRetail-Apparel-LH-58435d2b` (display name, then the first segment of
104+
#: the GUID). Stashed under a distinct payload key so the forbidden-key scan,
105+
#: which looks for a literal `obj_id`, does not mistake this deliberate copy for
106+
#: leaked identity.
107+
#:
108+
#: Why this is kept when `guid` and `fqn` are not, though all three were once
109+
#: grouped as "instance-local identity": they are not the same kind of thing.
110+
#: `guid` is a raw cluster UUID and `fqn` is a reference to one -- and a
111+
#: viz-level `fqn` is DROPPED on import, leaving the object with no data source.
112+
#: `obj_id` is the identifier ThoughtSpot introduced precisely so objects can be
113+
#: referenced across environments, and it survives import. Discarding it meant a
114+
#: converted model re-imported as a NEW object rather than updating the one it
115+
#: came from, which is the ordinary promote-between-environments workflow.
116+
MODEL_STASH_OBJ_ID = "tml_obj_id"
117+
102118
#: The source TML `formulas[].id` of a computed field or metric, stashed
103119
#: verbatim. TML's `formulas[].id` and `formulas[].name` are INDEPENDENT -- a
104120
#: formula renamed after creation keeps its original id -- but the Ossie -> TML
@@ -456,6 +472,8 @@ class StashKeyClass(Enum):
456472
# is no live value this can diverge FROM. That is also the point of keeping
457473
# it -- TML's id is independent of its name, so renaming the metric in Ossie
458474
# must NOT change the id, or every cross-reference written against it breaks.
475+
# Ossie has no object-identity concept, so nothing here can diverge from it.
476+
MODEL_STASH_OBJ_ID: StashKeyClass.INFORMATION_ONLY,
459477
FIELD_STASH_FORMULA_ID: StashKeyClass.INFORMATION_ONLY,
460478
FIELD_STASH_DB_COLUMN_NAME: StashKeyClass.SHADOWS_DERIVABLE,
461479
FIELD_STASH_DATA_TYPE: StashKeyClass.SHADOWS_DERIVABLE,

‎converters/thoughtspot/src/ossie_thoughtspot/ossie_to_thoughtspot.py‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
from . import datatypes, formula, identifiers, stash
7373
from .constants import (
7474
FIELD_STASH_FORMULA_ID,
75+
MODEL_STASH_OBJ_ID,
7576
DATASET_STASH_ALIAS,
7677
DATASET_STASH_CONNECTION_NAME,
7778
DATASET_STASH_SOURCE_PARTS,
@@ -2235,7 +2236,15 @@ def build_model(semantic_model: dict, tables: Sequence[TmlDocument], log: IssueL
22352236
# differently-scoped `joins_with[]` inside the same payload namespace.
22362237
body["joins_with"] = model_joins_with
22372238

2238-
return TmlDocument(kind="model", body=body, guid=None)
2239+
# `guid` is never restored -- it is raw cluster identity. `obj_id` is, and
2240+
# the difference is the point: it is the handle ThoughtSpot uses to
2241+
# recognise this as the SAME object on re-import, including in another
2242+
# environment, so keeping it is what makes the round trip an update
2243+
# rather than a duplicate.
2244+
return TmlDocument(
2245+
kind="model", body=body, guid=None,
2246+
obj_id=model_payload.get(MODEL_STASH_OBJ_ID),
2247+
)
22392248

22402249

22412250
# ---------------------------------------------------------------------------

‎converters/thoughtspot/src/ossie_thoughtspot/stash.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,13 @@
2727
from .errors import ConversionError
2828

2929
#: Instance-local identity never travels in a portable document.
30+
#:
31+
#: `obj_id` is in this scan but is NOT wholly forbidden: the scan stops it
32+
#: riding along unnoticed inside a block copied wholesale from source TML, while
33+
#: `constants.STASH_OBJ_ID` preserves it DELIBERATELY, under its own payload key.
34+
#: The three are not equivalent -- `guid` is a raw cluster UUID and `fqn` a
35+
#: reference to one, but `obj_id` is the handle ThoughtSpot introduced so objects
36+
#: can be referenced ACROSS environments, and unlike an `fqn` it survives import.
3037
_FORBIDDEN_KEYS = frozenset({"guid", "obj_id", "fqn"})
3138

3239

‎converters/thoughtspot/src/ossie_thoughtspot/tml.py‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,10 @@ class TmlDocument:
9393
kind: str
9494
body: dict
9595
guid: str | None
96+
#: ThoughtSpot's portable object handle. Read AND written, unlike `guid`:
97+
#: it is how a re-import updates the object it came from instead of
98+
#: creating a duplicate, and it survives moving between environments.
99+
obj_id: str | None = None
96100
source: str | None = None
97101

98102

@@ -132,7 +136,10 @@ def load_document(text: str, *, source: str | None = None) -> TmlDocument:
132136
body = data[kind]
133137
if not isinstance(body, dict):
134138
raise ConversionError(f"{source or '<input>'}: {kind} must be a mapping")
135-
return TmlDocument(kind=kind, body=body, guid=data.get("guid"), source=source)
139+
return TmlDocument(
140+
kind=kind, body=body, guid=data.get("guid"),
141+
obj_id=data.get("obj_id"), source=source,
142+
)
136143

137144

138145
def load_document_set(texts: Sequence[tuple[str, str]]) -> DocumentSet:
@@ -171,7 +178,11 @@ def _strip_nested_guids(value: object) -> object:
171178
def dump_document(document: TmlDocument) -> str:
172179
"""Serialise one document. `guid` is stripped unconditionally, at every depth of
173180
the body — not only at the document root."""
174-
return _yaml.dump({document.kind: _strip_nested_guids(document.body)})
181+
payload: dict = {}
182+
if document.obj_id:
183+
payload["obj_id"] = document.obj_id
184+
payload[document.kind] = _strip_nested_guids(document.body)
185+
return _yaml.dump(payload)
175186

176187

177188
def _safe_filename_component(name: object) -> str:

‎converters/thoughtspot/src/ossie_thoughtspot/tml_to_ossie.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@
128128
RELATIONSHIP_STASH_ON_EXPRESSION_WITNESS,
129129
RELATIONSHIP_STASH_REFERENCING_JOIN,
130130
RELATIONSHIP_STASH_TYPE,
131+
MODEL_STASH_OBJ_ID,
131132
STASH_TML_NAME,
132133
)
133134
from .errors import ConversionError
@@ -2022,6 +2023,10 @@ def convert(document_set: DocumentSet) -> OssieConversion:
20222023
)
20232024
semantic_model: dict = {"name": semantic_model_name, "datasets": []}
20242025
model_stash: dict = {}
2026+
# ThoughtSpot's portable object handle, so a re-import updates the model
2027+
# it came from rather than creating a duplicate beside it.
2028+
if document_set.model.obj_id:
2029+
model_stash[MODEL_STASH_OBJ_ID] = document_set.model.obj_id
20252030
if semantic_model_name != model_display_name:
20262031
model_stash[STASH_TML_NAME] = model_display_name
20272032

‎converters/thoughtspot/tests/test_roundtrip.py‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -922,3 +922,38 @@ def test_every_referencing_join_carries_the_compulsory_with_field():
922922
assert join.get("with"), (
923923
f"{fixture_name}: joins[] entry on {entry['name']!r} has no `with`: {join}"
924924
)
925+
926+
927+
def test_the_models_obj_id_survives_the_round_trip():
928+
"""`obj_id` is kept where `guid` and `fqn` are not, and the difference matters.
929+
930+
All three were once grouped as "instance-local identity". They are not the
931+
same kind of thing: `guid` is a raw cluster UUID and `fqn` a reference to
932+
one -- and a viz-level `fqn` is dropped on import anyway -- while `obj_id`
933+
(`SampleRetail-Apparel-LH-58435d2b`: display name, then the GUID's first
934+
segment) is the handle ThoughtSpot introduced so objects can be referenced
935+
ACROSS environments, and it survives import.
936+
937+
Discarding it meant a converted model re-imported as a NEW object rather
938+
than updating the one it came from -- which breaks the ordinary
939+
promote-between-environments workflow the converter exists to serve.
940+
"""
941+
table = TmlDocument(kind="table", guid=None, body={
942+
"name": "t", "db": "D", "schema": "S", "db_table": "T",
943+
"connection": {"name": "Conn"},
944+
"columns": [{"name": "a", "db_column_name": "A",
945+
"db_column_properties": {"data_type": "DOUBLE"}}]})
946+
model = TmlDocument(kind="model", guid="11111111-2222-3333-4444-555555555555",
947+
obj_id="SampleRetail-58435d2b", body={
948+
"name": "M", "model_tables": [{"name": "t"}],
949+
"columns": [{"name": "a", "column_id": "t::a",
950+
"properties": {"column_type": "ATTRIBUTE"}}]})
951+
ossie = tml_to_ossie.convert(DocumentSet(model=model, tables=(table,)))
952+
returned = ossie_to_thoughtspot.convert(ossie.model).documents.model
953+
assert returned.obj_id == "SampleRetail-58435d2b"
954+
955+
emitted = tml.dump_document(returned)
956+
assert "obj_id: SampleRetail-58435d2b" in emitted
957+
# The guid must still never travel.
958+
assert "11111111-2222" not in emitted
959+
assert "guid:" not in emitted

0 commit comments

Comments
 (0)