Xiao-zhen-Liu commented on PR #6143: URL: https://github.com/apache/texera/pull/6143#issuecomment-5075589003
Thanks for the updates, these all look good to me. Making Strict actually fail fast on schema errors instead of deferring it was the right call, and the two new strict tests pin the behavior well. The rewritten description also makes the "still two compilations" point clear up front, which was my main confusion. The red CI is not from your changes. The branch is now 165 commits behind main, and main has since picked up `LoopIntegrationSpec.scala` (from #5700), which still imports the `org.apache.texera.workflow.LogicalLink` that this PR removes. That single import is the only thing breaking the merged build: ``` amber/src/test/integration/org/apache/texera/amber/engine/e2e/LoopIntegrationSpec.scala:49 - import org.apache.texera.workflow.LogicalLink + import org.apache.texera.common.compiler.model.LogicalLink ``` A rebase plus that line should turn amber green. Worth doing soon: that file is actively changing on main (#6661 touched it again), and since this PR deletes a widely used symbol, the longer it sits the more likely something else picks up the old import. One thing to watch after the rebase: the amber tests never actually ran, they failed at compile, so the Strict fail-fast change hasn't been exercised against the existing e2e suite yet. -- 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]
