Skip to content

Commit

Permalink
[KYUUBI #6516] Fix KyuubiSparkUtil.buildURI
Browse files Browse the repository at this point in the history
# 🔍 Description
## Issue References 🔗

This pull request fixes #6516

## Describe Your Solution 🔧

1. Using `buildStaticChecked` instead of `build` for static method `fromUri`
2. Using `Array.empty[Object]` instead of empty argument when invoking the build method

## Types of changes 🔖

- [X] 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 ⚰️
Throw error like when running the unit test
```
java.lang.RuntimeException
	at org.apache.kyuubi.util.reflect.DynMethods$UnboundMethod.invoke(DynMethods.java:80)
	at org.apache.kyuubi.engine.spark.KyuubiSparkUtil$.buildURI(KyuubiSparkUtil.scala:143)
	at org.apache.kyuubi.engine.spark.KyuubiSparkUtilSuite.$anonfun$new$1(KyuubiSparkUtilSuite.scala:33)
	at org.scalatest.OutcomeOf.outcomeOf(OutcomeOf.scala:85)
	at org.scalatest.OutcomeOf.outcomeOf$(OutcomeOf.scala:83)
	at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104)
	at org.scalatest.Transformer.apply(Transformer.scala:22)
	at org.scalatest.Transformer.apply(Transformer.scala:20)
	at org.scalatest.funsuite.AnyFunSuiteLike$$anon$1.apply(AnyFunSuiteLike.scala:226)
	at org.scalatest.TestSuite.withFixture(TestSuite.scala:196)
	at org.scalatest.TestSuite.withFixture$(TestSuite.scala:195)
	at org.scalatest.funsuite.AnyFunSuite.withFixture(AnyFunSuite.scala:1564)
	at org.scalatest.funsuite.AnyFunSuiteLike.invokeWithFixture$1(AnyFunSuiteLike.scala:224)
	at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$runTest$1(AnyFunSuiteLike.scala:236)
	at org.scalatest.SuperEngine.runTestImpl(Engine.scala:306)
	at org.scalatest.funsuite.AnyFunSuiteLike.runTest(AnyFunSuiteLike.scala:236)
	at org.scalatest.funsuite.AnyFunSuiteLike.runTest$(AnyFunSuiteLike.scala:218)
	at org.scalatest.funsuite.AnyFunSuite.runTest(AnyFunSuite.scala:1564)
	at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$runTests$1(AnyFunSuiteLike.scala:269)
	at org.scalatest.SuperEngine.$anonfun$runTestsInBranch$1(Engine.scala:413)
	at scala.collection.immutable.List.foreach(List.scala:431)
	at org.scalatest.SuperEngine.traverseSubNodes$1(Engine.scala:401)
	at org.scalatest.SuperEngine.runTestsInBranch(Engine.scala:396)
	at org.scalatest.SuperEngine.runTestsImpl(Engine.scala:475)
	at org.scalatest.funsuite.AnyFunSuiteLike.runTests(AnyFunSuiteLike.scala:269)
	at org.scalatest.funsuite.AnyFunSuiteLike.runTests$(AnyFunSuiteLike.scala:268)
	at org.scalatest.funsuite.AnyFunSuite.runTests(AnyFunSuite.scala:1564)
	at org.scalatest.Suite.run(Suite.scala:1114)
	at org.scalatest.Suite.run$(Suite.scala:1096)
	at org.scalatest.funsuite.AnyFunSuite.org$scalatest$funsuite$AnyFunSuiteLike$$super$run(AnyFunSuite.scala:1564)
	at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$run$1(AnyFunSuiteLike.scala:273)
	at org.scalatest.SuperEngine.runImpl(Engine.scala:535)
	at org.scalatest.funsuite.AnyFunSuiteLike.run(AnyFunSuiteLike.scala:273)
	at org.scalatest.funsuite.AnyFunSuiteLike.run$(AnyFunSuiteLike.scala:272)
	at org.scalatest.funsuite.AnyFunSuite.run(AnyFunSuite.scala:1564)
	at org.scalatest.tools.SuiteRunner.run(SuiteRunner.scala:47)
	at org.scalatest.tools.Runner$.$anonfun$doRunRunRunDaDoRunRun$13(Runner.scala:1321)
	at org.scalatest.tools.Runner$.$anonfun$doRunRunRunDaDoRunRun$13$adapted(Runner.scala:1315)
	at scala.collection.immutable.List.foreach(List.scala:431)
	at org.scalatest.tools.Runner$.doRunRunRunDaDoRunRun(Runner.scala:1315)
	at org.scalatest.tools.Runner$.$anonfun$runOptionallyWithPassFailReporter$24(Runner.scala:992)
	at org.scalatest.tools.Runner$.$anonfun$runOptionallyWithPassFailReporter$24$adapted(Runner.scala:970)
	at org.scalatest.tools.Runner$.withClassLoaderAndDispatchReporter(Runner.scala:1481)
	at org.scalatest.tools.Runner$.runOptionallyWithPassFailReporter(Runner.scala:970)
	at org.scalatest.tools.Runner$.run(Runner.scala:798)
	at org.scalatest.tools.Runner.run(Runner.scala)
	at org.jetbrains.plugins.scala.testingSupport.scalaTest.ScalaTestRunner.runScalaTest2or3(ScalaTestRunner.java:43)
	at org.jetbrains.plugins.scala.testingSupport.scalaTest.ScalaTestRunner.main(ScalaTestRunner.java:26)
```

#### Behavior With This Pull Request 🎉
Test passed

#### 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 #6517 from jeanlyn/issue-6516.

Closes #6516

c6bf8b6 [jeanlyn] adjust
e50e585 [jeanlyn] adjust
8afdc0c [jeanlyn] fix
a61ae70 [jeanlyn] fix buildURI error and adding unit tests

Authored-by: jeanlyn <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
  • Loading branch information
jeanlyn authored and pan3793 committed Jul 1, 2024
1 parent ab273c8 commit 66b971f
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ object KyuubiSparkUtil extends Logging {
if (SPARK_ENGINE_RUNTIME_VERSION >= "4.0") {
var uriBuilder = DynMethods.builder("fromUri")
.impl("jakarta.ws.rs.core.UriBuilder", classOf[URI])
.build()
.buildStatic()
.invoke[AnyRef](uri)

uriBuilder = DynMethods.builder("fragment")
Expand All @@ -133,13 +133,13 @@ object KyuubiSparkUtil extends Logging {
.invoke[AnyRef](fragment)

DynMethods.builder("build")
.impl("jakarta.ws.rs.core.UriBuilder")
.impl("jakarta.ws.rs.core.UriBuilder", classOf[Array[AnyRef]])
.build(uriBuilder)
.invoke[URI]()
.invoke[URI](Array.empty[AnyRef])
} else {
var uriBuilder = DynMethods.builder("fromUri")
.impl("javax.ws.rs.core.UriBuilder", classOf[URI])
.build()
.buildStatic()
.invoke[AnyRef](uri)

uriBuilder = DynMethods.builder("fragment")
Expand All @@ -148,9 +148,9 @@ object KyuubiSparkUtil extends Logging {
.invoke[AnyRef](fragment)

DynMethods.builder("build")
.impl("javax.ws.rs.core.UriBuilder")
.impl("javax.ws.rs.core.UriBuilder", classOf[Array[AnyRef]])
.build(uriBuilder)
.invoke[URI]()
.invoke[URI](Array.empty[AnyRef])
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.kyuubi.engine.spark

import java.net.URI

import org.apache.kyuubi.KyuubiFunSuite
import org.apache.kyuubi.engine.spark.KyuubiSparkUtil.buildURI
import org.apache.kyuubi.engine.spark.operation.ExecutePython.DEFAULT_SPARK_PYTHON_HOME_ARCHIVE_FRAGMENT

class KyuubiSparkUtilSuite extends KyuubiFunSuite {
test("get build uri") {
val uri = new URI("hdfs://a/b/c.zip")
val buildedUri = buildURI(uri, DEFAULT_SPARK_PYTHON_HOME_ARCHIVE_FRAGMENT)
assert(buildedUri.getScheme == "hdfs")
assert(buildedUri.getFragment == DEFAULT_SPARK_PYTHON_HOME_ARCHIVE_FRAGMENT)
assert(buildedUri.getSchemeSpecificPart == "//a/b/c.zip")
}
}

0 comments on commit 66b971f

Please sign in to comment.