nsivabalan commented on code in PR #20195: URL: https://github.com/apache/hudi/pull/20195#discussion_r4279667846
########## scripts/generate_config_catalog.py: ########## @@ -0,0 +1,1421 @@ +#!/usr/bin/env python3 +# +# 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. +# + +"""Extract every Hudi configuration property from the source tree, with code context. + +The published configuration reference tells a user a config's key, default and +description. It cannot tell them where the value is actually read, what else is +read beside it, or under what condition the read happens at all. That last part +is what makes a config silently do nothing. This script recovers it from source. + +Three passes: + + 1. Declarations. Parse ``ConfigProperty`` builder chains (and Flink's + ``ConfigOptions`` chains whose key is a literal ``hoodie.*``) out of the + main source trees. Chains span many lines and end at the statement + semicolon, so the parser works on brace/paren-balanced statement text + rather than on single lines. + + 2. Accessors. Find config-class methods whose body is a single + ``getInt(SOME_CONFIG)`` style read, so that call sites of + ``getInlineCompactDeltaCommitMax()`` can be attributed back to + ``hoodie.compact.inline.max.delta.commits``. + + 3. Read sites. Walk every source file again, attributing each mention of a + config constant or of a resolved accessor to the enclosing method, and + recording the other configs mentioned in that same method (co-configs) and + the enclosing ``if``/``switch``/``case`` conditions (gating). The gating + pass is a textual heuristic; every condition it emits is marked as such. + +Standard library only. Written to degrade: anything it cannot fully parse still +appears in the catalog, carrying a ``parseWarnings`` list. + +Usage: + python3 scripts/generate_config_catalog.py [--repo-root DIR] [--out-dir DIR] +""" + +import argparse +import json +import os +import re +import subprocess +import sys +from collections import defaultdict, OrderedDict + +# -------------------------------------------------------------------------- +# Tree walking +# -------------------------------------------------------------------------- + +# Modules that declare configs. Anything under the repo root that is not one of +# these is still scanned for *read sites*; declarations are only looked for in +# main source trees, which SOURCE_DIR_MARKER enforces. +SOURCE_DIR_MARKER = os.path.join("src", "main") + +EXCLUDED_PATH_PARTS = ( + os.sep + "target" + os.sep, + os.sep + "test" + os.sep, + os.sep + "src" + os.sep + "test" + os.sep, + os.sep + "node_modules" + os.sep, + os.sep + ".git" + os.sep, +) + +# Directories we never descend into at all. +PRUNED_DIR_NAMES = {"target", "node_modules", ".git", ".idea", "docker", "rfc"} + +READ_SITE_EXTENSIONS = (".java", ".scala") + +MAX_READ_SITES = 20 +MAX_CO_CONFIGS = 25 +MAX_GATES_PER_SITE = 3 + + +def is_excluded(path): + normalized = os.sep + path.strip(os.sep) + os.sep + return any(part in normalized for part in EXCLUDED_PATH_PARTS) + + +def walk_source_files(repo_root, extensions): + """Yield absolute paths of non-test, non-generated source files.""" + for dirpath, dirnames, filenames in os.walk(repo_root): + dirnames[:] = [d for d in dirnames if d not in PRUNED_DIR_NAMES] + if is_excluded(dirpath): + continue + for filename in filenames: + if filename.endswith(extensions): + full = os.path.join(dirpath, filename) + if not is_excluded(full): + yield full + + +# -------------------------------------------------------------------------- +# Lightweight Java lexing helpers +# -------------------------------------------------------------------------- + +def strip_comments_and_strings(text, keep_string_bodies=False): + """Blank out comments and (optionally) string bodies, preserving offsets. + + Offsets are preserved so that any index computed on the stripped text maps + straight back onto the original. Newlines survive so line numbers hold. + """ + out = list(text) + i = 0 + n = len(text) + while i < n: + ch = text[i] + if ch == "/" and i + 1 < n and text[i + 1] == "/": + while i < n and text[i] != "\n": + out[i] = " " + i += 1 + elif ch == "/" and i + 1 < n and text[i + 1] == "*": + out[i] = out[i + 1] = " " + i += 2 + while i < n and not (text[i] == "*" and i + 1 < n and text[i + 1] == "/"): + if text[i] != "\n": + out[i] = " " + i += 1 + if i < n: + out[i] = " " + if i + 1 < n: + out[i + 1] = " " + i += 2 + elif ch in ('"', "'"): + quote = ch + i += 1 + while i < n: + if text[i] == "\\": + if not keep_string_bodies: + out[i] = " " + if i + 1 < n: + out[i + 1] = " " + i += 2 + continue + if text[i] == quote: + break + if not keep_string_bodies and text[i] != "\n": + out[i] = " " + i += 1 + i += 1 + else: + i += 1 + return "".join(out) + + +def line_of(text, index): + return text.count("\n", 0, index) + 1 + + +def find_statement_end(text, start): + """Index just past the ``;`` ending the statement that starts at ``start``. + + Respects nesting and ignores semicolons inside strings, chars and comments. + """ + masked = strip_comments_and_strings(text[start:]) + depth_paren = depth_brace = depth_bracket = 0 + for offset, ch in enumerate(masked): + if ch == "(": + depth_paren += 1 + elif ch == ")": + depth_paren -= 1 + elif ch == "{": + depth_brace += 1 + elif ch == "}": + depth_brace -= 1 + elif ch == "[": + depth_bracket += 1 + elif ch == "]": + depth_bracket -= 1 + elif ch == ";" and depth_paren <= 0 and depth_brace <= 0 and depth_bracket <= 0: + return start + offset + 1 + return -1 + + +def split_top_level_args(arg_text): + """Split a call's argument text on top-level commas.""" + masked = strip_comments_and_strings(arg_text) + args = [] + depth = 0 + current_start = 0 + for i, ch in enumerate(masked): + if ch in "([{": + depth += 1 + elif ch in ")]}": + depth -= 1 + elif ch == "," and depth == 0: + args.append(arg_text[current_start:i].strip()) + current_start = i + 1 + tail = arg_text[current_start:].strip() + if tail: + args.append(tail) + return args + + +def extract_call_args(text, call_start): + """Given an index at the ``(`` of a call, return (args_text, index_past_close).""" + masked = strip_comments_and_strings(text) + if call_start >= len(text) or text[call_start] != "(": + return None, call_start + depth = 0 + for i in range(call_start, len(masked)): + if masked[i] == "(": + depth += 1 + elif masked[i] == ")": + depth -= 1 + if depth == 0: + return text[call_start + 1:i], i + 1 + return None, call_start + + +# -------------------------------------------------------------------------- +# Java string-literal evaluation +# -------------------------------------------------------------------------- + +_JAVA_ESCAPES = { + "n": "\n", "t": "\t", "r": "\r", "b": "\b", "f": "\f", + '"': '"', "'": "'", "\\": "\\", "0": "\0", +} + +STRING_LITERAL_RE = re.compile(r'"((?:[^"\\]|\\.)*)"', re.DOTALL) + + +def unescape_java(raw): + out = [] + i = 0 + while i < len(raw): + if raw[i] == "\\" and i + 1 < len(raw): + nxt = raw[i + 1] + if nxt == "u": + try: + out.append(chr(int(raw[i + 2:i + 6], 16))) + i += 6 + continue + except ValueError: + pass + out.append(_JAVA_ESCAPES.get(nxt, nxt)) + i += 2 + else: + out.append(raw[i]) + i += 1 + return "".join(out) + + +IDENT_PATH_RE = re.compile(r"[A-Za-z_$][A-Za-z0-9_$]*(?:\s*\.\s*[A-Za-z_$][A-Za-z0-9_$]*)*") + + +def concatenated_string_literal(expr, string_constants=None, owner_class=None, _depth=0): + """Evaluate a Java expression that is a concatenation of string literals. + + With ``string_constants`` supplied, identifiers that name a ``static final + String`` constant are substituted too, which is how the many + ``SOME_PREFIX + "suffix"`` config keys get resolved. Returns None when the + expression contains anything else, so that callers can distinguish "a + literal we read exactly" from "something computed". + """ + expr = expr.strip() + if not expr or _depth > 8: + return None + pieces = [] + pos = 0 + saw_literal = False + while pos < len(expr): + match = STRING_LITERAL_RE.match(expr, pos) + if match: + pieces.append(unescape_java(match.group(1))) + saw_literal = True + pos = match.end() + continue + ch = expr[pos] + if ch.isspace() or ch == "+": + pos += 1 + continue + if string_constants is not None: + ident = IDENT_PATH_RE.match(expr, pos) + if ident: + resolved = lookup_string_constant( + ident.group(0), string_constants, owner_class, _depth) + if resolved is not None: + pieces.append(resolved) + saw_literal = True + pos = ident.end() + continue + return None + return "".join(pieces) if saw_literal else None + + +def lookup_string_constant(path, string_constants, owner_class, depth): + """Resolve ``PREFIX`` / ``Holder.PREFIX`` to its literal value, recursively.""" + parts = [p.strip() for p in path.split(".")] + const = parts[-1] + qualifier = parts[-2] if len(parts) >= 2 else None + for candidate in ((qualifier, const), (owner_class, const), (None, const)): + expression = string_constants.get(candidate) + if expression is None: + continue + resolved = concatenated_string_literal( + expression, string_constants, candidate[0] or owner_class, depth + 1) + if resolved is not None: + return resolved + return None + + +ENUM_DESCRIPTION_RE = re.compile(r"@EnumDescription\s*\(") +ENUM_FIELD_DESCRIPTION_RE = re.compile(r"@EnumFieldDescription\s*\(") +ENUM_DECL_RE = re.compile(r"\benum\s+([A-Za-z_$][A-Za-z0-9_$]*)") +# The last constant in an enum body is followed by `}` rather than `,` or `;`. +ENUM_CONSTANT_RE = re.compile(r"\A\s*([A-Z][A-Z0-9_]*)\s*[,;(){}]") + + +def collect_enum_documentation(repo_root): + """Pass 0b. Enum name -> its ``@EnumDescription`` and per-constant descriptions. + + ``withDocumentation(SomeEnum.class)`` means the user-facing text for that + config lives on the enum, not at the declaration. Without this the catalog + would report "no documentation" for exactly the configs whose valid values + matter most. + """ + documented = {} + for file_path in walk_source_files(repo_root, (".java",)): + if SOURCE_DIR_MARKER not in file_path: + continue + try: + with open(file_path, "r", encoding="utf-8", errors="replace") as handle: + original = handle.read() + except OSError: + continue + if "@EnumDescription" not in original: + continue + masked = strip_comments_and_strings(original, keep_string_bodies=True) + for match in ENUM_DESCRIPTION_RE.finditer(masked): + args, after = extract_call_args(masked, match.end() - 1) + if args is None: + continue + enum_match = ENUM_DECL_RE.search(masked, after) + if not enum_match: + continue + name = enum_match.group(1) + body_start = masked.find("{", enum_match.end()) + if body_start < 0: + continue + depth = 0 + body_end = len(masked) + for i in range(body_start, len(masked)): + if masked[i] == "{": + depth += 1 + elif masked[i] == "}": + depth -= 1 + if depth == 0: + body_end = i + break + values = [] + for field in ENUM_FIELD_DESCRIPTION_RE.finditer(masked, body_start, body_end): + field_args, field_after = extract_call_args(masked, field.end() - 1) + if field_args is None: + continue + tail = masked[field_after:field_after + 160] + constant = ENUM_CONSTANT_RE.match(tail) + if not constant: + continue + values.append(OrderedDict([ + ("value", constant.group(1)), + ("description", concatenated_string_literal(field_args) or ""), + ])) + documented[name] = OrderedDict([ + ("enum", name), + ("file", os.path.relpath(file_path, repo_root)), + ("description", concatenated_string_literal(args) or ""), + ("values", values), + ]) + return documented + + +STRING_CONSTANT_RE = re.compile( + r"\bstatic\s+final\s+String\s+(?P<name>[A-Za-z_$][A-Za-z0-9_$]*)\s*=\s*(?P<value>[^;]*);") + + +def collect_string_constants(repo_root): + """Pass 0. (class, constant) -> its initializer expression, for key prefixes. + + Keyed both by owning class and by bare name. A bare name claimed by two + different expressions is dropped, so an ambiguous prefix never silently + resolves to the wrong value. + """ + by_qualified = {} + bare = {} + bare_conflicts = set() + for file_path in walk_source_files(repo_root, (".java",)): + if SOURCE_DIR_MARKER not in file_path: + continue + try: + with open(file_path, "r", encoding="utf-8", errors="replace") as handle: + original = handle.read() + except OSError: + continue + if "static final String" not in original: + continue + masked = strip_comments_and_strings(original, keep_string_bodies=True) + for match in STRING_CONSTANT_RE.finditer(masked): + name = match.group("name") + value = match.group("value").strip() + if not value or len(value) > 400: + continue + owner = find_enclosing_class(masked, match.start()) + by_qualified[(owner, name)] = value + if name in bare and bare[name] != value: + bare_conflicts.add(name) + bare[name] = value + for name in bare_conflicts: + bare.pop(name, None) + constants = {(None, name): value for name, value in bare.items()} + constants.update(by_qualified) + return constants + + +def literal_or_expression(expr): + """Return (value, is_literal) for a default-value expression.""" + literal = concatenated_string_literal(expr) + if literal is not None: + return literal, True + collapsed = " ".join(expr.split()) + if re.fullmatch(r"(true|false)", collapsed): + return collapsed, True + if re.fullmatch(r"-?\d+[LlFfDd]?", collapsed): + return collapsed.rstrip("LlFfDd"), True + if re.fullmatch(r"-?\d*\.\d+[FfDd]?", collapsed): + return collapsed.rstrip("FfDd"), True + return collapsed, False + + +# -------------------------------------------------------------------------- +# Pass 1 -- config declarations +# -------------------------------------------------------------------------- + +# Matches both `ConfigProperty<String> FOO = ConfigProperty` and the Flink +# `ConfigOption<String> FOO = ConfigOptions` forms, plus the rarer single-line +# `= ConfigProperty.key(...)`. +DECLARATION_RE = re.compile( + r"(?P<decl>(?:public|protected|private)?\s*(?:static\s+)?(?:final\s+)?" + r"(?P<holder>ConfigProperty|ConfigOption)\s*<\s*(?P<type>[^>]*(?:<[^>]*>)?[^>]*)\s*>\s+" + r"(?P<name>[A-Za-z_$][A-Za-z0-9_$]*)\s*=\s*)" + r"(?P<builder>ConfigProperty|ConfigOptions)\b" +) + +CLASS_DECL_RE = re.compile( + r"\b(?:public|protected|private)?\s*(?:static\s+)?(?:final\s+)?(?:abstract\s+)?" + r"(?:class|interface|enum)\s+([A-Za-z_$][A-Za-z0-9_$]*)" +) + +# Builder steps we care about, applied to the chain text after the key. +CHAIN_STEP_RE = re.compile(r"\.\s*([A-Za-z_$][A-Za-z0-9_$]*)\s*\(") + + +def find_enclosing_class(text, index): + """Innermost-looking class name declared before ``index``.""" + best = None + for match in CLASS_DECL_RE.finditer(text, 0, index): + best = match.group(1) + return best + + +def parse_chain_steps(chain_text): + """Return an ordered list of (method_name, args_text) from a builder chain. + + Comments are blanked first -- authors do put explanatory comments between a + builder call's arguments, and those must not end up inside an extracted + value. String bodies are kept so literals survive. + """ + chain_text = strip_comments_and_strings(chain_text, keep_string_bodies=True) + steps = [] + pos = 0 + while True: + match = CHAIN_STEP_RE.search(chain_text, pos) + if not match: + break + args, after = extract_call_args(chain_text, match.end() - 1) + if args is None: + pos = match.end() + continue + steps.append((match.group(1), args)) + pos = after + return steps + + +DERIVED_KEY_RE = re.compile( + r"([A-Za-z_$][A-Za-z0-9_$.]*)\s*\.\s*key\(\s*\)\s*(?:\+\s*(.+))?") + + +def derived_from_config_key(expr, string_constants, owner_class, base_key_resolver): + """Resolve `OTHER_CONFIG.key()` and `OTHER_CONFIG.key() + ".suffix"`. + + Several configs name their own key, and several name an alternative key, + relative to another config's. Returns None when the base config is unknown. + """ + match = DERIVED_KEY_RE.fullmatch(" ".join(expr.split())) + if not match: + return None + base = base_key_resolver(match.group(1)) + if base is None: + return None + if match.group(2) is None: + return base + suffix = concatenated_string_literal(match.group(2), string_constants, owner_class) + return None if suffix is None else base + suffix + + +def parse_declaration(masked_for_scan, original, match, file_path, repo_root, + string_constants, base_key_resolver=lambda path: None, + enum_docs=None): + """Build one catalog entry from a declaration match. Never raises.""" + warnings = [] + start = match.start() + end = find_statement_end(original, start) + if end < 0: + end = min(len(original), start + 4000) + warnings.append("statement terminator not found; chain truncated at 4000 chars") + chain_text = original[start:end] + + steps = parse_chain_steps(chain_text) + step_names = [name for name, _ in steps] + + enclosing_class = find_enclosing_class(masked_for_scan, start) + + key = None + key_expr = None + key_resolution = None + for name, args in steps: + if name == "key": + key_expr = args.strip() + key = concatenated_string_literal(args) + if key is not None: + key_resolution = "literal" + else: + key = concatenated_string_literal( + args, string_constants, enclosing_class) + if key is not None: + key_resolution = "resolvedFromConstants" + break + + if key is None and key_expr: + # Flink re-exports read the key off another ConfigProperty, and some + # Java configs build the key from a shared prefix constant. + reference = re.fullmatch( + r"([A-Za-z_$][A-Za-z0-9_$.]*)\s*\.\s*key\(\s*\)", key_expr.strip()) + if reference: + return None, "alias:" + reference.group(1) + # `OTHER_CONFIG.key() + ".suffix"` -- a key derived from another config's. + key = derived_from_config_key( + key_expr, string_constants, enclosing_class, base_key_resolver) + if key is not None: + key_resolution = "derivedFromConfigKey" + else: + warnings.append( + "key is a computed expression: " + " ".join(key_expr.split())[:160]) + + entry = OrderedDict() + entry["key"] = key + entry["keyResolution"] = key_resolution + entry["keyExpression"] = (" ".join(key_expr.split()) + if key_expr and key_resolution != "literal" else None) + entry["type"] = " ".join(match.group("type").split()) + entry["declaredIn"] = OrderedDict([ + ("class", enclosing_class), + ("constant", match.group("name")), + ("file", os.path.relpath(file_path, repo_root)), + ("line", line_of(original, start)), + ]) + entry["configGroup"] = entry["declaredIn"]["class"] + entry["builderStyle"] = "ConfigProperty" if match.group("builder") == "ConfigProperty" else "FlinkConfigOptions" + + # Default value. + default_value = None + default_is_literal = False + has_default = None + for name, args in steps: + if name == "defaultValue": + parts = split_top_level_args(args) + if parts: + default_value, default_is_literal = literal_or_expression(parts[0]) + has_default = True + if len(parts) > 1: + doc_on_default = concatenated_string_literal(parts[1]) + if doc_on_default: + entry["docOnDefaultValue"] = doc_on_default + break + if name == "noDefaultValue": + has_default = False + break + if has_default is None: + has_default = False + if entry["builderStyle"] == "ConfigProperty": + warnings.append("neither defaultValue() nor noDefaultValue() found in chain") + entry["hasDefaultValue"] = has_default + entry["defaultValue"] = default_value + entry["defaultValueIsLiteral"] = default_is_literal + if has_default and not default_is_literal: + entry["defaultValueNote"] = "computed at class-init time; shown as the source expression" + + # Documentation: a string, or a class reference whose text lives elsewhere. + documentation = None + documentation_source = None + enum_documentation = None + for name, args in steps: + if name not in ("withDocumentation", "withDescription"): + continue + parts = split_top_level_args(args) + if not parts: + continue + class_ref = re.fullmatch(r"([A-Za-z_$][A-Za-z0-9_$.]*)\s*\.\s*class", parts[0].strip()) + if class_ref: + enum_name = class_ref.group(1).split(".")[-1] + documentation_source = "enumClass:" + enum_name + extra = concatenated_string_literal(parts[1]) if len(parts) > 1 else None + described = (enum_docs or {}).get(enum_name) + if described: + entry_enum = described + pieces = [p for p in (extra, described["description"]) if p] + documentation = "\n".join(pieces) + enum_documentation = entry_enum + else: + documentation = extra + warnings.append( + "documentation points at enum %s, whose @EnumDescription was not found" + % enum_name) + else: + documentation = concatenated_string_literal(parts[0]) + documentation_source = "literal" + if documentation is None: + documentation = concatenated_string_literal( + parts[0], string_constants, enclosing_class) + documentation_source = "resolvedFromConstants" + if documentation is None: + documentation_source = None + warnings.append("documentation is a computed expression") + break + entry["documentation"] = documentation + entry["documentationSource"] = documentation_source + entry["enumDocumentation"] = enum_documentation + + def collect_string_args(step_name): + for name, args in steps: + if name == step_name: + values = [] + for part in split_top_level_args(args): + literal = concatenated_string_literal(part) + if literal is None: + literal = concatenated_string_literal( + part, string_constants, enclosing_class) + if literal is None: + literal = derived_from_config_key( + part, string_constants, enclosing_class, base_key_resolver) + values.append(literal if literal is not None + else " ".join(part.split())) + return values + return [] + + entry["validValues"] = collect_string_args("withValidValues") + # A config whose documentation is an enum class declares its permitted values on the enum + # rather than via withValidValues(...). Fall back to the enum's constants so that + # "what can I set this to?" is answerable for strategy/policy configs, which are exactly + # the ones where it matters most. + if not entry["validValues"] and enum_documentation: + entry["validValues"] = [v["value"] for v in enum_documentation.get("values", []) if v.get("value")] + entry["alternatives"] = collect_string_args("withAlternatives") + since = collect_string_args("sinceVersion") + entry["sinceVersion"] = since[0] if since else None + deprecated = collect_string_args("deprecatedAfter") + entry["deprecatedAfter"] = deprecated[0] if deprecated else None + entry["supportedVersions"] = collect_string_args("supportedVersions") + entry["advanced"] = "markAdvanced" in step_names + entry["hasInferFunction"] = "withInferFunction" in step_names + entry["builderSteps"] = step_names + + if key is None: + warnings.append("key could not be resolved to a string literal") + if documentation is None and documentation_source != "enumClass": + warnings.append("no documentation text resolved") + + entry["parseWarnings"] = warnings + return entry, None + + +def collect_declarations(repo_root, verbose=False): + """Pass 1. Returns (entries, constant_index, stats).""" + entries = [] + # (file-local class, constant name) -> config key, so later passes can map a + # constant reference back to the config it names. + constant_index = {} + alias_pending = [] + stats = defaultdict(int) + + string_constants = collect_string_constants(repo_root) + stats["stringConstants"] = len(string_constants) + enum_docs = collect_enum_documentation(repo_root) + stats["documentedEnums"] = len(enum_docs) + if verbose: + print(" string constants indexed: %d" % len(string_constants), file=sys.stderr) + print(" documented enums indexed: %d" % len(enum_docs), file=sys.stderr) + + for file_path in walk_source_files(repo_root, (".java",)): + if SOURCE_DIR_MARKER not in file_path: + continue + try: + with open(file_path, "r", encoding="utf-8", errors="replace") as handle: + original = handle.read() + except OSError: + stats["unreadableFiles"] += 1 + continue + if "ConfigProperty" not in original and "ConfigOptions" not in original: + continue + + masked = strip_comments_and_strings(original) + + def resolve_base_key(path, _index=constant_index): + parts = path.split(".") + const = parts[-1] + qualifier = parts[-2] if len(parts) >= 2 else None + return _index.get((qualifier, const)) or _index.get((None, const)) + + for match in DECLARATION_RE.finditer(masked): + stats["declarationsSeen"] += 1 + entry, alias_target = parse_declaration( + masked, original, match, file_path, repo_root, string_constants, + resolve_base_key, enum_docs) + if alias_target is not None: + alias_pending.append(( + find_enclosing_class(masked, match.start()), + match.group("name"), + alias_target.split(":", 1)[1], + os.path.relpath(file_path, repo_root), + line_of(original, match.start()), + )) + continue + if entry is None: + stats["unparseable"] += 1 + continue + # Flink ConfigOptions that do not name a hoodie.* key belong to the + # Flink option namespace, not the Hudi config namespace. + if entry["builderStyle"] == "FlinkConfigOptions" and not ( + entry["key"] or "").startswith("hoodie."): Review Comment: Agreed this is a real gap. Punting it rather than guessing: `FlinkOptions` uses its own key namespace, so folding it in means two key spaces in one catalog, and the right people to get that shape right are the Flink maintainers. We'll ask Danny and Shuo to add the `FlinkOptions` pass as a follow-up — they'll catch namespace collisions and engine-specific semantics that we would not. Meanwhile the skill states its scope up front rather than implying coverage it does not have, and reports the engine explicitly wherever behaviour differs. Worth noting the `hoodie.*` keys Flink does read are already covered, with per-engine defaults — `hoodie.index.type` cites `OptionsResolver.isBucketIndexType` and carries `INMEMORY` for Flink, `hoodie.metadata.index.secondary.enable` is `false` on Flink against a declared `true`, and `hoodie.write.markers.type` is `DIRECT`. So a Flink user asking about a `hoodie.*` key gets a correct answer today; it is the `FlinkOptions` namespace specifically that is out of scope. -- 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]
