Copilot commented on code in PR #6052:
URL: https://github.com/apache/texera/pull/6052#discussion_r3509545440
##########
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/sql/mysql/MySQLSourceOpDescSpec.scala:
##########
@@ -65,16 +64,10 @@ class MySQLSourceOpDescSpec extends AnyFlatSpec with
Matchers {
}
"MySQLSourceOpDesc.getPhysicalOp" should
- "wire the MySQL exec as a source op with no input port and one output
port" in {
+ "throw because the operator is no longer executable" in {
val d = new MySQLSourceOpDesc
- val physical = d.getPhysicalOp(DEFAULT_WORKFLOW_ID, DEFAULT_EXECUTION_ID)
- physical.opExecInitInfo match {
- case OpExecWithClassName(className, _) =>
- className shouldBe
"org.apache.texera.amber.operator.source.sql.mysql.MySQLSourceOpExec"
- case other => fail(s"expected OpExecWithClassName, got $other")
- }
- physical.inputPorts.keySet shouldBe empty
- physical.outputPorts.keySet shouldBe
d.operatorInfo.outputPorts.map(_.id).toSet
+ an[UnsupportedOperationException] should be thrownBy
+ d.getPhysicalOp(DEFAULT_WORKFLOW_ID, DEFAULT_EXECUTION_ID)
Review Comment:
The spec doesn't currently exercise `sourceSchema()` with a fully-populated
connection config. With the executor/connection path removed, this is where a
legacy-deserialized operator is most likely to regress (e.g., NPE vs returning
null). Adding a regression test that sets all connection fields (including
`port`) would pin the intended "deserialization-only" behavior.
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/sql/mysql/MySQLSourceOpDesc.scala:
##########
@@ -36,21 +31,7 @@ class MySQLSourceOpDesc extends SQLSourceOpDesc {
workflowId: WorkflowIdentity,
executionId: ExecutionIdentity
): PhysicalOp =
- PhysicalOp
- .sourcePhysicalOp(
- workflowId,
- executionId,
- this.operatorIdentifier,
- OpExecWithClassName(
-
"org.apache.texera.amber.operator.source.sql.mysql.MySQLSourceOpExec",
- objectMapper.writeValueAsString(this)
- )
- )
- .withInputPorts(operatorInfo.inputPorts)
- .withOutputPorts(operatorInfo.outputPorts)
- .withPropagateSchema(
- SchemaPropagationFunc(_ => Map(operatorInfo.outputPorts.head.id ->
sourceSchema()))
- )
+ throw new UnsupportedOperationException("MySQL Source operator is no
longer executable.")
Review Comment:
`MySQLSourceOpDesc` now relies on `SQLSourceOpDesc`'s default
`establishConn`, which returns `null` (SQLSourceOpDesc.scala:207). If a legacy
workflow deserializes with all connection fields set, calling `sourceSchema()`
will hit `connection.setReadOnly(true)` and throw a `NullPointerException`
(SQLSourceOpDesc.scala:151-153). Since this descriptor is intended to be
deserialization-only/non-executable, it should override `sourceSchema()` (or
`establishConn`) to avoid NPEs and avoid attempting DB access.
--
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]
