SEZ9 commented on PR #11781: URL: https://github.com/apache/seatunnel/pull/11781#issuecomment-6159753096
Hi @zhangshenghang, thanks for clarifying — that lines up, your F5 and my F7 are the same point: replaying a frame actually written by a pre-#9689 release, not one produced by the current WALWriter. Agreed on deferring that. Producing a genuine old-format fixture does need a build from the pre-relocation release line, and that's not something I'd block this shim on. Two small asks so it doesn't get lost: - Please add a short note (a TODO in `WALReaderAndWriterTest.java` or a line in the PR description) stating that the legacy fixture is currently generated by the current WALWriter and that a real pre-#9689 frame fixture is planned as a follow-up. - Once this lands, please open the follow-up issue and link it back here so we have the trail. On the rest of the earlier review, could you confirm where things stand so I can do a final pass: - F1/F3: whether the regression test now asserts on the actual enum value (not only `getClass()`/`toString()`) and covers all three legacy mappings, including the `JobResult` and `JobStatusData` targets, so the string literals in `WALReader.java` are verified by a test rather than left unchecked. - F2: whether a `ClassNotFoundException` after the legacy-name rewrite now reports the rewritten target name alongside the original WAL class name. - F4/F5: whether an upgrade note was added for file-based IMap storage users and whether the #9689 relocation plus this shim's upgrade/downgrade semantics are recorded in `incompatible-changes.md`. - F6: a brief comment on the test-scoped `seatunnel-engine-common` dependency in `pom.xml` explaining why production code references the class names as strings. - F8: whether the lookup now handles chained relocations, or if you'd prefer to adjust the Javadoc wording to match the single-hop `getOrDefault` behavior. If any of these were addressed in a later push, just point me at it and I'll re-review. <!-- streview-comment:1738 --> -- 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]
