korbit-ai[bot] commented on code in PR #35621:
URL: https://github.com/apache/superset/pull/35621#discussion_r2428684790


##########
superset/utils/hashing.py:
##########
@@ -14,23 +14,80 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
+from __future__ import annotations
+
 import hashlib
-from typing import Any, Callable, Optional
+import logging
+from typing import Any, Callable, Literal, Optional
+
+from flask import current_app
 
 from superset.utils import json
 
+logger = logging.getLogger(__name__)
+
+HashAlgorithm = Literal["md5", "sha256"]
+
+
+def get_hash_algorithm() -> HashAlgorithm:
+    """
+    Get the configured hash algorithm for non-cryptographic purposes.
+
+    Returns:
+        Hash algorithm name ('md5' or 'sha256')
+    """
+    return current_app.config["HASH_ALGORITHM"]
+
 
-def md5_sha_from_str(val: str) -> str:
-    return hashlib.md5(val.encode("utf-8")).hexdigest()  # noqa: S324
+def hash_from_str(val: str, algorithm: Optional[HashAlgorithm] = None) -> str:
+    """
+    Generate a hash from a string using the configured or specified algorithm.
 
+    Args:
+        val: String to hash
+        algorithm: Hash algorithm to use (defaults to configured algorithm)
 
-def md5_sha_from_dict(
+    Returns:
+        Hexadecimal hash digest string
+
+    Examples:
+        >>> hash_from_str("test")  # Uses configured algorithm
+        '9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08'
+        >>> hash_from_str("test", algorithm="md5")  # Force MD5
+        '098f6bcd4621d373cade4e832627b4f6'
+    """
+    if algorithm is None:
+        algorithm = get_hash_algorithm()
+
+    if algorithm == "sha256":
+        return hashlib.sha256(val.encode("utf-8")).hexdigest()
+    elif algorithm == "md5":
+        # MD5 is only acceptable for legacy compatibility
+        return hashlib.md5(val.encode("utf-8")).hexdigest()  # noqa: S324
+    else:
+        raise ValueError(f"Unsupported hash algorithm: {algorithm}")

Review Comment:
   ### Inefficient algorithm selection with string comparisons <sub>![category 
Performance](https://img.shields.io/badge/Performance-4f46e5)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The hash_from_str function performs string comparisons and conditional 
branching on every call instead of using a more efficient lookup mechanism.
   
   
   ###### Why this matters
   String comparisons in conditional statements are slower than dictionary 
lookups, especially when this function is called frequently for hashing 
operations throughout the application.
   
   ###### Suggested change ∙ *Feature Preview*
   Use a dictionary lookup for algorithm selection to eliminate string 
comparisons:
   
   ```python
   _HASH_FUNCTIONS = {
       "sha256": lambda data: hashlib.sha256(data).hexdigest(),
       "md5": lambda data: hashlib.md5(data).hexdigest(),  # noqa: S324
   }
   
   def hash_from_str(val: str, algorithm: Optional[HashAlgorithm] = None) -> 
str:
       if algorithm is None:
           algorithm = get_hash_algorithm()
       
       hash_func = _HASH_FUNCTIONS.get(algorithm)
       if hash_func is None:
           raise ValueError(f"Unsupported hash algorithm: {algorithm}")
       
       return hash_func(val.encode("utf-8"))
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2062c794-8f2a-450a-b414-490c9548b68f/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2062c794-8f2a-450a-b414-490c9548b68f?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2062c794-8f2a-450a-b414-490c9548b68f?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2062c794-8f2a-450a-b414-490c9548b68f?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2062c794-8f2a-450a-b414-490c9548b68f)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:7414c983-8c20-4daa-afec-2232dcab013e -->
   
   
   [](7414c983-8c20-4daa-afec-2232dcab013e)



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to