This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-6808-b2854a1d069f6b791a997449d6e761814b88fe8f in repository https://gitbox.apache.org/repos/asf/texera.git
commit 00c16217d2942d05c2ffa18018000a7c1a44af75 Author: Kary Zheng <[email protected]> AuthorDate: Wed Jul 22 17:34:19 2026 -0700 fix(LogicalOp): register SklearnLogisticRegression(CV) subtypes exactly once (#6808) ### What changes were proposed in this PR? `LogicalOp`'s `@JsonSubTypes` registered `SklearnLogisticRegressionOpDesc` and `SklearnLogisticRegressionCVOpDesc` **twice each** (two separate active `new Type(...)` pairs). Any consumer that enumerates `@JsonSubTypes.value()` — operator discovery, metadata/schema generation, the operator palette — therefore saw each of these two operators twice. This PR removes the duplicate pair, keeping exactly one registration of each: ```diff - new Type(value = classOf[SklearnLogisticRegressionOpDesc], name = "SklearnLogisticRegression"), - new Type( - value = classOf[SklearnLogisticRegressionCVOpDesc], - name = "SklearnLogisticRegressionCV" - ), ``` ### Any related issues, documentation, discussions? Closes #6793 ### How was this PR tested? Added a `LogicalOpSpec` regression test that reads the `@JsonSubTypes` annotation and asserts no subtype **class** or **name** is registered more than once. Verified the test **fails** on the pre-fix (duplicated) registry and **passes** after the fix; full `LogicalOpSpec` is green (5/5). ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 4.8) --------- Co-authored-by: Claude Opus 4.8 <[email protected]> --- .../org/apache/texera/amber/operator/LogicalOp.scala | 5 ----- .../org/apache/texera/amber/operator/LogicalOpSpec.scala | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/LogicalOp.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/LogicalOp.scala index bb32f96abb..efa4614418 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/LogicalOp.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/LogicalOp.scala @@ -355,11 +355,6 @@ trait StateTransferFunc value = classOf[SklearnTrainingLogisticRegressionCVOpDesc], name = "SklearnTrainingLogisticRegressionCV" ), - new Type(value = classOf[SklearnLogisticRegressionOpDesc], name = "SklearnLogisticRegression"), - new Type( - value = classOf[SklearnLogisticRegressionCVOpDesc], - name = "SklearnLogisticRegressionCV" - ), new Type(value = classOf[SklearnRidgeOpDesc], name = "SklearnRidge"), new Type(value = classOf[SklearnRidgeCVOpDesc], name = "SklearnRidgeCV"), new Type(value = classOf[SklearnSDGOpDesc], name = "SklearnSDG"), diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/LogicalOpSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/LogicalOpSpec.scala index a91c50b4ad..606f86a8d2 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/LogicalOpSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/LogicalOpSpec.scala @@ -71,4 +71,19 @@ class LogicalOpSpec extends AnyFlatSpec { } assert(ex.getMessage == "operator DistinctOpDesc does not support reconfiguration") } + + "LogicalOp @JsonSubTypes" should "register each operator subtype exactly once" in { + // SklearnLogisticRegression / ...CV were each listed twice, so consumers + // enumerating the registry saw them twice. + val subTypes = classOf[LogicalOp] + .getAnnotation(classOf[com.fasterxml.jackson.annotation.JsonSubTypes]) + assert(subTypes != null, "LogicalOp is missing its @JsonSubTypes annotation") + val types = subTypes.value() + val dupClasses = + types.map(_.value()).groupBy(identity).collect { case (c, ts) if ts.length > 1 => c.getName } + val dupNames = + types.map(_.name()).groupBy(identity).collect { case (n, ts) if ts.length > 1 => n } + assert(dupClasses.isEmpty, s"duplicate subtype classes: ${dupClasses.mkString(", ")}") + assert(dupNames.isEmpty, s"duplicate subtype names: ${dupNames.mkString(", ")}") + } }
