Skip to content

Commit f3ce281

Browse files
skido-cwclaude
authored andcommitted
refactor: address PR review feedback on blob add_elements/add_edges
- _run_blob_process: skip the defensive list() copy when rows is already a list/tuple (the common case), only materializing iterators/generators, to avoid doubling memory on large bulk builds. - add_elements / add_edges: annotate return as Optional[Response] (the blob path returns None). - add_elements_use_blob / add_edges_use_blob: add @require_version("11.4") for an earlier, clearer error, matching FileService.create's gating. - update_or_create_hierarchy_from_dataframe: gate the blob add/delete paths on admin rights AND TM1 >= 11.4 (Contents API), falling back to REST on older servers so the method stays backward compatible for admins on v11 < 11.4. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 71936b6 commit f3ce281

2 files changed

Lines changed: 16 additions & 7 deletions

File tree

TM1py/Services/ElementService.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,10 @@ def _run_blob_process(self, rows: Iterable[Iterable], build_process, remove_blob
320320
:param remove_blob: Whether to delete the staged blob after execution (default: True).
321321
:return: None
322322
"""
323-
rows = list(rows)
323+
# callers typically pass a materialized list/tuple; only consume an iterator/generator
324+
# (which we must materialize to test for emptiness) to avoid doubling memory on large builds
325+
if not isinstance(rows, (list, tuple)):
326+
rows = list(rows)
324327
if not rows:
325328
return
326329

@@ -1367,7 +1370,7 @@ def add_edges(
13671370
use_blob: bool = False,
13681371
remove_blob: bool = True,
13691372
**kwargs,
1370-
) -> Response:
1373+
) -> Optional[Response]:
13711374
"""Add Edges to hierarchy. Fails if one edge already exists.
13721375
13731376
:param dimension_name:
@@ -1400,6 +1403,7 @@ def add_edges(
14001403

14011404
@require_data_admin
14021405
@require_ops_admin
1406+
@require_version(version="11.4")
14031407
def add_edges_use_blob(
14041408
self,
14051409
dimension_name: str,
@@ -1459,7 +1463,7 @@ def add_elements(
14591463
use_blob: bool = False,
14601464
remove_blob: bool = True,
14611465
**kwargs,
1462-
):
1466+
) -> Optional[Response]:
14631467
"""Add elements to hierarchy. Fails if one element already exists.
14641468
14651469
:param dimension_name:
@@ -1486,6 +1490,7 @@ def add_elements(
14861490

14871491
@require_data_admin
14881492
@require_ops_admin
1493+
@require_version(version="11.4")
14891494
def add_elements_use_blob(
14901495
self,
14911496
dimension_name: str,

TM1py/Services/HierarchyService.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,10 @@ def update_or_create_hierarchy_from_dataframe(
594594

595595
hierarchy_exists = self.exists(dimension_name, hierarchy_name)
596596

597+
# the blob-based bulk add/delete paths need admin rights and the Contents API (TM1 >= 11.4);
598+
# on older servers (or for non-admins) fall back to the REST paths to stay backward compatible
599+
use_blob = self.is_admin and verify_version(required_version="11.4", version=self.version)
600+
597601
if not hierarchy_exists:
598602
existing_element_identifiers = CaseAndSpaceInsensitiveSet()
599603
else:
@@ -641,7 +645,7 @@ def update_or_create_hierarchy_from_dataframe(
641645
dimension_name=dimension_name,
642646
hierarchy_name=hierarchy_name,
643647
elements=(Element(element_name, element_type) for element_name, element_type in new_elements.items()),
644-
use_blob=self.is_admin,
648+
use_blob=use_blob,
645649
)
646650

647651
# define the attribute columns in df. Applies to all elements in df, not only new ones.
@@ -737,7 +741,7 @@ def update_or_create_hierarchy_from_dataframe(
737741
dimension_name=dimension_name,
738742
hierarchy_name=hierarchy_name,
739743
edges=edges_to_delete,
740-
use_blob=self.is_admin,
744+
use_blob=use_blob,
741745
)
742746

743747
edges = CaseAndSpaceInsensitiveTuplesDict()
@@ -776,7 +780,7 @@ def update_or_create_hierarchy_from_dataframe(
776780
dimension_name=dimension_name,
777781
hierarchy_name=hierarchy_name,
778782
edges=edges_to_delete.keys(),
779-
use_blob=self.is_admin,
783+
use_blob=use_blob,
780784
)
781785

782786
new_edges = {
@@ -787,7 +791,7 @@ def update_or_create_hierarchy_from_dataframe(
787791
dimension_name=dimension_name,
788792
hierarchy_name=hierarchy_name,
789793
edges=new_edges,
790-
use_blob=self.is_admin,
794+
use_blob=use_blob,
791795
)
792796

793797
if hierarchy_sort_order:

0 commit comments

Comments
 (0)