xunliu commented on code in PR #5777:
URL: https://github.com/apache/gravitino/pull/5777#discussion_r1881461955
##########
clients/client-python/gravitino/api/credential/credential.py:
##########
Review Comment:
Look like lose two java function in the Python side
1. void initialize(Map<String, String> credentialInfo, long expireTimeInMs);
2. default Map<String, String> toProperties()
Don't we need these two function?
##########
clients/client-python/gravitino/utils/credential_utils.py:
##########
Review Comment:
I think we can keep the consistent Java class name `CredentialFactory`.
##########
clients/client-python/gravitino/catalog/base_schema_catalog.py:
##########
@@ -74,6 +79,11 @@ def __init__(
self.rest_client = rest_client
self._catalog_namespace = catalog_namespace
+ metadata_object = MetadataObjectImpl([name],
MetadataObject.Type.CATALOG)
+ self._credential_operations = MetadataObjectCredentialOperations(
Review Comment:
I think better to change `_credential_operations ` to
`_objectCredentialOperations`, keep consistent Java side.
and add `_objectCredentialOperations` into `BaseSchemaCatalog` member
variable.
##########
clients/client-python/gravitino/client/generic_fileset.py:
##########
@@ -0,0 +1,69 @@
+# 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.
+from typing import Optional, Dict, List
+
+from gravitino.api.fileset import Fileset
+from gravitino.api.metadata_object import MetadataObject
+from gravitino.api.credential.supports_credentials import SupportsCredentials
+from gravitino.api.credential.credential import Credential
+from gravitino.client.metadata_object_credential_operations import (
+ MetadataObjectCredentialOperations,
+)
+from gravitino.client.metadata_object_impl import MetadataObjectImpl
+from gravitino.dto.audit_dto import AuditDTO
+from gravitino.dto.fileset_dto import FilesetDTO
+from gravitino.namespace import Namespace
+from gravitino.utils import HTTPClient
+
+
+class GenericFileset(Fileset, SupportsCredentials):
+
+ def __init__(
+ self, fileset: FilesetDTO, rest_client: HTTPClient, full_namespace:
Namespace
+ ):
+ self._fileset = fileset
Review Comment:
Please add `_fileset` into `GenericFileset` member variable.
##########
clients/client-python/gravitino/api/credential/oss_token_credential.py:
##########
@@ -0,0 +1,105 @@
+# 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.
+
+from abc import ABC
+from typing import Dict
+
+from gravitino.api.credential.credential import Credential
+from gravitino.utils.precondition import Precondition
+
+
+class OSSTokenCredential(Credential, ABC):
+ """Represents OSS token credential."""
+
+ OSS_TOKEN_CREDENTIAL_TYPE = "oss-token"
+ _GRAVITINO_OSS_SESSION_ACCESS_KEY_ID = "oss-access-key-id"
+ _GRAVITINO_OSS_SESSION_SECRET_ACCESS_KEY = "oss-secret-access-key"
+ _GRAVITINO_OSS_TOKEN = "oss-security-token"
Review Comment:
Please add variable types for these variables
##########
clients/client-python/gravitino/api/credential/s3_secret_key_credential.py:
##########
@@ -0,0 +1,91 @@
+# 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.
+
+from abc import ABC
+from typing import Dict
+
+from gravitino.api.credential.credential import Credential
+from gravitino.utils.precondition import Precondition
+
+
+class S3SecretKeyCredential(Credential, ABC):
+ """Represents S3 secret key credential."""
+
+ S3_SECRET_KEY_CREDENTIAL_TYPE = "s3-secret-key"
+ _GRAVITINO_S3_STATIC_ACCESS_KEY_ID = "s3-access-key-id"
+ _GRAVITINO_S3_STATIC_SECRET_ACCESS_KEY = "s3-secret-access-key"
Review Comment:
Please add variable types for these variables
##########
clients/client-python/gravitino/client/metadata_object_credential_operations.py:
##########
@@ -0,0 +1,68 @@
+# 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 logging
+from typing import List
+from gravitino.api.credential.supports_credentials import SupportsCredentials
+from gravitino.api.credential.credential import Credential
+from gravitino.api.metadata_object import MetadataObject
+from gravitino.dto.credential_dto import CredentialDTO
+from gravitino.dto.responses.credential_response import CredentialResponse
+from gravitino.exceptions.handlers.credential_error_handler import (
+ CREDENTIAL_ERROR_HANDLER,
+)
+from gravitino.utils import HTTPClient
+from gravitino.utils.credential_utils import CredentialUtils
+
+logger = logging.getLogger(__name__)
+
+
+class MetadataObjectCredentialOperations(SupportsCredentials):
+ def __init__(
+ self,
+ metalake_name: str,
+ metadata_object: MetadataObject,
+ rest_client: HTTPClient,
+ ):
+ self._rest_client = rest_client
+ metadata_object_type = metadata_object.type().value
+ metadata_object_name = metadata_object.name()
+ self._request_path = (
+ f"api/metalakes/{metalake_name}objects/{metadata_object_type}/"
+ f"{metadata_object_name}/credentials"
+ )
Review Comment:
Please add `_rest_client ` and `_request_path ` into
`MetadataObjectCredentialOperations ` member variable.
##########
clients/client-python/gravitino/api/credential/gcs_token_credential.py:
##########
@@ -0,0 +1,75 @@
+# 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.
+
+from abc import ABC
+from typing import Dict
+
+from gravitino.api.credential.credential import Credential
+from gravitino.utils.precondition import Precondition
+
+
+class GCSTokenCredential(Credential, ABC):
+ """Represents the GCS token credential."""
+
+ GCS_TOKEN_CREDENTIAL_TYPE = "gcs-token"
+ _GCS_TOKEN_NAME = "token"
+
+ _expire_time_in_ms = 0
Review Comment:
Please add variable types for these variables
--
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]
