Skip to content

Improvement #26033: StorageServices missing form the entities SDK in the python client#26164

Merged
harshach merged 8 commits intomainfrom
26033
Mar 2, 2026
Merged

Improvement #26033: StorageServices missing form the entities SDK in the python client#26164
harshach merged 8 commits intomainfrom
26033

Conversation

@harshach
Copy link
Copy Markdown
Collaborator

@harshach harshach commented Feb 28, 2026

Describe your changes

Fixes #26033.

This PR includes two related SDK parity improvements requested in the issue thread:

  1. Python SDK StorageServices exposure
  • Add StorageServices entity facade in metadata.sdk.entities.
  • Export StorageServices from both metadata.sdk.entities and top-level metadata.sdk.
  1. Table fluent operations parity across Python and Java
  • Python: add Tables.add_custom_metric, Tables.add_sample_data, and Tables.get_sample_data.
  • Java: add table fluent operations for sample data and custom metrics.
  • Ensure fluent behavior preserves pending changes before server-side table operations.

Why both are in this PR:

  • The issue discussion expanded from missing StorageServices export to SDK fluent parity for table operations across both Python and Java.
  • These changes touch the same SDK surface area and were reviewed together to keep API behavior aligned.

How this was tested

Python:

  • Unit tests for StorageServices exports and retrieval.
  • Unit tests for table custom metric + sample data operations.
  • Updated table sample-data tests to validate lightweight table reference values/types.

Java:

  • Unit tests for table operations and fluent wrapper behavior.
  • Added init test to verify StorageServices fluent client registration.

Type of change

  • Bug fix
  • Improvement
  • New feature
  • Breaking change
  • Documentation

Checklist

  • I have read the CONTRIBUTING document.
  • I have added tests around the new logic.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses missing StorageServices exposure in the Python SDK entities module (per #26033), and also adds/extends Table-related fluent operations (sample data + custom metrics) across the Java SDK and Python SDK, with accompanying unit tests.

Changes:

  • Export StorageServices from metadata.sdk.entities and metadata.sdk (Python), with unit tests verifying import paths and fluent retrieval.
  • Add Table sample data retrieval plus fluent helpers for sample data and custom metrics (Java + Python), with new/updated unit tests.
  • Register Java fluent StorageServices in OM.init(...) and add an init test to validate default client wiring.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
openmetadata-sdk/src/test/java/org/openmetadata/sdk/entities/TableEntityOperationsTest.java Adds unit tests for new Tables.forTable(...).add/getSampleData() and custom metric fluent operations.
openmetadata-sdk/src/test/java/org/openmetadata/sdk/OMStorageServicesInitTest.java Verifies OM.init registers the fluent StorageServices default client.
openmetadata-sdk/src/main/java/org/openmetadata/sdk/services/dataassets/TableService.java Adds getSampleData(...) GET endpoint wrapper.
openmetadata-sdk/src/main/java/org/openmetadata/sdk/fluent/wrappers/FluentTable.java Adds fluent wrapper methods for sample data and custom metrics on a table instance.
openmetadata-sdk/src/main/java/org/openmetadata/sdk/fluent/Tables.java Adds static helpers and a forTable(...) operations wrapper for sample data/custom metrics.
openmetadata-sdk/src/main/java/org/openmetadata/sdk/OM.java Registers fluent StorageServices default client during SDK init.
ingestion/tests/unit/sdk/test_table_entity.py Adds unit tests for Python Tables.add_custom_metric, add_sample_data, and get_sample_data.
ingestion/tests/unit/sdk/test_sdk_plural_entities.py Extends plural-entity assertions to include StorageServices.
ingestion/tests/unit/sdk/test_sdk_fluent_api.py Adds tests for StorageServices.retrieve and for exports from metadata.sdk.entities.
ingestion/src/metadata/sdk/entities/tables.py Adds Python Tables helpers for custom metrics and sample data operations.
ingestion/src/metadata/sdk/entities/storage_services.py Introduces StorageServices entity facade in the Python SDK.
ingestion/src/metadata/sdk/entities/init.py Exports StorageServices from the entities package.
ingestion/src/metadata/sdk/init.py Exposes StorageServices at top-level SDK and adds lowercase alias + __all__ entries.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Feb 28, 2026

🛡️ TRIVY SCAN RESULT 🛡️

Target: openmetadata-ingestion-base-slim:trivy (debian 12.13)

No Vulnerabilities Found

🛡️ TRIVY SCAN RESULT 🛡️

Target: Java

Vulnerabilities (37)

Package Vulnerability ID Severity Installed Version Fixed Version
com.fasterxml.jackson.core:jackson-core CVE-2025-52999 🚨 HIGH 2.12.7 2.15.0
com.fasterxml.jackson.core:jackson-core GHSA-72hv-8253-57qq 🚨 HIGH 2.12.7 2.18.6, 2.21.1, 3.1.0
com.fasterxml.jackson.core:jackson-core CVE-2025-52999 🚨 HIGH 2.13.4 2.15.0
com.fasterxml.jackson.core:jackson-core GHSA-72hv-8253-57qq 🚨 HIGH 2.13.4 2.18.6, 2.21.1, 3.1.0
com.fasterxml.jackson.core:jackson-core GHSA-72hv-8253-57qq 🚨 HIGH 2.15.2 2.18.6, 2.21.1, 3.1.0
com.fasterxml.jackson.core:jackson-databind CVE-2022-42003 🚨 HIGH 2.12.7 2.12.7.1, 2.13.4.2
com.fasterxml.jackson.core:jackson-databind CVE-2022-42004 🚨 HIGH 2.12.7 2.12.7.1, 2.13.4
com.google.code.gson:gson CVE-2022-25647 🚨 HIGH 2.2.4 2.8.9
com.google.protobuf:protobuf-java CVE-2021-22569 🚨 HIGH 3.3.0 3.16.1, 3.18.2, 3.19.2
com.google.protobuf:protobuf-java CVE-2022-3509 🚨 HIGH 3.3.0 3.16.3, 3.19.6, 3.20.3, 3.21.7
com.google.protobuf:protobuf-java CVE-2022-3510 🚨 HIGH 3.3.0 3.16.3, 3.19.6, 3.20.3, 3.21.7
com.google.protobuf:protobuf-java CVE-2024-7254 🚨 HIGH 3.3.0 3.25.5, 4.27.5, 4.28.2
com.google.protobuf:protobuf-java CVE-2021-22569 🚨 HIGH 3.7.1 3.16.1, 3.18.2, 3.19.2
com.google.protobuf:protobuf-java CVE-2022-3509 🚨 HIGH 3.7.1 3.16.3, 3.19.6, 3.20.3, 3.21.7
com.google.protobuf:protobuf-java CVE-2022-3510 🚨 HIGH 3.7.1 3.16.3, 3.19.6, 3.20.3, 3.21.7
com.google.protobuf:protobuf-java CVE-2024-7254 🚨 HIGH 3.7.1 3.25.5, 4.27.5, 4.28.2
com.nimbusds:nimbus-jose-jwt CVE-2023-52428 🚨 HIGH 9.8.1 9.37.2
com.squareup.okhttp3:okhttp CVE-2021-0341 🚨 HIGH 3.12.12 4.9.2
commons-beanutils:commons-beanutils CVE-2025-48734 🚨 HIGH 1.9.4 1.11.0
commons-io:commons-io CVE-2024-47554 🚨 HIGH 2.8.0 2.14.0
dnsjava:dnsjava CVE-2024-25638 🚨 HIGH 2.1.7 3.6.0
io.airlift:aircompressor CVE-2025-67721 🚨 HIGH 0.27 2.0.3
io.netty:netty-codec-http2 CVE-2025-55163 🚨 HIGH 4.1.96.Final 4.2.4.Final, 4.1.124.Final
io.netty:netty-codec-http2 GHSA-xpw8-rcwv-8f8p 🚨 HIGH 4.1.96.Final 4.1.100.Final
io.netty:netty-handler CVE-2025-24970 🚨 HIGH 4.1.96.Final 4.1.118.Final
net.minidev:json-smart CVE-2021-31684 🚨 HIGH 1.3.2 1.3.3, 2.4.4
net.minidev:json-smart CVE-2023-1370 🚨 HIGH 1.3.2 2.4.9
org.apache.avro:avro CVE-2024-47561 🔥 CRITICAL 1.7.7 1.11.4
org.apache.avro:avro CVE-2023-39410 🚨 HIGH 1.7.7 1.11.3
org.apache.derby:derby CVE-2022-46337 🔥 CRITICAL 10.14.2.0 10.14.3, 10.15.2.1, 10.16.1.2, 10.17.1.0
org.apache.ivy:ivy CVE-2022-46751 🚨 HIGH 2.5.1 2.5.2
org.apache.mesos:mesos CVE-2018-1330 🚨 HIGH 1.4.3 1.6.0
org.apache.thrift:libthrift CVE-2019-0205 🚨 HIGH 0.12.0 0.13.0
org.apache.thrift:libthrift CVE-2020-13949 🚨 HIGH 0.12.0 0.14.0
org.apache.zookeeper:zookeeper CVE-2023-44981 🔥 CRITICAL 3.6.3 3.7.2, 3.8.3, 3.9.1
org.eclipse.jetty:jetty-server CVE-2024-13009 🚨 HIGH 9.4.56.v20240826 9.4.57.v20241219
org.lz4:lz4-java CVE-2025-12183 🚨 HIGH 1.8.0 1.8.1

🛡️ TRIVY SCAN RESULT 🛡️

Target: Node.js

No Vulnerabilities Found

🛡️ TRIVY SCAN RESULT 🛡️

Target: Python

Vulnerabilities (9)

Package Vulnerability ID Severity Installed Version Fixed Version
apache-airflow CVE-2025-68438 🚨 HIGH 3.1.5 3.1.6
apache-airflow CVE-2025-68675 🚨 HIGH 3.1.5 3.1.6, 2.11.1
cryptography CVE-2026-26007 🚨 HIGH 42.0.8 46.0.5
jaraco.context CVE-2026-23949 🚨 HIGH 6.0.1 6.1.0
starlette CVE-2025-62727 🚨 HIGH 0.48.0 0.49.1
urllib3 CVE-2025-66418 🚨 HIGH 1.26.20 2.6.0
urllib3 CVE-2025-66471 🚨 HIGH 1.26.20 2.6.0
urllib3 CVE-2026-21441 🚨 HIGH 1.26.20 2.6.3
wheel CVE-2026-24049 🚨 HIGH 0.45.1 0.46.2

🛡️ TRIVY SCAN RESULT 🛡️

Target: /etc/ssl/private/ssl-cert-snakeoil.key

No Vulnerabilities Found

🛡️ TRIVY SCAN RESULT 🛡️

Target: /ingestion/pipelines/extended_sample_data.yaml

No Vulnerabilities Found

🛡️ TRIVY SCAN RESULT 🛡️

Target: /ingestion/pipelines/lineage.yaml

No Vulnerabilities Found

🛡️ TRIVY SCAN RESULT 🛡️

Target: /ingestion/pipelines/sample_data.json

No Vulnerabilities Found

🛡️ TRIVY SCAN RESULT 🛡️

Target: /ingestion/pipelines/sample_data.yaml

No Vulnerabilities Found

🛡️ TRIVY SCAN RESULT 🛡️

Target: /ingestion/pipelines/sample_data_aut.yaml

No Vulnerabilities Found

🛡️ TRIVY SCAN RESULT 🛡️

Target: /ingestion/pipelines/sample_usage.json

No Vulnerabilities Found

🛡️ TRIVY SCAN RESULT 🛡️

Target: /ingestion/pipelines/sample_usage.yaml

No Vulnerabilities Found

🛡️ TRIVY SCAN RESULT 🛡️

Target: /ingestion/pipelines/sample_usage_aut.yaml

No Vulnerabilities Found

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Feb 28, 2026

🛡️ TRIVY SCAN RESULT 🛡️

Target: openmetadata-ingestion:trivy (debian 12.12)

Vulnerabilities (4)

Package Vulnerability ID Severity Installed Version Fixed Version
libpam-modules CVE-2025-6020 🚨 HIGH 1.5.2-6+deb12u1 1.5.2-6+deb12u2
libpam-modules-bin CVE-2025-6020 🚨 HIGH 1.5.2-6+deb12u1 1.5.2-6+deb12u2
libpam-runtime CVE-2025-6020 🚨 HIGH 1.5.2-6+deb12u1 1.5.2-6+deb12u2
libpam0g CVE-2025-6020 🚨 HIGH 1.5.2-6+deb12u1 1.5.2-6+deb12u2

🛡️ TRIVY SCAN RESULT 🛡️

Target: Java

Vulnerabilities (38)

Package Vulnerability ID Severity Installed Version Fixed Version
com.fasterxml.jackson.core:jackson-core CVE-2025-52999 🚨 HIGH 2.12.7 2.15.0
com.fasterxml.jackson.core:jackson-core GHSA-72hv-8253-57qq 🚨 HIGH 2.12.7 2.18.6, 2.21.1, 3.1.0
com.fasterxml.jackson.core:jackson-core CVE-2025-52999 🚨 HIGH 2.13.4 2.15.0
com.fasterxml.jackson.core:jackson-core GHSA-72hv-8253-57qq 🚨 HIGH 2.13.4 2.18.6, 2.21.1, 3.1.0
com.fasterxml.jackson.core:jackson-core GHSA-72hv-8253-57qq 🚨 HIGH 2.15.2 2.18.6, 2.21.1, 3.1.0
com.fasterxml.jackson.core:jackson-core GHSA-72hv-8253-57qq 🚨 HIGH 2.16.1 2.18.6, 2.21.1, 3.1.0
com.fasterxml.jackson.core:jackson-databind CVE-2022-42003 🚨 HIGH 2.12.7 2.12.7.1, 2.13.4.2
com.fasterxml.jackson.core:jackson-databind CVE-2022-42004 🚨 HIGH 2.12.7 2.12.7.1, 2.13.4
com.google.code.gson:gson CVE-2022-25647 🚨 HIGH 2.2.4 2.8.9
com.google.protobuf:protobuf-java CVE-2021-22569 🚨 HIGH 3.3.0 3.16.1, 3.18.2, 3.19.2
com.google.protobuf:protobuf-java CVE-2022-3509 🚨 HIGH 3.3.0 3.16.3, 3.19.6, 3.20.3, 3.21.7
com.google.protobuf:protobuf-java CVE-2022-3510 🚨 HIGH 3.3.0 3.16.3, 3.19.6, 3.20.3, 3.21.7
com.google.protobuf:protobuf-java CVE-2024-7254 🚨 HIGH 3.3.0 3.25.5, 4.27.5, 4.28.2
com.google.protobuf:protobuf-java CVE-2021-22569 🚨 HIGH 3.7.1 3.16.1, 3.18.2, 3.19.2
com.google.protobuf:protobuf-java CVE-2022-3509 🚨 HIGH 3.7.1 3.16.3, 3.19.6, 3.20.3, 3.21.7
com.google.protobuf:protobuf-java CVE-2022-3510 🚨 HIGH 3.7.1 3.16.3, 3.19.6, 3.20.3, 3.21.7
com.google.protobuf:protobuf-java CVE-2024-7254 🚨 HIGH 3.7.1 3.25.5, 4.27.5, 4.28.2
com.nimbusds:nimbus-jose-jwt CVE-2023-52428 🚨 HIGH 9.8.1 9.37.2
com.squareup.okhttp3:okhttp CVE-2021-0341 🚨 HIGH 3.12.12 4.9.2
commons-beanutils:commons-beanutils CVE-2025-48734 🚨 HIGH 1.9.4 1.11.0
commons-io:commons-io CVE-2024-47554 🚨 HIGH 2.8.0 2.14.0
dnsjava:dnsjava CVE-2024-25638 🚨 HIGH 2.1.7 3.6.0
io.airlift:aircompressor CVE-2025-67721 🚨 HIGH 0.27 2.0.3
io.netty:netty-codec-http2 CVE-2025-55163 🚨 HIGH 4.1.96.Final 4.2.4.Final, 4.1.124.Final
io.netty:netty-codec-http2 GHSA-xpw8-rcwv-8f8p 🚨 HIGH 4.1.96.Final 4.1.100.Final
io.netty:netty-handler CVE-2025-24970 🚨 HIGH 4.1.96.Final 4.1.118.Final
net.minidev:json-smart CVE-2021-31684 🚨 HIGH 1.3.2 1.3.3, 2.4.4
net.minidev:json-smart CVE-2023-1370 🚨 HIGH 1.3.2 2.4.9
org.apache.avro:avro CVE-2024-47561 🔥 CRITICAL 1.7.7 1.11.4
org.apache.avro:avro CVE-2023-39410 🚨 HIGH 1.7.7 1.11.3
org.apache.derby:derby CVE-2022-46337 🔥 CRITICAL 10.14.2.0 10.14.3, 10.15.2.1, 10.16.1.2, 10.17.1.0
org.apache.ivy:ivy CVE-2022-46751 🚨 HIGH 2.5.1 2.5.2
org.apache.mesos:mesos CVE-2018-1330 🚨 HIGH 1.4.3 1.6.0
org.apache.thrift:libthrift CVE-2019-0205 🚨 HIGH 0.12.0 0.13.0
org.apache.thrift:libthrift CVE-2020-13949 🚨 HIGH 0.12.0 0.14.0
org.apache.zookeeper:zookeeper CVE-2023-44981 🔥 CRITICAL 3.6.3 3.7.2, 3.8.3, 3.9.1
org.eclipse.jetty:jetty-server CVE-2024-13009 🚨 HIGH 9.4.56.v20240826 9.4.57.v20241219
org.lz4:lz4-java CVE-2025-12183 🚨 HIGH 1.8.0 1.8.1

🛡️ TRIVY SCAN RESULT 🛡️

Target: Node.js

No Vulnerabilities Found

🛡️ TRIVY SCAN RESULT 🛡️

Target: Python

Vulnerabilities (21)

Package Vulnerability ID Severity Installed Version Fixed Version
Werkzeug CVE-2024-34069 🚨 HIGH 2.2.3 3.0.3
aiohttp CVE-2025-69223 🚨 HIGH 3.12.12 3.13.3
aiohttp CVE-2025-69223 🚨 HIGH 3.13.2 3.13.3
apache-airflow CVE-2025-68438 🚨 HIGH 3.1.5 3.1.6
apache-airflow CVE-2025-68675 🚨 HIGH 3.1.5 3.1.6, 2.11.1
azure-core CVE-2026-21226 🚨 HIGH 1.37.0 1.38.0
cryptography CVE-2026-26007 🚨 HIGH 42.0.8 46.0.5
google-cloud-aiplatform CVE-2026-2472 🚨 HIGH 1.130.0 1.131.0
google-cloud-aiplatform CVE-2026-2473 🚨 HIGH 1.130.0 1.133.0
jaraco.context CVE-2026-23949 🚨 HIGH 5.3.0 6.1.0
jaraco.context CVE-2026-23949 🚨 HIGH 6.0.1 6.1.0
protobuf CVE-2026-0994 🚨 HIGH 4.25.8 6.33.5, 5.29.6
pyasn1 CVE-2026-23490 🚨 HIGH 0.6.1 0.6.2
python-multipart CVE-2026-24486 🚨 HIGH 0.0.20 0.0.22
ray CVE-2025-62593 🔥 CRITICAL 2.47.1 2.52.0
starlette CVE-2025-62727 🚨 HIGH 0.48.0 0.49.1
urllib3 CVE-2025-66418 🚨 HIGH 1.26.20 2.6.0
urllib3 CVE-2025-66471 🚨 HIGH 1.26.20 2.6.0
urllib3 CVE-2026-21441 🚨 HIGH 1.26.20 2.6.3
wheel CVE-2026-24049 🚨 HIGH 0.45.1 0.46.2
wheel CVE-2026-24049 🚨 HIGH 0.45.1 0.46.2

🛡️ TRIVY SCAN RESULT 🛡️

Target: usr/bin/docker

Vulnerabilities (4)

Package Vulnerability ID Severity Installed Version Fixed Version
stdlib CVE-2025-68121 🔥 CRITICAL v1.25.5 1.24.13, 1.25.7, 1.26.0-rc.3
stdlib CVE-2025-61726 🚨 HIGH v1.25.5 1.24.12, 1.25.6
stdlib CVE-2025-61728 🚨 HIGH v1.25.5 1.24.12, 1.25.6
stdlib CVE-2025-61730 🚨 HIGH v1.25.5 1.24.12, 1.25.6

🛡️ TRIVY SCAN RESULT 🛡️

Target: /etc/ssl/private/ssl-cert-snakeoil.key

No Vulnerabilities Found

🛡️ TRIVY SCAN RESULT 🛡️

Target: /home/airflow/openmetadata-airflow-apis/openmetadata_managed_apis.egg-info/PKG-INFO

No Vulnerabilities Found

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 1, 2026

The Python checkstyle failed.

Please run make py_format and py_format_check in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Python code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 1, 2026

The Python checkstyle failed.

Please run make py_format and py_format_check in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Python code formatting.

You can install the pre-commit hooks with make install_test precommit_install.


// ==================== Table Operations ====================

public static class TableOperations {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Quality: TableOperations methods return Table, preventing fluent chaining

The TableOperations inner class is accessed via the factory method Tables.forTable(id), which suggests fluent chaining like Tables.forTable(id).addSampleData(data).getSampleData(). However, addSampleData(), getSampleData(), and addCustomMetric() all return Table rather than TableOperations, making chaining impossible.

This contrasts with the FluentTable wrapper (in wrappers/FluentTable.java) which properly returns FluentTable for chaining.

If chaining is intended, the methods should return this (with the Table result stored internally). If chaining is not intended and these are just convenience one-shot wrappers, the class works fine as-is but the factory pattern (forTable()) may set misleading expectations for SDK users.

Suggested fix:

public TableOperations addSampleData(TableData sampleData) {
  this.lastResult = client.tables().updateSampleData(tableId, sampleData);
  return this;
}

public TableOperations addCustomMetric(CustomMetric customMetric) {
  this.lastResult = client.tables().updateCustomMetric(tableId, customMetric);
  return this;
}

public Table getResult() {
  return lastResult;
}

Was this helpful? React with 👍 / 👎

@gitar-bot
Copy link
Copy Markdown

gitar-bot bot commented Mar 2, 2026

Code Review 👍 Approved with suggestions 2 resolved / 4 findings

Clean SDK parity improvement for StorageServices and table operations. Two minor previous findings (synthetic FQN in logs, non-chainable TableOperations) remain unresolved but are low-impact quality concerns.

💡 Quality: Placeholder FQN in _build_table_reference produces misleading logs

📄 ingestion/src/metadata/sdk/entities/tables.py:106

The _build_table_reference method constructs a Table with fullyQualifiedName="sdk.table.reference" and name="sdk_table_reference". The underlying client methods (ingest_table_sample_data, get_sample_data) use table.fullyQualifiedName.root in warning/error log messages (e.g., "Error trying to PUT sample data for sdk.table.reference").

When failures occur, these placeholder values make it harder to identify which table caused the problem. Consider passing through the actual table_id value so that log messages are actionable, e.g., fullyQualifiedName=FullyQualifiedEntityName(root=f"sdk.ref.{table_id_value}").

Suggested fix
    @classmethod
    def _build_table_reference(cls, table_id: UuidLike) -> Table:
        """Build a lightweight table reference object for table-specific operations."""
        table_id_value = cls._stringify_identifier(table_id)
        return Table(
            id=Uuid(root=table_id_value),
            name=f"sdk_ref_{table_id_value[:8]}",
            fullyQualifiedName=FullyQualifiedEntityName(root=f"sdk.ref.{table_id_value}"),
            columns=[],
        )
💡 Quality: TableOperations methods return Table, preventing fluent chaining

📄 openmetadata-sdk/src/main/java/org/openmetadata/sdk/fluent/Tables.java:319 📄 openmetadata-sdk/src/main/java/org/openmetadata/sdk/fluent/Tables.java:184

The TableOperations inner class is accessed via the factory method Tables.forTable(id), which suggests fluent chaining like Tables.forTable(id).addSampleData(data).getSampleData(). However, addSampleData(), getSampleData(), and addCustomMetric() all return Table rather than TableOperations, making chaining impossible.

This contrasts with the FluentTable wrapper (in wrappers/FluentTable.java) which properly returns FluentTable for chaining.

If chaining is intended, the methods should return this (with the Table result stored internally). If chaining is not intended and these are just convenience one-shot wrappers, the class works fine as-is but the factory pattern (forTable()) may set misleading expectations for SDK users.

Suggested fix
    public TableOperations addSampleData(TableData sampleData) {
      this.lastResult = client.tables().updateSampleData(tableId, sampleData);
      return this;
    }

    public TableOperations addCustomMetric(CustomMetric customMetric) {
      this.lastResult = client.tables().updateCustomMetric(tableId, customMetric);
      return this;
    }

    public Table getResult() {
      return lastResult;
    }
✅ 2 resolved
Bug: FluentTable server-side ops silently discard unsaved local changes

📄 openmetadata-sdk/src/main/java/org/openmetadata/sdk/fluent/wrappers/FluentTable.java:253 📄 openmetadata-sdk/src/main/java/org/openmetadata/sdk/fluent/wrappers/FluentTable.java:265 📄 openmetadata-sdk/src/main/java/org/openmetadata/sdk/fluent/wrappers/FluentTable.java:277
The new addSampleData(), addCustomMetric(), and refreshSampleData() methods in FluentTable replace this.table with the server response and set modified = false. If a user has made local modifications (e.g., withDescription(), addTag()) that haven't been saved yet, those changes are silently lost.

Example of data loss:

new FluentTable(table, client)
    .withDescription("Updated description")  // modified = true, local change
    .addSampleData(sampleData)               // replaces this.table, modified = false
    .save()                                   // no-op because modified == false!
    .get();                                   // description change is lost

These methods should either:

  1. Call save() first if modified == true (flush pending changes before the server-side operation), or
  2. Throw IllegalStateException if modified == true to prevent silent data loss, or
  3. Re-apply local changes on top of the server response.
Performance: Python add_sample_data/get_sample_data make unnecessary retrieve call

📄 ingestion/src/metadata/sdk/entities/tables.py:91 📄 ingestion/src/metadata/sdk/entities/tables.py:100
Both add_sample_data() and get_sample_data() call cls.retrieve(table_id) to fetch the full Table entity before passing it to the underlying client methods. However, the mixin methods (ingest_table_sample_data and get_sample_data in table_mixin.py) only use table.id.root and table.fullyQualifiedName.root from the table object to construct the URL path.

This means every call to these SDK methods makes two HTTP round-trips (one retrieve + one actual operation) when only one is needed. The Java SDK equivalents (Tables.addSampleData, Tables.getSampleData) take the ID directly and make a single API call.

Consider constructing a minimal Table stub with just the id field set, or refactoring the underlying mixin methods to accept a table ID string directly, to avoid the unnecessary GET call.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@harshach harshach added the To release Will cherry-pick this PR into the release branch label Mar 2, 2026
@harshach harshach merged commit 48b6b7a into main Mar 2, 2026
62 of 68 checks passed
@harshach harshach deleted the 26033 branch March 2, 2026 15:04
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 2, 2026

Failed to cherry-pick changes to the 1.11.12 branch.
Please cherry-pick the changes manually.
You can find more details here.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Mar 2, 2026

chirag-madlani pushed a commit that referenced this pull request Mar 10, 2026
…the python client (#26164)

* StorageServices missing form the entities SDK in the python client

* fix(sdk): address table fluent review comments and pyright warning

* Fix checkstyle

* fix(sdk): align table reference typing and tests for CI

* test(sdk): isolate TestCaseMockTest default client state

* Fix pycheck

(cherry picked from commit 48b6b7a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend safe to test Add this label to run secure Github workflows on PRs To release Will cherry-pick this PR into the release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

StorageServices missing form the entities SDK in the python client.

2 participants