FANNG1 commented on code in PR #5997:
URL: https://github.com/apache/gravitino/pull/5997#discussion_r1906577156


##########
clients/client-python/gravitino/filesystem/gvfs.py:
##########
@@ -1001,10 +1151,67 @@ def _get_abs_filesystem(self):
                 "ABS account key is not found in the options."
             )
 
-        return importlib.import_module("adlfs").AzureBlobFileSystem(
-            account_name=abs_account_name,
-            account_key=abs_account_key,
+        return (
+            TIME_WITHOUT_EXPIRATION,
+            importlib.import_module("adlfs").AzureBlobFileSystem(
+                account_name=abs_account_name,
+                account_key=abs_account_key,
+            ),
+        )
+
+    def _get_most_suitable_s3_credential(self, credentials: List[Credential]):
+        for credential in credentials:
+            # Prefer to use the token credential, if it does not exist, use the
+            # secret key credential.
+            if isinstance(credential, S3TokenCredential):
+                return credential
+
+        for credential in credentials:
+            if isinstance(credential, S3SecretKeyCredential):
+                return credential
+        return None
+
+    def _get_most_suitable_oss_credential(self, credentials: List[Credential]):
+        for credential in credentials:
+            # Prefer to use the token credential, if it does not exist, use the
+            # secret key credential.
+            if isinstance(credential, OSSTokenCredential):
+                return credential
+
+        for credential in credentials:
+            if isinstance(credential, OSSSecretKeyCredential):
+                return credential
+        return None
+
+    def _get_most_suitable_gcs_credential(self, credentials: List[Credential]):
+        for credential in credentials:
+            # Prefer to use the token credential, if it does not exist, return 
None.
+            if isinstance(credential, GCSTokenCredential):
+                return credential
+        return None
+
+    def _get_most_suitable_abs_credential(self, credentials: List[Credential]):
+        for credential in credentials:
+            # Prefer to use the token credential, if it does not exist, use the
+            # account key credential
+            if isinstance(credential, ADLSTokenCredential):
+                return credential
+
+        for credential in credentials:
+            if isinstance(credential, AzureAccountKeyCredential):
+                return credential
+        return None
+
+    def _get_expire_time_by_ratio(self, expire_time: int):
+        if expire_time <= 0:
+            return TIME_WITHOUT_EXPIRATION
+
+        ratio = float(
+            self._options.get(
+                GVFSConfig.GVFS_FILESYSTEM_CREDENTIAL_EXPIRED_TIME_RATIO, 0.9

Review Comment:
   I still think `0.9` is too high for client



##########
clients/client-python/gravitino/filesystem/gvfs_config.py:
##########
@@ -44,3 +44,8 @@ class GVFSConfig:
 
     GVFS_FILESYSTEM_AZURE_ACCOUNT_NAME = "abs_account_name"
     GVFS_FILESYSTEM_AZURE_ACCOUNT_KEY = "abs_account_key"
+
+    # This configuration marks the expired time of the credential. For 
instance, if the credential
+    # fetched from Gravitino server has expired time of 3600 seconds, and the 
credential_expired_time_ration is 0.5
+    # then the credential will be considered as expired after 1800 seconds and 
will try to fetch a new credential.
+    GVFS_FILESYSTEM_CREDENTIAL_EXPIRED_TIME_RATIO = 
"credential_expired_time_ratio"

Review Comment:
   credential_expire_ratio?



-- 
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]

Reply via email to