aglinxinyuan commented on PR #5729: URL: https://github.com/apache/texera/pull/5729#issuecomment-5276388120
Pushed a merge of `main` to this branch to clear the conflicts — `223e4c3073`. Worth a look before you continue, because `main` changed the shape of this PR. [#6143](https://github.com/apache/texera/pull/6143) (`refactor(workflow-compiler): unify duplicate compilers into common/workflow-compiler`) consolidated the two duplicate `LogicalLink` classes — amber's `org.apache.texera.workflow.LogicalLink` and the compiling service's `org.apache.texera.amber.compiler.model.LogicalLink` — into a single `org.apache.texera.common.compiler.model.LogicalLink` under `common/workflow-compiler`. So the two parallel edits here collapse onto one class, and the diff drops from 4 files to 2: | Path (before) | Now | | --- | --- | | `amber/.../workflow/LogicalLink.scala` | → `common/workflow-compiler/.../common/compiler/model/LogicalLink.scala` — the `readOperatorIdentity` helper and the `@JsonCreator` `JsonNode` constructor live here | | `amber/.../workflow/LogicalLinkSpec.scala` | → `common/workflow-compiler/src/test/.../LogicalLinkSpec.scala` — the single spec, 23 tests | | `workflow-compiling-service/.../amber/compiler/model/LogicalLink.scala` | deleted upstream | | `workflow-compiling-service/.../amber/compiler/model/LogicalLinkSpec.scala` | dropped — see below | The compiling-service spec had to go. Its unique cases pinned the *lenient* contract of the duplicate that upstream deleted (`accept a null OperatorIdentity id without throwing`, `accept a self-loop link ... without throwing`, the two `(lenient)` `OperatorIdentity(null)` cases), and the surviving class rejects all of those via its `require` guards — so those assertions are no longer just redundant, they're inverted. Every other case in that file duplicated the strict spec, which already covers each `readOperatorIdentity` branch in its rejecting form. One oddity in the resolution, flagging it so it doesn't look like a stray edit: git mis-detected the deleted `workflow-compiling-service/.../LogicalLink.scala` as a *rename* of main's new `common/auth/.../HeaderFieldSpec.scala` (they share only the ASF header), and spliced this branch's `LogicalLink` body into that spec. I restored `HeaderFieldSpec.scala` to main's content verbatim — it's byte-identical to `main` now. Also retargeted the "keep in sync with the other copy" comment on `readOperatorIdentity`, which no longer had another copy to point at. Verified locally on the merge result: ``` sbt "WorkflowCompiler/scalafmtCheckAll" "WorkflowCompiler/scalafixAll --check" \ "WorkflowCompiler/testOnly *LogicalLinkSpec" \ "WorkflowCompilingService/Test/compile" "WorkflowExecutionService/Test/compile" ``` 23/23 pass, lint clean, both dependent modules compile. Left the PR description alone — the testing section still cites the old `18/18` + `15/15` split, so that needs a pass from you. -- 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]
