[#3205] improvement(client-python): Apply naming convention to client…

…-python (#3369) ### What changes were proposed in this pull request? - Define naming style of each class, module, function and variable - See `pylintrc` for detail settings ### Why are the changes needed? Fix: #3205 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? No --------- Co-authored-by: TimWang <[email protected]> (cherry picked from commit 0a3caed)
apache · Jun 28, 2024 · 5f0ea5f · 5f0ea5f
1 parent b4cfbd2
commit 5f0ea5f
Show file tree

Hide file tree

Showing 9 changed files with 136 additions and 69 deletions.
diff --git a/clients/client-python/gravitino/client/gravitino_admin_client.py b/clients/client-python/gravitino/client/gravitino_admin_client.py
@@ -2,13 +2,13 @@
 Copyright 2024 Datastrato Pvt Ltd.
 This software is licensed under the Apache License version 2.
 """
+
 import logging
 from typing import List, Dict
 
 from gravitino.client.gravitino_client_base import GravitinoClientBase
 from gravitino.client.gravitino_metalake import GravitinoMetalake
 from gravitino.dto.dto_converters import DTOConverters
-from gravitino.dto.metalake_dto import MetalakeDTO
 from gravitino.dto.requests.metalake_create_request import MetalakeCreateRequest
 from gravitino.dto.requests.metalake_updates_request import MetalakeUpdatesRequest
 from gravitino.dto.responses.drop_response import DropResponse
@@ -22,12 +22,12 @@
 
 class GravitinoAdminClient(GravitinoClientBase):
     """
-    Gravitino Client for the administrator to interact with the Gravitino API, allowing the client to list, load, create, and alter Metalakes.
+    Gravitino Client for the administrator to interact with the Gravitino API.
+    It allows the client to list, load, create, and alter Metalakes.
     Normal users should use {@link GravitinoClient} to connect with the Gravitino server.
     """
 
-    def __init__(self, uri):  # TODO: AuthDataProvider authDataProvider
-        super().__init__(uri)
+    # TODO: AuthDataProvider authDataProvider
 
     def list_metalakes(self) -> List[GravitinoMetalake]:
         """Retrieves a list of Metalakes from the Gravitino API.
@@ -36,12 +36,19 @@ def list_metalakes(self) -> List[GravitinoMetalake]:
             An array of GravitinoMetalake objects representing the Metalakes.
         """
         resp = self._rest_client.get(self.API_METALAKES_LIST_PATH)
-        metalake_list_resp = MetalakeListResponse.from_json(resp.body, infer_missing=True)
+        metalake_list_resp = MetalakeListResponse.from_json(
+            resp.body, infer_missing=True
+        )
         metalake_list_resp.validate()
 
-        return [GravitinoMetalake(o, self._rest_client) for o in metalake_list_resp.metalakes()]
+        return [
+            GravitinoMetalake(o, self._rest_client)
+            for o in metalake_list_resp.metalakes()
+        ]
 
-    def create_metalake(self, ident: NameIdentifier, comment: str, properties: Dict[str, str]) -> GravitinoMetalake:
+    def create_metalake(
+        self, ident: NameIdentifier, comment: str, properties: Dict[str, str]
+    ) -> GravitinoMetalake:
         """Creates a new Metalake using the Gravitino API.
 
         Args:
@@ -65,7 +72,9 @@ def create_metalake(self, ident: NameIdentifier, comment: str, properties: Dict[
 
         return GravitinoMetalake(metalake, self._rest_client)
 
-    def alter_metalake(self, ident: NameIdentifier, *changes: MetalakeChange) -> GravitinoMetalake:
+    def alter_metalake(
+        self, ident: NameIdentifier, *changes: MetalakeChange
+    ) -> GravitinoMetalake:
         """Alters a specific Metalake using the Gravitino API.
 
         Args:
@@ -83,7 +92,9 @@ def alter_metalake(self, ident: NameIdentifier, *changes: MetalakeChange) -> Gra
         updates_request = MetalakeUpdatesRequest(reqs)
         updates_request.validate()
 
-        resp = self._rest_client.put(self.API_METALAKES_IDENTIFIER_PATH + ident.name(), updates_request)
+        resp = self._rest_client.put(
+            self.API_METALAKES_IDENTIFIER_PATH + ident.name(), updates_request
+        )
         metalake_response = MetalakeResponse.from_json(resp.body, infer_missing=True)
         metalake_response.validate()
         metalake = metalake_response.metalake()
@@ -102,10 +113,12 @@ def drop_metalake(self, ident: NameIdentifier) -> bool:
         NameIdentifier.check_metalake(ident)
 
         try:
-            resp = self._rest_client.delete(self.API_METALAKES_IDENTIFIER_PATH + ident.name())
-            dropResponse = DropResponse.from_json(resp.body, infer_missing=True)
-
-            return dropResponse.dropped()
-        except Exception as e:
-            logger.warning(f"Failed to drop metalake {ident}")
+            resp = self._rest_client.delete(
+                self.API_METALAKES_IDENTIFIER_PATH + ident.name()
+            )
+            drop_response = DropResponse.from_json(resp.body, infer_missing=True)
+
+            return drop_response.dropped()
+        except Exception:
+            logger.warning("Failed to drop metalake %s", ident)
             return False
diff --git a/clients/client-python/gravitino/client/gravitino_metalake.py b/clients/client-python/gravitino/client/gravitino_metalake.py
@@ -67,9 +67,9 @@ def list_catalogs(self, namespace: Namespace) -> List[NameIdentifier]:
         Namespace.check_catalog(namespace)
         url = f"api/metalakes/{namespace.level(0)}/catalogs"
         response = self.rest_client.get(url)
-        entityList = EntityListResponse.from_json(response.body, infer_missing=True)
-        entityList.validate()
-        return entityList.identifiers()
+        entity_list = EntityListResponse.from_json(response.body, infer_missing=True)
+        entity_list.validate()
+        return entity_list.identifiers()
 
     def list_catalogs_info(self, namespace: Namespace) -> List[Catalog]:
         """List all the catalogs with their information under this metalake with specified namespace.

diff --git a/clients/client-python/gravitino/dto/version_dto.py b/clients/client-python/gravitino/dto/version_dto.py
@@ -13,8 +13,8 @@ class VersionDTO:
     version: str = ""
     """The version of the software."""
 
-    compileDate: str = ""
+    compile_date: str = ""
     """The date the software was compiled."""
 
-    gitCommit: str = ""
+    git_commit: str = ""
     """The git commit of the software."""
diff --git a/clients/client-python/gravitino/name_identifier.py b/clients/client-python/gravitino/name_identifier.py
@@ -2,11 +2,16 @@
 Copyright 2024 Datastrato Pvt Ltd.
 This software is licensed under the Apache License version 2.
 """
+
+from typing import ClassVar
+
 from dataclasses import dataclass, field
 
 from dataclasses_json import DataClassJsonMixin, config
 
-from gravitino.exceptions.illegal_name_identifier_exception import IllegalNameIdentifierException
+from gravitino.exceptions.illegal_name_identifier_exception import (
+    IllegalNameIdentifierException,
+)
 from gravitino.namespace import Namespace
 
 
@@ -18,10 +23,10 @@ class NameIdentifier(DataClassJsonMixin):
     schema.
     """
 
-    _name: str = field(metadata=config(field_name='name'))
-    _namespace: Namespace = field(metadata=config(field_name='namespace'))
+    _DOT: ClassVar[str] = "."
 
-    DOT: str = '.'
+    _name: str = field(metadata=config(field_name="name"))
+    _namespace: Namespace = field(metadata=config(field_name="namespace"))
 
     @classmethod
     def builder(cls, namespace: Namespace, name: str):
@@ -34,7 +39,7 @@ def name(self):
         return self._name
 
     @staticmethod
-    def of(*names: str) -> 'NameIdentifier':
+    def of(*names: str) -> "NameIdentifier":
         """Create the NameIdentifier with the given levels of names.
 
         Args:
@@ -44,13 +49,17 @@ def of(*names: str) -> 'NameIdentifier':
             The created NameIdentifier
         """
 
-        NameIdentifier.check(names is not None, "Cannot create a NameIdentifier with null names")
-        NameIdentifier.check(len(names) > 0, "Cannot create a NameIdentifier with no names")
+        NameIdentifier.check(
+            names is not None, "Cannot create a NameIdentifier with null names"
+        )
+        NameIdentifier.check(
+            len(names) > 0, "Cannot create a NameIdentifier with no names"
+        )
 
         return NameIdentifier.builder(Namespace.of(*names[:-1]), names[-1])
 
     @staticmethod
-    def of_namespace(namespace: Namespace, name: str) -> 'NameIdentifier':
+    def of_namespace(namespace: Namespace, name: str) -> "NameIdentifier":
         """Create the NameIdentifier with the given Namespace and name.
 
         Args:
@@ -63,7 +72,7 @@ def of_namespace(namespace: Namespace, name: str) -> 'NameIdentifier':
         return NameIdentifier.builder(namespace, name)
 
     @staticmethod
-    def of_metalake(metalake: str) -> 'NameIdentifier':
+    def of_metalake(metalake: str) -> "NameIdentifier":
         """Create the metalake NameIdentifier with the given name.
 
         Args:
@@ -75,7 +84,7 @@ def of_metalake(metalake: str) -> 'NameIdentifier':
         return NameIdentifier.of(metalake)
 
     @staticmethod
-    def of_catalog(metalake: str, catalog: str) -> 'NameIdentifier':
+    def of_catalog(metalake: str, catalog: str) -> "NameIdentifier":
         """Create the catalog NameIdentifier with the given metalake and catalog name.
 
         Args:
@@ -88,7 +97,7 @@ def of_catalog(metalake: str, catalog: str) -> 'NameIdentifier':
         return NameIdentifier.of(metalake, catalog)
 
     @staticmethod
-    def of_schema(metalake: str, catalog: str, schema: str) -> 'NameIdentifier':
+    def of_schema(metalake: str, catalog: str, schema: str) -> "NameIdentifier":
         """Create the schema NameIdentifier with the given metalake, catalog and schema name.
 
         Args:
@@ -102,7 +111,9 @@ def of_schema(metalake: str, catalog: str, schema: str) -> 'NameIdentifier':
         return NameIdentifier.of(metalake, catalog, schema)
 
     @staticmethod
-    def of_table(metalake: str, catalog: str, schema: str, table: str) -> 'NameIdentifier':
+    def of_table(
+        metalake: str, catalog: str, schema: str, table: str
+    ) -> "NameIdentifier":
         """Create the table NameIdentifier with the given metalake, catalog, schema and table name.
 
         Args:
@@ -117,7 +128,9 @@ def of_table(metalake: str, catalog: str, schema: str, table: str) -> 'NameIdent
         return NameIdentifier.of(metalake, catalog, schema, table)
 
     @staticmethod
-    def of_fileset(metalake: str, catalog: str, schema: str, fileset: str) -> 'NameIdentifier':
+    def of_fileset(
+        metalake: str, catalog: str, schema: str, fileset: str
+    ) -> "NameIdentifier":
         """Create the fileset NameIdentifier with the given metalake, catalog, schema and fileset name.
 
         Args:
@@ -132,7 +145,9 @@ def of_fileset(metalake: str, catalog: str, schema: str, fileset: str) -> 'NameI
         return NameIdentifier.of(metalake, catalog, schema, fileset)
 
     @staticmethod
-    def of_topic(metalake: str, catalog: str, schema: str, topic: str) -> 'NameIdentifier':
+    def of_topic(
+        metalake: str, catalog: str, schema: str, topic: str
+    ) -> "NameIdentifier":
         """Create the topic NameIdentifier with the given metalake, catalog, schema and topic
         name.
 
@@ -148,7 +163,7 @@ def of_topic(metalake: str, catalog: str, schema: str, topic: str) -> 'NameIdent
         return NameIdentifier.of(metalake, catalog, schema, topic)
 
     @staticmethod
-    def check_metalake(ident: 'NameIdentifier') -> None:
+    def check_metalake(ident: "NameIdentifier") -> None:
         """Check the given NameIdentifier is a metalake identifier. Throw an {@link
         IllegalNameIdentifierException} if it's not.
 
@@ -159,7 +174,7 @@ def check_metalake(ident: 'NameIdentifier') -> None:
         Namespace.check_metalake(ident.namespace())
 
     @staticmethod
-    def check_catalog(ident: 'NameIdentifier') -> None:
+    def check_catalog(ident: "NameIdentifier") -> None:
         """Check the given NameIdentifier is a catalog identifier. Throw an {@link
         IllegalNameIdentifierException} if it's not.
 
@@ -170,7 +185,7 @@ def check_catalog(ident: 'NameIdentifier') -> None:
         Namespace.check_catalog(ident.namespace())
 
     @staticmethod
-    def check_schema(ident: 'NameIdentifier') -> None:
+    def check_schema(ident: "NameIdentifier") -> None:
         """Check the given NameIdentifier is a schema identifier. Throw an {@link
         IllegalNameIdentifierException} if it's not.
 
@@ -181,7 +196,7 @@ def check_schema(ident: 'NameIdentifier') -> None:
         Namespace.check_schema(ident.namespace())
 
     @staticmethod
-    def check_table(ident: 'NameIdentifier') -> None:
+    def check_table(ident: "NameIdentifier") -> None:
         """Check the given NameIdentifier is a table identifier. Throw an {@link
         IllegalNameIdentifierException} if it's not.
 
@@ -192,7 +207,7 @@ def check_table(ident: 'NameIdentifier') -> None:
         Namespace.check_table(ident.namespace())
 
     @staticmethod
-    def check_fileset(ident: 'NameIdentifier') -> None:
+    def check_fileset(ident: "NameIdentifier") -> None:
         """Check the given NameIdentifier is a fileset identifier. Throw an {@link
         IllegalNameIdentifierException} if it's not.
 
@@ -203,7 +218,7 @@ def check_fileset(ident: 'NameIdentifier') -> None:
         Namespace.check_fileset(ident.namespace())
 
     @staticmethod
-    def check_topic(ident: 'NameIdentifier') -> None:
+    def check_topic(ident: "NameIdentifier") -> None:
         """Check the given NameIdentifier is a topic identifier. Throw an {@link
         IllegalNameIdentifierException} if it's not.
 
@@ -214,7 +229,7 @@ def check_topic(ident: 'NameIdentifier') -> None:
         Namespace.check_topic(ident.namespace())
 
     @staticmethod
-    def parse(identifier: str) -> 'NameIdentifier':
+    def parse(identifier: str) -> "NameIdentifier":
         """Create a NameIdentifier from the given identifier string.
 
         Args:
@@ -223,9 +238,12 @@ def parse(identifier: str) -> 'NameIdentifier':
         Returns:
             The created NameIdentifier
         """
-        NameIdentifier.check(identifier is not None and identifier != '', "Cannot parse a null or empty identifier")
+        NameIdentifier.check(
+            identifier is not None and identifier != "",
+            "Cannot parse a null or empty identifier",
+        )
 
-        parts = identifier.split(NameIdentifier.DOT)
+        parts = identifier.split(NameIdentifier._DOT)
         return NameIdentifier.of(*parts)
 
     def has_namespace(self):
@@ -263,8 +281,7 @@ def __hash__(self):
     def __str__(self):
         if self.has_namespace():
             return str(self._namespace) + "." + self._name
-        else:
-            return self._name
+        return self._name
 
     @staticmethod
     def check(condition, message, *args):

diff --git a/clients/client-python/gravitino/namespace.py b/clients/client-python/gravitino/namespace.py
@@ -3,7 +3,7 @@
 This software is licensed under the Apache License version 2.
 """
 
-from typing import List
+from typing import List, ClassVar
 
 
 class Namespace:
@@ -12,7 +12,7 @@ class Namespace:
     "metalake1.catalog1.schema1" are all valid namespaces.
     """
 
-    _DOT: str = "."
+    _DOT: ClassVar[str] = "."
 
     _levels: List[str] = []
 

diff --git a/clients/client-python/gravitino/typing.py b/clients/client-python/gravitino/typing.py
@@ -6,6 +6,6 @@
 from typing import Mapping, Sequence, Union
 
 # https://github.com/python/typing/issues/182#issuecomment-1320974824
-JSON_ro = Union[
-    Mapping[str, "JSON_ro"], Sequence["JSON_ro"], str, int, float, bool, None
+JSONType = Union[
+    Mapping[str, "JSONType"], Sequence["JSONType"], str, int, float, bool, None
 ]
diff --git a/clients/client-python/gravitino/utils/http_client.py b/clients/client-python/gravitino/utils/http_client.py
@@ -28,7 +28,7 @@
 from urllib.error import HTTPError
 import json as _json
 
-from gravitino.typing import JSON_ro
+from gravitino.typing import JSONType
 from gravitino.utils.exceptions import handle_error
 from gravitino.constants import TIMEOUT
 
@@ -165,7 +165,7 @@ def close(self):
 
 def unpack(path: str):
     def decorator(func):
-        def wrapper(*args, **kwargs) -> JSON_ro:
+        def wrapper(*args, **kwargs) -> JSONType:
             resp = func(*args, **kwargs)
             rv = resp.json()
             for p in path.split("."):