kunwp1 commented on code in PR #5434:
URL: https://github.com/apache/texera/pull/5434#discussion_r3376358720
##########
amber/src/test/scala/org/apache/texera/web/service/ExecutionResultServiceSpec.scala:
##########
@@ -36,8 +56,110 @@ import
org.apache.texera.web.service.ExecutionResultService.{
}
import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.matchers.should.Matchers
+import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
+
+import java.net.URI
+import java.sql.Timestamp
+import scala.jdk.CollectionConverters._
+
+class ExecutionResultServiceSpec
+ extends AnyFlatSpec
+ with Matchers
+ with BeforeAndAfterAll
+ with BeforeAndAfterEach
+ with MockTexeraDB {
+
+ private val testWid: Integer = 9000 + scala.util.Random.nextInt(1000)
+ private val testUid: Integer = 9000 + scala.util.Random.nextInt(1000)
+ private var executionsDao: WorkflowExecutionsDao = _
+
+ override protected def beforeAll(): Unit = {
+ initializeDBAndReplaceDSLContext()
+ }
+
+ override protected def afterAll(): Unit = {
+ shutdownDB()
+ }
+
+ override protected def beforeEach(): Unit = {
+ val user = new User
+ user.setUid(testUid)
+ user.setName("execution-result-test-user")
+ user.setEmail(s"[email protected]")
+ user.setPassword("password")
+ new UserDao(getDSLContext.configuration()).insert(user)
+
+ val workflow = new Workflow
+ workflow.setWid(testWid)
+ workflow.setName(s"execution-result-test-$testWid")
+ workflow.setContent("{}")
+ workflow.setDescription("")
+ workflow.setCreationTime(new Timestamp(System.currentTimeMillis()))
+ workflow.setLastModifiedTime(new Timestamp(System.currentTimeMillis()))
+ new WorkflowDao(getDSLContext.configuration()).insert(workflow)
+
+ val version = new WorkflowVersion
+ version.setWid(testWid)
+ version.setContent("{}")
+ version.setCreationTime(new Timestamp(System.currentTimeMillis()))
+ new WorkflowVersionDao(getDSLContext.configuration()).insert(version)
+
+ executionsDao = new WorkflowExecutionsDao(getDSLContext.configuration())
+ }
+
+ override protected def afterEach(): Unit = {
+ val ctx = getDSLContext
+ ctx
+ .deleteFrom(OPERATOR_PORT_EXECUTIONS)
+ .where(
+ OPERATOR_PORT_EXECUTIONS.WORKFLOW_EXECUTION_ID.in(
+ ctx.select(WORKFLOW_EXECUTIONS.EID).from(WORKFLOW_EXECUTIONS)
+ )
+ )
+ .execute()
+ ctx.deleteFrom(WORKFLOW_EXECUTIONS).execute()
+ import org.apache.texera.dao.jooq.generated.Tables.{USER, WORKFLOW,
WORKFLOW_VERSION}
+
ctx.deleteFrom(WORKFLOW_VERSION).where(WORKFLOW_VERSION.WID.eq(testWid)).execute()
+ ctx.deleteFrom(WORKFLOW).where(WORKFLOW.WID.eq(testWid)).execute()
+ ctx.deleteFrom(USER).where(USER.UID.eq(testUid)).execute()
+ }
-class ExecutionResultServiceSpec extends AnyFlatSpec with Matchers {
+ "persistOperatorPortResultUri" should
+ "insert the URI carried by an OperatorPortResultUriAvailable event" in {
+ val execution = new WorkflowExecutions
+ execution.setVid(1)
Review Comment:
It's minor but I think it's better to use `version.getVid` instead of
hardcoded `1`.
##########
amber/src/main/scala/org/apache/texera/web/service/ExecutionResultService.scala:
##########
@@ -353,6 +357,10 @@ class ExecutionResultService(
)
)
+ addSubscription(
Review Comment:
Is it possible to add a test what happens if this registerCallback is not
called at all? Seems like the main changes of this PR requires this
registerCallback to be executed before starting the workflow.
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala:
##########
@@ -239,15 +246,11 @@ object WorkflowExecutionsResource {
globalPortId: GlobalPortIdentity,
uri: URI
): Unit = {
- context
- .insertInto(OPERATOR_PORT_EXECUTIONS)
- .columns(
- OPERATOR_PORT_EXECUTIONS.WORKFLOW_EXECUTION_ID,
- OPERATOR_PORT_EXECUTIONS.GLOBAL_PORT_ID,
- OPERATOR_PORT_EXECUTIONS.RESULT_URI
- )
- .values(eid.id.toInt, globalPortId.serializeAsString, uri.toString)
- .execute()
+ val pojo = new OperatorPortExecutions()
+ pojo.setWorkflowExecutionId(eid.id.toInt)
+ pojo.setGlobalPortId(globalPortId.serializeAsString)
+ pojo.setResultUri(uri.toString)
+ new OperatorPortExecutionsDao(context.configuration()).insert(pojo)
Review Comment:
The PR description says this helper is deleted and uses `withTransaction`,
but I don't see the use of `withTransaction`. Can you update the description?
--
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]
