This is an automated email from the ASF dual-hosted git repository.
bowenliang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kyuubi.git
The following commit(s) were added to refs/heads/master by this push:
new 928e3d243f [KYUUBI #6731] [REST] Check all required extra resource
files uploaded in creating batch request
928e3d243f is described below
commit 928e3d243f2c0e60732293e9934458eeb2ac8300
Author: Bowen Liang <[email protected]>
AuthorDate: Fri Oct 18 08:44:13 2024 +0800
[KYUUBI #6731] [REST] Check all required extra resource files uploaded in
creating batch request
# :mag: Description
## Issue References ๐
This pull request fixes #
## Describe Your Solution ๐ง
- check all the required extra resource files are uploaded in POST
multi-part request as expected, when creating batch with REST Batch API
## Types of changes :bookmark:
- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
## Test Plan ๐งช
#### Behavior Without This Pull Request :coffin:
#### Behavior With This Pull Request :tada:
#### Related Unit Tests
---
# Checklist ๐
- [ ] This patch was not authored or co-authored using [Generative
Tooling](https://www.apache.org/legal/generative-tooling.html)
**Be nice. Be informative.**
Closes #6731 from bowenliang123/extra-resource-check.
Closes #6731
116a47ea5 [Bowen Liang] update
cd4433a8c [Bowen Liang] update
4852b1569 [Bowen Liang] update
5bb2955e8 [Bowen Liang] update
1696e7328 [Bowen Liang] update
911a9c195 [Bowen Liang] update
042e42d23 [Bowen Liang] update
56dc7fb8a [Bowen Liang] update
Authored-by: Bowen Liang <[email protected]>
Signed-off-by: Bowen Liang <[email protected]>
---
.../kyuubi/server/api/v1/BatchesResource.scala | 12 ++++++++
.../server/rest/client/BatchRestApiSuite.scala | 35 ++++++++++++++++++++++
.../org/apache/kyuubi/util/AssertionUtils.scala | 13 ++++++++
3 files changed, 60 insertions(+)
diff --git
a/kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/BatchesResource.scala
b/kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/BatchesResource.scala
index d757a0d2bb..f778bcb0b7 100644
---
a/kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/BatchesResource.scala
+++
b/kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/BatchesResource.scala
@@ -26,6 +26,7 @@ import javax.ws.rs._
import javax.ws.rs.core.MediaType
import scala.collection.JavaConverters._
+import scala.collection.convert.ImplicitConversions.`collection
AsScalaIterable`
import scala.util.{Failure, Success, Try}
import scala.util.control.NonFatal
@@ -200,6 +201,17 @@ private[v1] class BatchesResource extends
ApiRequestContext with Logging {
batchRequest != null,
"batchRequest is required and please check the content type" +
" of batchRequest is application/json")
+
+ val unUploadedExtraResourceFileNames =
+
batchRequest.getExtraResourcesMap.values.asScala.flatMap(_.split(",")).toSet.diff(
+
formDataMultiPart.getFields.values.flatten.map(_.getContentDisposition.getFileName).toSet)
+ .filter(StringUtils.isNotBlank(_))
+ require(
+ unUploadedExtraResourceFileNames.isEmpty,
+ f"required extra resource files " +
+ f"[${unUploadedExtraResourceFileNames.toList.sorted.mkString(",")}]" +
+ f" are not uploaded in the multipart form data")
+
openBatchSessionInternal(
batchRequest,
isResourceFromUpload = true,
diff --git
a/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/BatchRestApiSuite.scala
b/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/BatchRestApiSuite.scala
index e6ea0d162f..6d385bac26 100644
---
a/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/BatchRestApiSuite.scala
+++
b/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/BatchRestApiSuite.scala
@@ -23,6 +23,7 @@ import java.util.zip.{ZipEntry, ZipOutputStream}
import scala.collection.JavaConverters._
+import org.apache.http.client.HttpResponseException
import org.scalatest.time.SpanSugar.convertIntToGrainOfTime
import org.apache.kyuubi.{BatchTestHelper, KYUUBI_VERSION,
RestClientTestHelper}
@@ -32,6 +33,7 @@ import org.apache.kyuubi.client.exception.KyuubiRestException
import org.apache.kyuubi.config.KyuubiReservedKeys
import org.apache.kyuubi.metrics.{MetricsConstants, MetricsSystem}
import org.apache.kyuubi.session.{KyuubiSession, SessionHandle}
+import org.apache.kyuubi.util.AssertionUtils.interceptCauseContains
import org.apache.kyuubi.util.GoldenFileUtils.getCurrentModuleHome
class BatchRestApiSuite extends RestClientTestHelper with BatchTestHelper {
@@ -183,6 +185,39 @@ class BatchRestApiSuite extends RestClientTestHelper with
BatchTestHelper {
}
}
+ test("basic batch rest client with uploading resource and extra resources of
unuploaded") {
+
+ val basicKyuubiRestClient: KyuubiRestClient =
+ KyuubiRestClient.builder(baseUri.toString)
+ .authHeaderMethod(KyuubiRestClient.AuthHeaderMethod.BASIC)
+ .username(ldapUser)
+ .password(ldapUserPasswd)
+ .socketTimeout(5 * 60 * 1000)
+ .build()
+ val batchRestApi: BatchRestApi = new BatchRestApi(basicKyuubiRestClient)
+
+ val pythonScriptsPath =
s"${getCurrentModuleHome(this)}/src/test/resources/python/"
+ val appScriptFileName = "app.py"
+ val appScriptFile = Paths.get(pythonScriptsPath, appScriptFileName).toFile
+ val requestObj = newSparkBatchRequest(Map("spark.master" -> "local"))
+ requestObj.setBatchType("PYSPARK")
+ requestObj.setName("pyspark-test")
+ requestObj.setExtraResourcesMap(Map(
+ "spark.submit.pyFiles" -> "non-existed-zip.zip",
+ "spark.files" -> "non-existed-jar.jar",
+ "spark.some.config1" -> "",
+ "spark.some.config2" -> " ").asJava)
+
+ try {
+ interceptCauseContains[KyuubiRestException, HttpResponseException] {
+ batchRestApi.createBatch(requestObj, appScriptFile, List().asJava)
+ }("required extra resource files
[non-existed-jar.jar,non-existed-zip.zip]" +
+ " are not uploaded in the multipart form data")
+ } finally {
+ basicKyuubiRestClient.close()
+ }
+ }
+
test("basic batch rest client with invalid user") {
val totalConnections =
MetricsSystem.counterValue(MetricsConstants.REST_CONN_TOTAL).getOrElse(0L)
diff --git
a/kyuubi-util-scala/src/test/scala/org/apache/kyuubi/util/AssertionUtils.scala
b/kyuubi-util-scala/src/test/scala/org/apache/kyuubi/util/AssertionUtils.scala
index fc7d0db7ab..d5a0f513c4 100644
---
a/kyuubi-util-scala/src/test/scala/org/apache/kyuubi/util/AssertionUtils.scala
+++
b/kyuubi-util-scala/src/test/scala/org/apache/kyuubi/util/AssertionUtils.scala
@@ -193,4 +193,17 @@ object AssertionUtils {
val exception = intercept[T](f)(classTag, pos)
assert(exception.getMessage.endsWith(end))
}
+
+ /**
+ * Asserts that the given function throws an exception of the given type T
+ * with a cause of type Q and with the cause message of the exception equals
to expected string
+ */
+ def interceptCauseContains[T <: Exception, Q <: Throwable](f: =>
Any)(contained: String)(
+ implicit
+ classTag: ClassTag[T],
+ pos: Position): Unit = {
+ assert(contained != null)
+ val exception = intercept[T](f)(classTag, pos)
+ assert(exception.getCause.asInstanceOf[Q].getMessage.contains(contained))
+ }
}
