Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[KYUUBI-SHADED #33] Step 1/2: Port ZOOKEEPER-1718 to support JLine2 in ZK client 3.4 #33

Closed
wants to merge 2 commits into from

Conversation

pan3793
Copy link
Member

@pan3793 pan3793 commented Jan 15, 2024

Why are the changes needed?

This PR aims to port ZOOKEEPER-1718 to make the Zookeeper 3.4 to support JLine2.

To gain a clear change history, I propose to split the change into 2 PRs,

  1. copy org.apache.zookeeper.ZooKeeperMain and org.apache.zookeeper.JLineZNodeCompletor from Apache Zookeeper v3.4.14 as-is
  2. apply changes in ZOOKEEPER-1718

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before make a pull request

This reverts commit 012b8c037f042de3616b722f73d1d7a071f448ac.
@pan3793 pan3793 changed the title Step 1/2: ZOOKEEPER-1718 - Overwrite ZooKeeperMain to support JLine2 in ZK client 3.4 [KYUUBI-SHADED #33] Step 1/2: ZOOKEEPER-1718 - Overwrite ZooKeeperMain to support JLine2 in ZK client 3.4 Jan 15, 2024
@pan3793 pan3793 changed the title [KYUUBI-SHADED #33] Step 1/2: ZOOKEEPER-1718 - Overwrite ZooKeeperMain to support JLine2 in ZK client 3.4 [KYUUBI-SHADED #33] Step 1/2: Port ZOOKEEPER-1718 to support JLine2 in ZK client 3.4 Jan 15, 2024
pan3793 added a commit to pan3793/kyuubi-shaded that referenced this pull request Jan 15, 2024
@pan3793 pan3793 requested a review from cxzl25 January 17, 2024 04:27
@pan3793 pan3793 added this to the 0.3.0 milestone Jan 17, 2024
@pan3793 pan3793 self-assigned this Jan 17, 2024
Copy link

@cxzl25 cxzl25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compared with zookeeper 3.4. LGTM

@pan3793 pan3793 closed this in 967c60b Jan 19, 2024
@pan3793
Copy link
Member Author

pan3793 commented Jan 19, 2024

Thanks, merged to master

pan3793 added a commit to pan3793/kyuubi-shaded that referenced this pull request Jan 19, 2024
pan3793 added a commit that referenced this pull request Feb 20, 2024
… to support JLine2 in ZK client 3.4

### _Why are the changes needed?_

This PR aims to port ZOOKEEPER-1718 to make the Zookeeper 3.4 to support JLine2.

To gain a clear change history, I propose to split the change into 2 PRs,

1. copy [org.apache.zookeeper.ZooKeeperMain](https://raw.githubusercontent.com/apache/zookeeper/release-3.4.14/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeperMain.java) and [org.apache.zookeeper.JLineZNodeCompletor](https://raw.githubusercontent.com/apache/zookeeper/release-3.4.14/zookeeper-server/src/main/java/org/apache/zookeeper/JLineZNodeCompletor.java) from Apache Zookeeper v3.4.14 as-is
2. apply changes in ZOOKEEPER-1718

### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [x] Add screenshots for manual tests if appropriate

1. perform `build/mvn clean package` to generate `kyuubi-relocated-zookeeper-parent/kyuubi-relocated-zookeeper-34/target/kyuubi-relocated-zookeeper-34-0.3.0-SNAPSHOT.jar`
2. download and install [apache-kyuubi-1.8.0-bin.tgz](https://dlcdn.apache.org/kyuubi/kyuubi-1.8.0/apache-kyuubi-1.8.0-bin.tgz)
3. replace `KYUUBI_HOME/jars/kyuubi-shaded-zookeeper-34-0.1.0.jar` with `kyuubi-relocated-zookeeper-34-0.3.0-SNAPSHOT.jar`
4. replace `KYUUBI_HOME/jars/jline-0.9.94.jar` with `jline-2.14.6.jar`
5. perform `bin/kyuubi-zk-cli -server xxxxx:2181` to connect a Zookeeper cluster
6. check the logs to make sure JLine is enabled and run some command to check basic functionalities
```
...
2024-01-15 19:52:04.593 INFO org.apache.kyuubi.shaded.zookeeper.ZooKeeper: Client environment:java.class.path=...:jline-2.14.6.jar:...:kyuubi-relocated-zookeeper-34-0.3.0-SNAPSHOT.jar...
...
JLine support is enabled
...
```

- [ ] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #34 from pan3793/jline2-2.

e769ee8 [Cheng Pan] [KYUUBI-SHADED #33] Step 2/2: Port ZOOKEEPER-1718 to support JLine2 in ZK client 3.4

Authored-by: Cheng Pan <chengpan@apache.org>
Signed-off-by: Cheng Pan <chengpan@apache.org>
@pan3793 pan3793 deleted the jline2-1 branch February 29, 2024 03:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants