codeant-ai-for-open-source[bot] commented on code in PR #42154: URL: https://github.com/apache/superset/pull/42154#discussion_r3607448516
########## superset/config.py: ########## @@ -3041,6 +3042,28 @@ class ExtraAccessQueryFilters(TypedDict, total=False): # ------------------------------------------------------------------- # Don't add config values below this line since local configs won't be # able to override them. + + +def _config_fingerprint(config_file: str | None) -> str: + """ + A short digest of the config file's bytes as read at import time. + + Auto-reloaders (e.g. werkzeug's) re-import this module when the config + file changes, and on some filesystems (notably macOS Docker mounts) the + re-read can race the write and observe stale content while still + "loading successfully". Logging the digest of what was *actually read* + makes that skew diagnosable: compare it against + ``md5 <path>`` on the host. + """ + if not config_file: + return "unknown" + try: + with open(config_file, "rb") as fh: + return hashlib.md5(fh.read()).hexdigest()[:12] # noqa: S324 Review Comment: **Suggestion:** The fingerprint is computed by reopening the config file after import, so it can hash different bytes than the interpreter actually executed if the file changes between `exec_module` and this read. That preserves the same reload race this change is trying to diagnose and can log a misleading digest. Compute the digest from the exact source bytes used for module execution (or from the loaded module source provided by the loader) instead of doing a second filesystem read. [race condition] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ⚠️ Config fingerprint may not match executed configuration bytes. - ⚠️ Misleading md5 complicates diagnosing auto-reload race conditions. - ⚠️ Developers may incorrectly trust logged configuration version. ``` </details> <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Start the Superset dev server with a local configuration file referenced by CONFIG_PATH_ENV_VAR so the import block at `superset/config.py:3068-3079` executes (the `spec_from_file_location` and `spec.loader.exec_module(override_conf)` path). 2. Arrange for the `superset_config` file at `cfg_path` to be modified between the `spec.loader.exec_module(override_conf)` call at `superset/config.py:3074-3076` and the subsequent call to `_config_fingerprint(cfg_path)` inside the `click.secho` logging block at `superset/config.py:3081-3084` (for example, by saving the file twice rapidly or using a script that rewrites it after a short delay). 3. On that reload, the `override_conf` module is executed using the bytes returned by the loader during `exec_module`, while `_config_fingerprint()` at `superset/config.py:3048-3063` reopens `cfg_path` and hashes the current on-disk bytes, which can differ if the second write completed after module execution. 4. Observe in the terminal that the log line `Loaded your LOCAL configuration at [...] (md5:...)` reports the digest for the post-write file contents, while the in-process configuration still reflects the pre-write module state, so comparing the logged md5 against `md5 <path>` no longer reliably indicates which configuration bytes were actually executed. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=695b62c71cc6425dbccba52415a72395&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=695b62c71cc6425dbccba52415a72395&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) *(Use Cmd/Ctrl + Click for best experience)* <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset/config.py **Line:** 3061:3062 **Comment:** *Race Condition: The fingerprint is computed by reopening the config file after import, so it can hash different bytes than the interpreter actually executed if the file changes between `exec_module` and this read. That preserves the same reload race this change is trying to diagnose and can log a misleading digest. Compute the digest from the exact source bytes used for module execution (or from the loaded module source provided by the loader) instead of doing a second filesystem read. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42154&comment_hash=12a9765ef88bbfb986d0ac6cb6991c85d2540730125dd79251c092de773794a2&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42154&comment_hash=12a9765ef88bbfb986d0ac6cb6991c85d2540730125dd79251c092de773794a2&reaction=dislike'>👎</a> -- 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]
