MonkeyCanCode commented on code in PR #2812:
URL: https://github.com/apache/polaris/pull/2812#discussion_r2483013459
##########
client/python/apache_polaris/.keep:
##########
Review Comment:
This file is no longer needed as now we have `__init__.py` under this path.
##########
client/python/integration_tests/conftest.py:
##########
@@ -379,31 +381,37 @@ def format_namespace(namespace: List[str]) -> str:
def _patch_generated_models() -> None:
"""
The OpenAPI generator creates an `api_client` that dynamically looks up
- model classes from the `polaris.catalog.models` module using `getattr()`.
+ model classes from the `apache_polaris.sdk.catalog.models` module using
`getattr()`.
For example, when a response for a `create_policy` call is received, the
deserializer tries to find the `LoadPolicyResponse` class by looking for
- `polaris.catalog.models.LoadPolicyResponse`.
+ `apache_polaris.sdk.catalog.models.LoadPolicyResponse`.
However, the generator fails to add the necessary `import` statements
to the `polaris/catalog/models/__init__.py` file. This means that even
though the model files exist (e.g., `load_policy_response.py`), the classes
- are not part of the `polaris.catalog.models` namespace.
+ are not part of the `apache_polaris.sdk.catalog.models` namespace.
This fixture works around the bug in the generated code without modifying
the source files. It runs once per test session, before any tests, and
manually injects the missing response-side model classes into the
- `polaris.catalog.models` namespace, allowing the deserializer to find them.
+ `apache_polaris.sdk.catalog.models` namespace, allowing the deserializer
to find them.
"""
- import polaris.catalog.models
- from polaris.catalog.models.applicable_policy import ApplicablePolicy
- from polaris.catalog.models.get_applicable_policies_response import (
+ import apache_polaris.sdk.catalog.models
+ from apache_polaris.sdk.catalog.models.applicable_policy import
ApplicablePolicy
+ from apache_polaris.sdk.catalog.models.get_applicable_policies_response
import (
GetApplicablePoliciesResponse,
)
- from polaris.catalog.models.list_policies_response import
ListPoliciesResponse
- from polaris.catalog.models.load_policy_response import LoadPolicyResponse
- from polaris.catalog.models.policy import Policy
- from polaris.catalog.models.policy_attachment_target import
PolicyAttachmentTarget
- from polaris.catalog.models.policy_identifier import PolicyIdentifier
+ from apache_polaris.sdk.catalog.models.list_policies_response import (
Review Comment:
NIT: for consistency, either use `\` for new line in case if we reach "max"
character per line and not use `()` for single import.
##########
client/python/integration_tests/test_catalog_apis.py:
##########
@@ -19,28 +19,29 @@
import json
import os.path
-import time
-
import pytest
+import time
+from pyiceberg.catalog import Catalog as PyIcebergCatalog
+from pyiceberg.schema import Schema
-from integration_tests.conftest import format_namespace
-from polaris.catalog import (
+from apache_polaris.sdk.catalog import (
CreateNamespaceRequest,
CreateTableRequest,
ModelSchema,
TableIdentifier,
IcebergCatalogAPI,
)
-from polaris.catalog.models.attach_policy_request import AttachPolicyRequest
-from polaris.catalog.models.create_policy_request import CreatePolicyRequest
-from polaris.catalog.models.detach_policy_request import DetachPolicyRequest
-from polaris.catalog.models.policy_attachment_target import
PolicyAttachmentTarget
-from polaris.catalog.models.update_policy_request import UpdatePolicyRequest
-from polaris.catalog.exceptions import NotFoundException
-from polaris.catalog.api.policy_api import PolicyAPI
-from pyiceberg.schema import Schema
-from polaris.management import Catalog
-from pyiceberg.catalog import Catalog as PyIcebergCatalog
+from apache_polaris.sdk.catalog.api.policy_api import PolicyAPI
+from apache_polaris.sdk.catalog.exceptions import NotFoundException
+from apache_polaris.sdk.catalog.models.attach_policy_request import
AttachPolicyRequest
+from apache_polaris.sdk.catalog.models.create_policy_request import
CreatePolicyRequest
+from apache_polaris.sdk.catalog.models.detach_policy_request import
DetachPolicyRequest
+from apache_polaris.sdk.catalog.models.policy_attachment_target import (
Review Comment:
NIT: for consistency, either use \ for new line in case if we reach "max"
character per line and not use () for single import.
##########
client/python/integration_tests/conftest.py:
##########
@@ -379,31 +381,37 @@ def format_namespace(namespace: List[str]) -> str:
def _patch_generated_models() -> None:
"""
The OpenAPI generator creates an `api_client` that dynamically looks up
- model classes from the `polaris.catalog.models` module using `getattr()`.
+ model classes from the `apache_polaris.sdk.catalog.models` module using
`getattr()`.
For example, when a response for a `create_policy` call is received, the
deserializer tries to find the `LoadPolicyResponse` class by looking for
- `polaris.catalog.models.LoadPolicyResponse`.
+ `apache_polaris.sdk.catalog.models.LoadPolicyResponse`.
However, the generator fails to add the necessary `import` statements
to the `polaris/catalog/models/__init__.py` file. This means that even
Review Comment:
Path `polaris/catalog/models/__init__.py` is outdated. Change to
`apache_polaris/sdk/catalog/models/__init__.py`
##########
client/python/apache_polaris/cli/polaris_cli.py:
##########
@@ -50,29 +50,29 @@ class PolarisCli:
def _patch_generated_models() -> None:
"""
The OpenAPI generator creates an `api_client` that dynamically looks up
- model classes from the `polaris.catalog.models` module using
`getattr()`.
+ model classes from the `apache_polaris.sdk.catalog.models` module
using `getattr()`.
For example, when a response for a `create_policy` call is received,
the
deserializer tries to find the `LoadPolicyResponse` class by looking
for
- `polaris.catalog.models.LoadPolicyResponse`.
+ `apache_polaris.sdk.catalog.models.LoadPolicyResponse`.
However, the generator fails to add the necessary `import` statements
to the `polaris/catalog/models/__init__.py` file. This means that even
Review Comment:
`polaris/catalog/models/__init__.py` is now outdated with this PR. Change to
`apache_polaris/sdk/catalog/models/__init__.py`
##########
client/python/integration_tests/test_management_apis.py:
##########
@@ -33,6 +28,11 @@
Catalog,
ResetPrincipalRequest,
)
+from integration_tests.conftest import (
Review Comment:
NIT: for consistency, either use \ for new line in case if we reach "max"
character per line and not use () for single import.
##########
client/python/integration_tests/test_catalog_apis.py:
##########
@@ -19,28 +19,29 @@
import json
import os.path
-import time
-
import pytest
+import time
+from pyiceberg.catalog import Catalog as PyIcebergCatalog
+from pyiceberg.schema import Schema
-from integration_tests.conftest import format_namespace
-from polaris.catalog import (
+from apache_polaris.sdk.catalog import (
Review Comment:
NIT: for consistency, either use \ for new line in case if we reach "max"
character per line and not use () for single import.
##########
regtests/.dockerignore:
##########
@@ -21,4 +21,4 @@
.pytest_cache
t_pyspark/src/spark-warehouse
t_pyspark/src/.pytest_cache
-polaris/polaris.management.egg-info
\ No newline at end of file
+polaris/apache_polaris.sdk.management.egg-info
Review Comment:
This line can be remove as this no longer existed.
--
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]
