biangjuang commented on code in PR #5412:
URL: https://github.com/apache/kyuubi/pull/5412#discussion_r1359013228
##########
kyuubi-zookeeper/src/main/scala/org/apache/kyuubi/zookeeper/EmbeddedZookeeper.scala:
##########
@@ -31,12 +31,18 @@ class EmbeddedZookeeper extends
AbstractService("EmbeddedZookeeper") {
private var zks: ZooKeeperServer = _
private var serverFactory: NIOServerCnxnFactory = _
private var dataDirectory: File = _
+ private var dataLogDirectory: File = _
// TODO: Is it right in prod?
private val deleteDataDirectoryOnClose = true
private var host: String = _
override def initialize(conf: KyuubiConf): Unit = synchronized {
- dataDirectory = new File(conf.get(ZK_DATA_DIR))
+ val kyuubiHome = sys.env.getOrElse(KyuubiConf.KYUUBI_HOME, ".")
+ val kyuubiHomeWithDollar = "$" + KyuubiConf.KYUUBI_HOME
Review Comment:
> the idea looks cool, but what if the user configures it to
`${KYUUBI_HOME}/embedded_zookeeper` or `$ANOTHER_ENV_VAR/embedded_zookeeper`?
> the idea looks cool, but what if the user configures it to
`${KYUUBI_HOME}/embedded_zookeeper` or `$ANOTHER_ENV_VAR/embedded_zookeeper`?
Yes ,the current design is not elegant and universal.
But in my opinion，
If we allow configuration with system env and java properties, it should be
implemented in `KyuubiConf` class, `KyuubiConf` can substitute the variables
when load all the values.
Generating zk directories everywhere annoys me. This PR is just to ensure
that it works well out of the box.
Or we should set the default dir under `java.io.tmpdir`?
--
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]
