[incubator-kyuubi] branch master updated: [KYUUBI #3848] [Improvement] Sort config map returned in`KyuubiConf.getAll`

[ch...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

chengpan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-kyuubi.git


The following commit(s) were added to refs/heads/master by this push:
     new b5bfd97e0 [KYUUBI #3848] [Improvement] Sort config map returned in`KyuubiConf.getAll`
b5bfd97e0 is described below

commit b5bfd97e01966adce88f9fc4a4eedd005feae38d
Author: liangbowen <li...@gf.com.cn>
AuthorDate: Fri Nov 25 18:44:52 2022 +0800

    [KYUUBI #3848] [Improvement] Sort config map returned in`KyuubiConf.getAll`
    
    ### _Why are the changes needed?_
    
    to close #3848 .
    
    Sort the config map returned in KyuubiConf.getAll to improve readability of engine launching configs in output.
    
    ### _How was this patch tested?_
    - [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible
    
    - [ ] Add screenshots for manual tests if appropriate
    
    - [x] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
    
    Closes #3849 from bowenliang123/sort-configs.
    
    Closes #3848
    
    75e60c89 [liangbowen] sort the returned map of getAll in KyuubiConf  (+4 squashed commits) Squashed commits: [f102b423] update method comment [465d7b25] use TreeMap for sorting [1a66e05b] add getAllSorted [2b3dbf91] sort the returned map of getAll in KyuubiConf
    
    Authored-by: liangbowen <li...@gf.com.cn>
    Signed-off-by: Cheng Pan <ch...@apache.org>
---
 .../scala/org/apache/kyuubi/config/KyuubiConf.scala     |  8 ++++++--
 .../org/apache/kyuubi/config/KyuubiConfSuite.scala      | 17 +++++++++++++++++
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
index ddf831834..b61a32747 100644
--- a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
+++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
@@ -24,6 +24,7 @@ import java.util.concurrent.ConcurrentHashMap
 import java.util.regex.Pattern
 
 import scala.collection.JavaConverters._
+import scala.collection.immutable.TreeMap
 import scala.util.matching.Regex
 
 import org.apache.kyuubi.{Logging, Utils}
@@ -112,9 +113,12 @@ case class KyuubiConf(loadSysDefault: Boolean = true) extends Logging {
     unset(entry.key)
   }
 
-  /** Get all parameters as map */
+  /**
+   * Get all parameters as map
+   * sorted by key in ascending order
+   */
   def getAll: Map[String, String] = {
-    settings.entrySet().asScala.map(x => (x.getKey, x.getValue)).toMap[String, String]
+    TreeMap(settings.asScala.toSeq: _*)
   }
 
   /** Get all envs as map */
diff --git a/kyuubi-common/src/test/scala/org/apache/kyuubi/config/KyuubiConfSuite.scala b/kyuubi-common/src/test/scala/org/apache/kyuubi/config/KyuubiConfSuite.scala
index 215c5327e..f05e15d8a 100644
--- a/kyuubi-common/src/test/scala/org/apache/kyuubi/config/KyuubiConfSuite.scala
+++ b/kyuubi-common/src/test/scala/org/apache/kyuubi/config/KyuubiConfSuite.scala
@@ -183,4 +183,21 @@ class KyuubiConfSuite extends KyuubiFunSuite {
     assert(kyuubiConf.getBatchConf("spark") == Map("spark.yarn.tags" -> "kyuubi"))
     assert(kyuubiConf.getBatchConf("flink") == Map("yarn.tags" -> "kyuubi"))
   }
+
+  test("KYUUBI #3848 - Sort config map returned in KyuubiConf.getAll") {
+    val kyuubiConf = KyuubiConf(false)
+    kyuubiConf.set("kyuubi.xyz", "123")
+    kyuubiConf.set("kyuubi.efg", "")
+    kyuubiConf.set("kyuubi.abc", "789")
+
+    var kSeq = Seq[String]()
+    kyuubiConf.getAll.foreach { case (k, v) =>
+      kSeq = kSeq :+ k
+    }
+
+    assertResult(kSeq.size)(3)
+    assertResult(kSeq.head)("kyuubi.abc")
+    assertResult(kSeq(1))("kyuubi.efg")
+    assertResult(kSeq(2))("kyuubi.xyz")
+  }
 }