sharpca commented on PR #1347: URL: https://github.com/apache/flink-agents/pull/1347#issuecomment-6578091786
Thanks — coordinated on #1337 as you suggested (comment linked there). Summary and where that leaves this PR: **What I found in #1337's current head `89ae183`.** The clarified rules are not implemented there yet either: it selects `config.yaml` and falls back to `flink-conf.yaml` only when `config.yaml` is absent, and both files still go through `YamlParserUtils.loadYamlFile` — so there is no version branch and no legacy parsing path yet. Python is also currently the inverse of Java: `__find_config_file` tries `flink-conf.yaml` first, then `config.yaml`. So the two languages already disagree on selection, independently of the new rules. **What I asked semlanghi**, since I need agreement before writing a Python parser: 1. Where the Flink-version decision comes from on the Java side (Python can branch via `FlinkVersionManager`, but I want the same input). 2. Whether the legacy parser lands in #1337 or a follow-up, and which `GlobalConfiguration.loadLegacyYAMLResource()` edge cases must match — values containing `:`, unquoted scalars, comments, duplicate keys, nested indentation. 3. Whether to keep Java+Python in #1337 or keep #1347 as the Python counterpart. **What I did and did not change here.** No code change in this push: the flat `agent.*` extraction and its tests stand as they are, and I am not claiming legacy-parsing compatibility for them. I deliberately did not add a `yaml.safe_load` variant that pretends to be the legacy parser, because it would diverge from the Java side — exactly the inconsistency you want to avoid. **Plan once semlanghi answers.** #1347 becomes: version-dependent file selection matching the Java side, a small legacy parser for `flink-conf.yaml` with tests for the `:`-in-value case, and standard YAML for `config.yaml`. If they prefer a single PR, I'll move that work into #1337 and close #1347 instead. I'll also revisit the docs bullet I added, since it currently describes only the flat-key addition. If you'd rather I go ahead and implement against the rules as written in the discussion — ignoring the fact that #1337's head doesn't implement them yet — say so and I'll pick the version source from `FlinkVersionManager` and proceed. -- 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]
