bowenliang123 commented on code in PR #5352:
URL: https://github.com/apache/kyuubi/pull/5352#discussion_r1354588568


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/api/Metadata.scala:
##########
@@ -78,6 +78,7 @@ case class Metadata(
     engineState: String = null,
     engineError: Option[String] = None,
     endTime: Long = 0L,
+    priority: Int = 10,

Review Comment:
   There's a magic code here. Default priority is worth to be extracted to 
explicit static value , and further mentioned in config docs. Or it's confusing 
for user and developers reviewing the UTs.



##########
kyuubi-server/src/test/scala/org/apache/kyuubi/server/metadata/MetadataManagerSuite.scala:
##########
@@ -142,6 +145,94 @@ class MetadataManagerSuite extends KyuubiFunSuite {
     }
   }
 
+  test("[KYUUBI #5328] Test MetadataManager#pickBatchForSubmitting in order") {
+    withMetadataManager(Map(METADATA_STORE_JDBC_PRIORITY_ENABLED.key -> 
"true")) {
+      metadataManager =>
+        val mockKyuubiInstance = "mock_kyuubi_instance"
+        val time = System.currentTimeMillis()
+        val mockBatchJob1 = Metadata(
+          identifier = "mock_batch_job_1",
+          sessionType = SessionType.BATCH,
+          realUser = "kyuubi",
+          username = "kyuubi",
+          engineType = EngineType.SPARK_SQL.toString,
+          state = OperationState.INITIALIZED.toString,
+          priority = 20,
+          createTime = time + 10000)
+        val mockBatchJob2 = Metadata(
+          identifier = "mock_batch_job_2",
+          sessionType = SessionType.BATCH,
+          realUser = "kyuubi",
+          username = "kyuubi",
+          engineType = EngineType.SPARK_SQL.toString,
+          state = OperationState.INITIALIZED.toString,
+          createTime = time)
+        val mockBatchJob3 = Metadata(
+          identifier = "mock_batch_job_3",
+          sessionType = SessionType.BATCH,
+          realUser = "kyuubi",
+          username = "kyuubi",
+          engineType = EngineType.SPARK_SQL.toString,
+          state = OperationState.INITIALIZED.toString,
+          createTime = time + 10000)

Review Comment:
   Can be extracted to a dedicate method for generating mocked batch job 
objects?
   And make sure same job objects are the same in positive and negative tests.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to