danny0405 commented on issue #19779: URL: https://github.com/apache/hudi/issues/19779#issuecomment-5673874459
The append-only COW first milestone looks like a reasonable way to validate the Flink flow. The distinction between sink-side validation and end-to-end guarantees, and the explicit replay-idempotence gate, are useful. After reading the proposal alongside the existing Architect validator and the Hudi 1.2.0 Flink code, I suggest tightening three acceptance details before implementation: 1. **Pin the validation inputs as well as the capability document.** The existing [`validate_config_keys.py`](https://github.com/apache/hudi/blob/8ea7aaa7b4857504fd0c0dc146dc4a2886598af4/hudi-agent-gateway/skills/hudi-architect/validate_config_keys.py#L44-L78) discovers the enclosing checkout and scans its sources. Extending that approach on a newer branch could accept options introduced after 1.2.0. Please specify how the Flink validator and factory fixtures use the release baseline: for example, a capability manifest generated from a pinned release commit, with fixtures built against the corresponding Hudi/Flink versions. Include the source revision in validation evidence and a negative test for an option absent from that baseline. 2. **Make the Milestone 1 negative fixtures explicit; factory construction alone is insufficient.** In 1.2.0, [`HoodieTableFactory.sanityCheck`](https://github.com/apache/hudi/blob/release-1.2.0/hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/table/HoodieTableFactory.java#L179-L187) skips `checkRecordKey` in append mode. The skill's validator therefore needs its own test rejecting an explicitly supplied record-key field that is absent from the physical schema, even on the supported append path. Also, [`OptionsResolver.isAppendMode`](https://github.com/apache/hudi/blob/release-1.2.0/hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/configuration/OptionsResolver.java#L83-L90) depends on `write.insert.cluster=false` for COW, not just `write.operation=insert`. Pin or validate that effective setting and test an incompatible override. For the generated `INSERT INTO`, add a planner fixture using a declared source schema/changelog contract; a successful sink fa ctory call does not establish that the source-to-sink statement meets the append-only contract. These checks can remain local and need no external source service. 3. **Define status precedence when multiple gates fail.** For example, missing schema calls for `INCOMPLETE`, unknown writers call for `REVIEW_REQUIRED`, and replay-idempotence in Milestone 1 calls for `BLOCKED`; one request can hit all three. Please define how the single final status is selected while preserving all unresolved reasons, and add a combined-gate fixture asserting that executable eligibility stays false. Stable reason identifiers in test fixtures would make this testable without comparing natural-language wording or introducing a separate architecture decision engine. These are proposed acceptance-test refinements from source inspection; I have not run the future skill flow or new Flink fixtures. -- 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]
