Copilot commented on code in PR #10861: URL: https://github.com/apache/gravitino/pull/10861#discussion_r3194322203
########## clients/client-python/tests/integration/test_supports_tags.py: ########## @@ -0,0 +1,416 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import typing as tp +from random import randint + +from gravitino import Catalog, GravitinoAdminClient, GravitinoClient, GravitinoMetalake +from gravitino.api.file.fileset import Fileset +from gravitino.api.model.model import Model +from gravitino.api.rel.table import Table +from gravitino.api.rel.table_catalog import TableCatalog +from gravitino.api.rel.types.types import Types +from gravitino.api.tag import Tag +from gravitino.api.tag.supports_tags import SupportsTags +from gravitino.dto.rel.column_dto import ColumnDTO +from gravitino.dto.rel.partitioning.identity_partitioning_dto import ( + IdentityPartitioningDTO, +) +from gravitino.exceptions.base import NoSuchTagException +from gravitino.name_identifier import NameIdentifier +from tests.integration.containers.hdfs_container import HDFSContainer +from tests.integration.integration_test_env import IntegrationTestEnv + +# pylint: disable=too-many-instance-attributes + + +class TestSupportsTags(IntegrationTestEnv): + relational_catalog_provider: str = "hive" + fileset_comment: str = "fileset_comment" + catalog_location_prop: str = "location" + + fileset_location: str = "/tmp/TestFilesetCatalog" + fileset_properties_key1: str = "fileset_properties_key1" + fileset_properties_value1: str = "fileset_properties_value1" + fileset_properties_key2: str = "fileset_properties_key2" + fileset_properties_value2: str = "fileset_properties_value2" + fileset_properties: tp.Dict[str, str] = { + fileset_properties_key1: fileset_properties_value1, + fileset_properties_key2: fileset_properties_value2, + } + multiple_locations_fileset_properties: tp.Dict[str, str] = { + Fileset.PROPERTY_DEFAULT_LOCATION_NAME: "location1", + **fileset_properties, + } + + _metalake_name: str = "tag_it_metalake" + str(randint(0, 1000)) + _relational_catalog_name: str = "relational_catalog" + str(randint(0, 1000)) + _model_catalog_name: str = "model_catalog" + str(randint(0, 1000)) + _fileset_catalog_name: str = "fileset_catalog" + str(randint(0, 1000)) + # SCHEMA + _schema_name = "tag_it_schema" + str(randint(0, 1000)) + # OTHER + _table_name: str = "tag_it_table" + str(randint(0, 1000)) + _fileset_name: str = "tag_it_fileset" + str(randint(0, 1000)) + _model_name: str = "tag_it_model" + str(randint(0, 1000)) + + _tag_name1: str = "tag_it_tag1" + str(randint(0, 1000)) + _tag_name2: str = "tag_it_tag2" + str(randint(0, 1000)) + _tag_name3: str = "tag_it_tag3" + str(randint(0, 1000)) + _tag_name4: str = "tag_it_tag4" + str(randint(0, 1000)) + + _gravitino_admin_client: GravitinoAdminClient + _gravitino_client: GravitinoClient + _metalake: GravitinoMetalake + _model_catalog: Catalog + _table_catalog: TableCatalog + _relational_catalog: Catalog + _fileset_catalog: Catalog + _tag1: Tag + _tag2: Tag + _tag3: Tag + _tag4: Tag + + _table_ident: NameIdentifier + _fileset_ident: NameIdentifier + _model_ident: NameIdentifier + _hdfs_container: HDFSContainer + + @classmethod + def setUpClass(cls) -> None: + super().setUpClass() + cls._hdfs_container = HDFSContainer() + cls._gravitino_admin_client = GravitinoAdminClient(uri="http://localhost:8090";) + + cls._metalake = cls._gravitino_admin_client.create_metalake( + cls._metalake_name, comment="test metalake", properties={} + ) + cls._gravitino_client = GravitinoClient( + uri="http://localhost:8090";, metalake_name=cls._metalake_name + ) + + cls._gravitino_client.create_tag(cls._tag_name1, "test tag1", {}) + cls._gravitino_client.create_tag(cls._tag_name2, "test tag2", {}) + cls._gravitino_client.create_tag(cls._tag_name3, "test tag3", {}) + cls._gravitino_client.create_tag(cls._tag_name4, "test tag4", {}) + + cls._tag1 = cls._gravitino_client.get_tag(cls._tag_name1) + cls._tag2 = cls._gravitino_client.get_tag(cls._tag_name2) + cls._tag3 = cls._gravitino_client.get_tag(cls._tag_name3) + cls._tag4 = cls._gravitino_client.get_tag(cls._tag_name4) + + hive_metastore_uri = f"thrift://{cls._hdfs_container.get_ip()}:9083" + + cls._model_catalog = cls._gravitino_client.create_catalog( + name=cls._model_catalog_name, + catalog_type=Catalog.Type.MODEL, + provider="", + comment="comment", + properties={}, + ) + cls._fileset_catalog = cls._gravitino_client.create_catalog( + name=cls._fileset_catalog_name, + catalog_type=Catalog.Type.FILESET, + provider="", + comment="", + properties={cls.catalog_location_prop: "/tmp/test1"}, + ) + cls._relational_catalog = cls._gravitino_client.create_catalog( + name=cls._relational_catalog_name, + catalog_type=Catalog.Type.RELATIONAL, + provider=cls.relational_catalog_provider, + comment="Test relational catalog", + properties={"metastore.uris": hive_metastore_uri}, + ) + cls._table_catalog = cls._relational_catalog.as_table_catalog() + + @classmethod + def tearDownClass(cls) -> None: + cls._gravitino_client.drop_catalog(name=cls._model_catalog_name, force=True) + cls._gravitino_client.drop_catalog( + name=cls._relational_catalog_name, force=True + ) + cls._gravitino_client.drop_catalog(name=cls._fileset_catalog_name, force=True) + + cls._gravitino_client.delete_tag(cls._tag_name1) + cls._gravitino_client.delete_tag(cls._tag_name2) + cls._gravitino_client.delete_tag(cls._tag_name3) + cls._gravitino_client.delete_tag(cls._tag_name4) + + cls._gravitino_admin_client.drop_metalake(name=cls._metalake_name, force=True) + + def setUp(self) -> None: + self._table_ident: NameIdentifier = NameIdentifier.of( + self._schema_name, + self._table_name, + ) + self._fileset_ident: NameIdentifier = NameIdentifier.of( + self._schema_name, + self._fileset_name, + ) + self._model_ident = NameIdentifier.of( + self._schema_name, + self._model_name, + ) + + def tearDown(self) -> None: + self._model_catalog.as_schemas().drop_schema(self._schema_name, True) + self._relational_catalog.as_schemas().drop_schema(self._schema_name, True) + self._fileset_catalog.as_schemas().drop_schema(self._schema_name, True) Review Comment: `tearDown()` unconditionally drops the same schema in the model/relational/fileset catalogs, but several tests only create the schema in one catalog (and `test_catalog_tag_operations` creates none). `BaseSchemaCatalog.drop_schema()` raises `NoSuchSchemaException` on missing schemas, so this teardown will fail the tests. Consider either creating the schema in all relevant catalogs in `setUp()` (and dropping in `tearDown()`), or catching/ignoring `NoSuchSchemaException` per catalog, or tracking which schemas were created before attempting to drop. ########## clients/client-python/tests/integration/test_supports_tags.py: ########## @@ -0,0 +1,416 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import typing as tp +from random import randint + +from gravitino import Catalog, GravitinoAdminClient, GravitinoClient, GravitinoMetalake +from gravitino.api.file.fileset import Fileset +from gravitino.api.model.model import Model +from gravitino.api.rel.table import Table +from gravitino.api.rel.table_catalog import TableCatalog +from gravitino.api.rel.types.types import Types +from gravitino.api.tag import Tag +from gravitino.api.tag.supports_tags import SupportsTags +from gravitino.dto.rel.column_dto import ColumnDTO +from gravitino.dto.rel.partitioning.identity_partitioning_dto import ( + IdentityPartitioningDTO, +) +from gravitino.exceptions.base import NoSuchTagException +from gravitino.name_identifier import NameIdentifier +from tests.integration.containers.hdfs_container import HDFSContainer +from tests.integration.integration_test_env import IntegrationTestEnv + +# pylint: disable=too-many-instance-attributes + + +class TestSupportsTags(IntegrationTestEnv): + relational_catalog_provider: str = "hive" + fileset_comment: str = "fileset_comment" + catalog_location_prop: str = "location" + + fileset_location: str = "/tmp/TestFilesetCatalog" + fileset_properties_key1: str = "fileset_properties_key1" + fileset_properties_value1: str = "fileset_properties_value1" + fileset_properties_key2: str = "fileset_properties_key2" + fileset_properties_value2: str = "fileset_properties_value2" + fileset_properties: tp.Dict[str, str] = { + fileset_properties_key1: fileset_properties_value1, + fileset_properties_key2: fileset_properties_value2, + } + multiple_locations_fileset_properties: tp.Dict[str, str] = { + Fileset.PROPERTY_DEFAULT_LOCATION_NAME: "location1", + **fileset_properties, + } + + _metalake_name: str = "tag_it_metalake" + str(randint(0, 1000)) + _relational_catalog_name: str = "relational_catalog" + str(randint(0, 1000)) + _model_catalog_name: str = "model_catalog" + str(randint(0, 1000)) + _fileset_catalog_name: str = "fileset_catalog" + str(randint(0, 1000)) + # SCHEMA + _schema_name = "tag_it_schema" + str(randint(0, 1000)) + # OTHER + _table_name: str = "tag_it_table" + str(randint(0, 1000)) + _fileset_name: str = "tag_it_fileset" + str(randint(0, 1000)) + _model_name: str = "tag_it_model" + str(randint(0, 1000)) + + _tag_name1: str = "tag_it_tag1" + str(randint(0, 1000)) + _tag_name2: str = "tag_it_tag2" + str(randint(0, 1000)) + _tag_name3: str = "tag_it_tag3" + str(randint(0, 1000)) + _tag_name4: str = "tag_it_tag4" + str(randint(0, 1000)) + + _gravitino_admin_client: GravitinoAdminClient + _gravitino_client: GravitinoClient + _metalake: GravitinoMetalake + _model_catalog: Catalog + _table_catalog: TableCatalog + _relational_catalog: Catalog + _fileset_catalog: Catalog + _tag1: Tag + _tag2: Tag + _tag3: Tag + _tag4: Tag + + _table_ident: NameIdentifier + _fileset_ident: NameIdentifier + _model_ident: NameIdentifier + _hdfs_container: HDFSContainer + + @classmethod + def setUpClass(cls) -> None: + super().setUpClass() + cls._hdfs_container = HDFSContainer() + cls._gravitino_admin_client = GravitinoAdminClient(uri="http://localhost:8090";) + + cls._metalake = cls._gravitino_admin_client.create_metalake( + cls._metalake_name, comment="test metalake", properties={} + ) + cls._gravitino_client = GravitinoClient( + uri="http://localhost:8090";, metalake_name=cls._metalake_name + ) + + cls._gravitino_client.create_tag(cls._tag_name1, "test tag1", {}) + cls._gravitino_client.create_tag(cls._tag_name2, "test tag2", {}) + cls._gravitino_client.create_tag(cls._tag_name3, "test tag3", {}) + cls._gravitino_client.create_tag(cls._tag_name4, "test tag4", {}) + + cls._tag1 = cls._gravitino_client.get_tag(cls._tag_name1) + cls._tag2 = cls._gravitino_client.get_tag(cls._tag_name2) + cls._tag3 = cls._gravitino_client.get_tag(cls._tag_name3) + cls._tag4 = cls._gravitino_client.get_tag(cls._tag_name4) + + hive_metastore_uri = f"thrift://{cls._hdfs_container.get_ip()}:9083" + + cls._model_catalog = cls._gravitino_client.create_catalog( + name=cls._model_catalog_name, + catalog_type=Catalog.Type.MODEL, + provider="", + comment="comment", + properties={}, + ) + cls._fileset_catalog = cls._gravitino_client.create_catalog( + name=cls._fileset_catalog_name, + catalog_type=Catalog.Type.FILESET, + provider="", + comment="", + properties={cls.catalog_location_prop: "/tmp/test1"}, + ) + cls._relational_catalog = cls._gravitino_client.create_catalog( + name=cls._relational_catalog_name, + catalog_type=Catalog.Type.RELATIONAL, + provider=cls.relational_catalog_provider, + comment="Test relational catalog", + properties={"metastore.uris": hive_metastore_uri}, + ) + cls._table_catalog = cls._relational_catalog.as_table_catalog() + + @classmethod + def tearDownClass(cls) -> None: + cls._gravitino_client.drop_catalog(name=cls._model_catalog_name, force=True) + cls._gravitino_client.drop_catalog( + name=cls._relational_catalog_name, force=True + ) + cls._gravitino_client.drop_catalog(name=cls._fileset_catalog_name, force=True) + + cls._gravitino_client.delete_tag(cls._tag_name1) + cls._gravitino_client.delete_tag(cls._tag_name2) + cls._gravitino_client.delete_tag(cls._tag_name3) + cls._gravitino_client.delete_tag(cls._tag_name4) + + cls._gravitino_admin_client.drop_metalake(name=cls._metalake_name, force=True) Review Comment: `tearDownClass()` does not call `super().tearDownClass()`. Since `IntegrationTestEnv.tearDownClass()` stops the Gravitino server when it was started by the test harness, skipping the superclass teardown can leave the server running and affect subsequent integration tests. Please call `super().tearDownClass()` (ideally in a `finally` block after your cleanup). ########## clients/client-python/tests/integration/test_supports_tags.py: ########## @@ -0,0 +1,416 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import typing as tp +from random import randint + +from gravitino import Catalog, GravitinoAdminClient, GravitinoClient, GravitinoMetalake +from gravitino.api.file.fileset import Fileset +from gravitino.api.model.model import Model +from gravitino.api.rel.table import Table +from gravitino.api.rel.table_catalog import TableCatalog +from gravitino.api.rel.types.types import Types +from gravitino.api.tag import Tag +from gravitino.api.tag.supports_tags import SupportsTags +from gravitino.dto.rel.column_dto import ColumnDTO +from gravitino.dto.rel.partitioning.identity_partitioning_dto import ( + IdentityPartitioningDTO, +) +from gravitino.exceptions.base import NoSuchTagException +from gravitino.name_identifier import NameIdentifier +from tests.integration.containers.hdfs_container import HDFSContainer +from tests.integration.integration_test_env import IntegrationTestEnv + +# pylint: disable=too-many-instance-attributes + + +class TestSupportsTags(IntegrationTestEnv): + relational_catalog_provider: str = "hive" + fileset_comment: str = "fileset_comment" + catalog_location_prop: str = "location" + + fileset_location: str = "/tmp/TestFilesetCatalog" + fileset_properties_key1: str = "fileset_properties_key1" + fileset_properties_value1: str = "fileset_properties_value1" + fileset_properties_key2: str = "fileset_properties_key2" + fileset_properties_value2: str = "fileset_properties_value2" + fileset_properties: tp.Dict[str, str] = { + fileset_properties_key1: fileset_properties_value1, + fileset_properties_key2: fileset_properties_value2, + } + multiple_locations_fileset_properties: tp.Dict[str, str] = { + Fileset.PROPERTY_DEFAULT_LOCATION_NAME: "location1", + **fileset_properties, + } Review Comment: `multiple_locations_fileset_properties` is defined but never used in this test module, which adds noise and can confuse future maintenance. Consider removing it or adding a test case that actually exercises multi-location fileset properties. ########## clients/client-python/gravitino/api/catalog.py: ########## @@ -189,7 +190,7 @@ def as_model_catalog(self) -> "ModelCatalog": # noqa: F821 """ raise UnsupportedOperationException("Catalog does not support model operations") Review Comment: `as_model_catalog()` now advertises a return type of `GenericModelCatalog`, but the docstring still refers to `ModelCatalog` (and uses a Java-style `{@link ...}` reference). Please update the docstring to match the actual Python class/type to avoid confusion for API consumers and type-checkers. ########## clients/client-python/tests/integration/test_supports_tags.py: ########## @@ -0,0 +1,416 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import typing as tp +from random import randint + +from gravitino import Catalog, GravitinoAdminClient, GravitinoClient, GravitinoMetalake +from gravitino.api.file.fileset import Fileset +from gravitino.api.model.model import Model +from gravitino.api.rel.table import Table +from gravitino.api.rel.table_catalog import TableCatalog +from gravitino.api.rel.types.types import Types +from gravitino.api.tag import Tag +from gravitino.api.tag.supports_tags import SupportsTags +from gravitino.dto.rel.column_dto import ColumnDTO +from gravitino.dto.rel.partitioning.identity_partitioning_dto import ( + IdentityPartitioningDTO, +) +from gravitino.exceptions.base import NoSuchTagException +from gravitino.name_identifier import NameIdentifier +from tests.integration.containers.hdfs_container import HDFSContainer +from tests.integration.integration_test_env import IntegrationTestEnv + +# pylint: disable=too-many-instance-attributes + + +class TestSupportsTags(IntegrationTestEnv): + relational_catalog_provider: str = "hive" + fileset_comment: str = "fileset_comment" + catalog_location_prop: str = "location" + + fileset_location: str = "/tmp/TestFilesetCatalog" + fileset_properties_key1: str = "fileset_properties_key1" + fileset_properties_value1: str = "fileset_properties_value1" + fileset_properties_key2: str = "fileset_properties_key2" + fileset_properties_value2: str = "fileset_properties_value2" + fileset_properties: tp.Dict[str, str] = { + fileset_properties_key1: fileset_properties_value1, + fileset_properties_key2: fileset_properties_value2, + } + multiple_locations_fileset_properties: tp.Dict[str, str] = { + Fileset.PROPERTY_DEFAULT_LOCATION_NAME: "location1", + **fileset_properties, + } + + _metalake_name: str = "tag_it_metalake" + str(randint(0, 1000)) + _relational_catalog_name: str = "relational_catalog" + str(randint(0, 1000)) + _model_catalog_name: str = "model_catalog" + str(randint(0, 1000)) + _fileset_catalog_name: str = "fileset_catalog" + str(randint(0, 1000)) + # SCHEMA + _schema_name = "tag_it_schema" + str(randint(0, 1000)) + # OTHER + _table_name: str = "tag_it_table" + str(randint(0, 1000)) + _fileset_name: str = "tag_it_fileset" + str(randint(0, 1000)) + _model_name: str = "tag_it_model" + str(randint(0, 1000)) + + _tag_name1: str = "tag_it_tag1" + str(randint(0, 1000)) + _tag_name2: str = "tag_it_tag2" + str(randint(0, 1000)) + _tag_name3: str = "tag_it_tag3" + str(randint(0, 1000)) + _tag_name4: str = "tag_it_tag4" + str(randint(0, 1000)) + + _gravitino_admin_client: GravitinoAdminClient + _gravitino_client: GravitinoClient + _metalake: GravitinoMetalake + _model_catalog: Catalog + _table_catalog: TableCatalog + _relational_catalog: Catalog + _fileset_catalog: Catalog + _tag1: Tag + _tag2: Tag + _tag3: Tag + _tag4: Tag + + _table_ident: NameIdentifier + _fileset_ident: NameIdentifier + _model_ident: NameIdentifier + _hdfs_container: HDFSContainer + + @classmethod + def setUpClass(cls) -> None: + super().setUpClass() + cls._hdfs_container = HDFSContainer() + cls._gravitino_admin_client = GravitinoAdminClient(uri="http://localhost:8090";) + Review Comment: The HDFS/Hive docker container created in `setUpClass()` (`cls._hdfs_container = HDFSContainer()`) is never closed. Other integration tests explicitly call `hdfs_container.close()` in `tearDownClass()` to avoid leaking containers/networks across test runs. Please close `cls._hdfs_container` during teardown (in `finally`) before calling `super().tearDownClass()`. ########## clients/client-python/tests/integration/test_supports_tags.py: ########## @@ -0,0 +1,416 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import typing as tp +from random import randint + +from gravitino import Catalog, GravitinoAdminClient, GravitinoClient, GravitinoMetalake +from gravitino.api.file.fileset import Fileset +from gravitino.api.model.model import Model +from gravitino.api.rel.table import Table +from gravitino.api.rel.table_catalog import TableCatalog +from gravitino.api.rel.types.types import Types +from gravitino.api.tag import Tag +from gravitino.api.tag.supports_tags import SupportsTags +from gravitino.dto.rel.column_dto import ColumnDTO +from gravitino.dto.rel.partitioning.identity_partitioning_dto import ( + IdentityPartitioningDTO, +) +from gravitino.exceptions.base import NoSuchTagException +from gravitino.name_identifier import NameIdentifier +from tests.integration.containers.hdfs_container import HDFSContainer +from tests.integration.integration_test_env import IntegrationTestEnv + +# pylint: disable=too-many-instance-attributes + + +class TestSupportsTags(IntegrationTestEnv): + relational_catalog_provider: str = "hive" + fileset_comment: str = "fileset_comment" + catalog_location_prop: str = "location" + + fileset_location: str = "/tmp/TestFilesetCatalog" + fileset_properties_key1: str = "fileset_properties_key1" + fileset_properties_value1: str = "fileset_properties_value1" + fileset_properties_key2: str = "fileset_properties_key2" + fileset_properties_value2: str = "fileset_properties_value2" + fileset_properties: tp.Dict[str, str] = { + fileset_properties_key1: fileset_properties_value1, + fileset_properties_key2: fileset_properties_value2, + } + multiple_locations_fileset_properties: tp.Dict[str, str] = { + Fileset.PROPERTY_DEFAULT_LOCATION_NAME: "location1", + **fileset_properties, + } + + _metalake_name: str = "tag_it_metalake" + str(randint(0, 1000)) + _relational_catalog_name: str = "relational_catalog" + str(randint(0, 1000)) + _model_catalog_name: str = "model_catalog" + str(randint(0, 1000)) + _fileset_catalog_name: str = "fileset_catalog" + str(randint(0, 1000)) + # SCHEMA + _schema_name = "tag_it_schema" + str(randint(0, 1000)) + # OTHER + _table_name: str = "tag_it_table" + str(randint(0, 1000)) + _fileset_name: str = "tag_it_fileset" + str(randint(0, 1000)) + _model_name: str = "tag_it_model" + str(randint(0, 1000)) + + _tag_name1: str = "tag_it_tag1" + str(randint(0, 1000)) + _tag_name2: str = "tag_it_tag2" + str(randint(0, 1000)) + _tag_name3: str = "tag_it_tag3" + str(randint(0, 1000)) + _tag_name4: str = "tag_it_tag4" + str(randint(0, 1000)) + + _gravitino_admin_client: GravitinoAdminClient + _gravitino_client: GravitinoClient + _metalake: GravitinoMetalake + _model_catalog: Catalog + _table_catalog: TableCatalog + _relational_catalog: Catalog + _fileset_catalog: Catalog + _tag1: Tag + _tag2: Tag + _tag3: Tag + _tag4: Tag + + _table_ident: NameIdentifier + _fileset_ident: NameIdentifier + _model_ident: NameIdentifier + _hdfs_container: HDFSContainer + + @classmethod + def setUpClass(cls) -> None: + super().setUpClass() + cls._hdfs_container = HDFSContainer() + cls._gravitino_admin_client = GravitinoAdminClient(uri="http://localhost:8090";) + + cls._metalake = cls._gravitino_admin_client.create_metalake( + cls._metalake_name, comment="test metalake", properties={} + ) + cls._gravitino_client = GravitinoClient( + uri="http://localhost:8090";, metalake_name=cls._metalake_name + ) + + cls._gravitino_client.create_tag(cls._tag_name1, "test tag1", {}) + cls._gravitino_client.create_tag(cls._tag_name2, "test tag2", {}) + cls._gravitino_client.create_tag(cls._tag_name3, "test tag3", {}) + cls._gravitino_client.create_tag(cls._tag_name4, "test tag4", {}) + + cls._tag1 = cls._gravitino_client.get_tag(cls._tag_name1) + cls._tag2 = cls._gravitino_client.get_tag(cls._tag_name2) + cls._tag3 = cls._gravitino_client.get_tag(cls._tag_name3) + cls._tag4 = cls._gravitino_client.get_tag(cls._tag_name4) + + hive_metastore_uri = f"thrift://{cls._hdfs_container.get_ip()}:9083" + + cls._model_catalog = cls._gravitino_client.create_catalog( + name=cls._model_catalog_name, + catalog_type=Catalog.Type.MODEL, + provider="", + comment="comment", + properties={}, + ) + cls._fileset_catalog = cls._gravitino_client.create_catalog( + name=cls._fileset_catalog_name, + catalog_type=Catalog.Type.FILESET, + provider="", Review Comment: For MODEL/FILESET catalogs, other integration tests pass `provider=None` when using managed catalogs. Here `provider=""` is passed, which may round-trip as an empty provider value and break client-side validation (e.g., some catalog DTO validations require a non-blank provider). Consider using `provider=None` for managed model/fileset catalogs for consistency and to avoid empty-string edge cases. ########## clients/client-python/gravitino/api/model/model.py: ########## @@ -72,3 +74,6 @@ def latest_version(self) -> int: The latest version of the model object. """ pass + + def supports_tags(self) -> SupportsTags: + raise UnsupportedOperationException("Table does not support tag operations.") Review Comment: The exception message in `Model.supports_tags()` says "Table does not support tag operations", which is misleading for model objects. Please update the message to reference Model (or a generic "Object"). ########## clients/client-python/gravitino/api/schema.py: ########## @@ -43,3 +45,6 @@ def comment(self) -> Optional[str]: def properties(self) -> Dict[str, str]: """Returns the properties of the Schema. An empty dictionary is returned if no properties are set.""" return {} + + def supports_tags(self) -> SupportsTags: + raise UnsupportedOperationException("Table does not support tag operations.") Review Comment: The exception message in `Schema.supports_tags()` says "Table does not support tag operations", which is misleading for schema objects. Please update the message to reference Schema (or a generic "Object") to avoid confusing users debugging unsupported operations. ########## clients/client-python/gravitino/api/file/fileset.py: ########## @@ -209,3 +211,6 @@ def properties(self) -> Dict[str, str]: The properties of the fileset object. Empty map is returned if no properties are set. """ pass + + def supports_tags(self) -> SupportsTags: + raise UnsupportedOperationException("Table does not support tag operations.") Review Comment: The exception message in `Fileset.supports_tags()` says "Table does not support tag operations", which is misleading for fileset objects. Please update the message to reference Fileset (or a generic "Object"). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
