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.

Sign up for GitHub

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

Jump to bottom

ZOOKEEPER-3640: Implement "batch mode" in cli_mt #1173

Closed
wants to merge 1 commit into from
Closed

ZOOKEEPER-3640: Implement "batch mode" in cli_mt #1173

wants to merge 1 commit into from

Conversation

ztzg
Copy link
Contributor

@ztzg ztzg commented Dec 11, 2019 •
edited
Loading

Batch mode never was implemented in cli_mt. This patch seems to work, but:

  1. There may be a cleaner way of waiting for the completion;
  2. nanosleep is POSIX; the Windows path should probably use Sleep (DONE).

@symat: Comments welcome.

@ztzg ztzg force-pushed the ZOOKEEPER-3640-implement-batch-mode-in-cli-mt branch from c830d74 to a40a114 Compare December 11, 2019 08:44
@ztzg ztzg changed the title ZOOKEEPER-3640: Implement "batch mode" in cli_mt (WIP) ZOOKEEPER-3640: Implement "batch mode" in cli_mt Dec 11, 2019
@asf-ci
Copy link

asf-ci commented Dec 11, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build-maven/1705/

@asf-ci
Copy link

asf-ci commented Dec 11, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build-maven/1706/

Build result: FAILURE

[...truncated 926.32 KB...][JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/pom.xml to org.apache.zookeeper/parent/3.6.0-SNAPSHOT/parent-3.6.0-SNAPSHOT.pom[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-recipes/zookeeper-recipes-lock/pom.xml to org.apache.zookeeper/zookeeper-recipes-lock/3.6.0-SNAPSHOT/zookeeper-recipes-lock-3.6.0-SNAPSHOT.pom[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-contrib/pom.xml to org.apache.zookeeper/zookeeper-contrib/3.6.0-SNAPSHOT/zookeeper-contrib-3.6.0-SNAPSHOT.pom[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-client/pom.xml to org.apache.zookeeper/zookeeper-client/3.6.0-SNAPSHOT/zookeeper-client-3.6.0-SNAPSHOT.pom[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-jute/pom.xml to org.apache.zookeeper/zookeeper-jute/3.6.0-SNAPSHOT/zookeeper-jute-3.6.0-SNAPSHOT.pom[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-jute/target/zookeeper-jute-3.6.0-SNAPSHOT.jar to org.apache.zookeeper/zookeeper-jute/3.6.0-SNAPSHOT/zookeeper-jute-3.6.0-SNAPSHOT.jar[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-jute/target/zookeeper-jute-3.6.0-SNAPSHOT-tests.jar to org.apache.zookeeper/zookeeper-jute/3.6.0-SNAPSHOT/zookeeper-jute-3.6.0-SNAPSHOT-tests.jar[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-jute/target/zookeeper-jute-3.6.0-SNAPSHOT-sources.jar to org.apache.zookeeper/zookeeper-jute/3.6.0-SNAPSHOT/zookeeper-jute-3.6.0-SNAPSHOT-sources.jar[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-jute/target/zookeeper-jute-3.6.0-SNAPSHOT-javadoc.jar to org.apache.zookeeper/zookeeper-jute/3.6.0-SNAPSHOT/zookeeper-jute-3.6.0-SNAPSHOT-javadoc.jar[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-recipes/zookeeper-recipes-queue/pom.xml to org.apache.zookeeper/zookeeper-recipes-queue/3.6.0-SNAPSHOT/zookeeper-recipes-queue-3.6.0-SNAPSHOT.pom[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-docs/pom.xml to org.apache.zookeeper/zookeeper-docs/3.6.0-SNAPSHOT/zookeeper-docs-3.6.0-SNAPSHOT.pom[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-docs/target/zookeeper-docs-3.6.0-SNAPSHOT.jar to org.apache.zookeeper/zookeeper-docs/3.6.0-SNAPSHOT/zookeeper-docs-3.6.0-SNAPSHOT.jar[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-docs/target/zookeeper-docs-3.6.0-SNAPSHOT-tests.jar to org.apache.zookeeper/zookeeper-docs/3.6.0-SNAPSHOT/zookeeper-docs-3.6.0-SNAPSHOT-tests.jar[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-docs/target/zookeeper-docs-3.6.0-SNAPSHOT-sources.jar to org.apache.zookeeper/zookeeper-docs/3.6.0-SNAPSHOT/zookeeper-docs-3.6.0-SNAPSHOT-sources.jar[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-recipes/pom.xml to org.apache.zookeeper/zookeeper-recipes/3.6.0-SNAPSHOT/zookeeper-recipes-3.6.0-SNAPSHOT.pom[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-client/zookeeper-client-c/pom.xml to org.apache.zookeeper/zookeeper-client-c/3.6.0-SNAPSHOT/zookeeper-client-c-3.6.0-SNAPSHOT.pom[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-contrib/zookeeper-contrib-rest/pom.xml to org.apache.zookeeper/zookeeper-contrib-rest/3.6.0-SNAPSHOT/zookeeper-contrib-rest-3.6.0-SNAPSHOT.pom[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-assembly/pom.xml to org.apache.zookeeper/zookeeper-assembly/3.6.0-SNAPSHOT/zookeeper-assembly-3.6.0-SNAPSHOT.pom[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-contrib/zookeeper-contrib-zooinspector/pom.xml to org.apache.zookeeper/zookeeper-contrib-zooinspector/3.6.0-SNAPSHOT/zookeeper-contrib-zooinspector-3.6.0-SNAPSHOT.pomchannel stopped[SpotBugs] Skipping execution of recorder since overall result is 'FAILURE'Setting status of a40a114 to FAILURE with url https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build-maven/1706/ and message: 'FAILURE 'Using context: JenkinsMaven

@ztzg
Copy link
Contributor Author

ztzg commented Dec 11, 2019

retest maven build

@symat
Copy link
Contributor

symat commented Dec 11, 2019

I quickly tested the patch, and it seems working now both for cli_mt and cli_st. I haven't tested it on windows yet (that takes more time for me), but happy to do if you think the patch is finished.

I was thinking on the code and haven't find a nicer way to wait for the async command to be finished. I think your solution is in general inline with the way how things are implemented now in the command line client.

One improvement proposal could be to allow to execute multiple commands in the same client session with the --cmd option. Or even better, to make it possible to pipe a text file into the command-line client so that all the commands in the text file gets executed. Something like cat commands.txt | cli_mt --host localhost:2181 --non-interactive. But actually I am not really sure if this worths the effort, as the C CLI is not widely used.

@asf-ci
Copy link

asf-ci commented Dec 11, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build-maven/1707/

@symat
Copy link
Contributor

symat commented Dec 11, 2019

One improvement proposal could be

I didn't meant to implement this as part of this PR of course

@ztzg
Copy link
Contributor Author

ztzg commented Dec 11, 2019

One improvement proposal could be

I didn't meant to implement this as part of this PR of course

Right. Good suggestions, but implementing them seems like a distinct task. I'll keep them in mind for when I circle back to cli.c, but feel free to open a ticket.

symat
symat approved these changes Dec 11, 2019
View reviewed changes
Copy link
Contributor

@symat symat left a comment

Choose a reason for hiding this comment

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

LGTM

@symat
Copy link
Contributor

symat commented Dec 11, 2019

FYI: I created https://issues.apache.org/jira/browse/ZOOKEEPER-3646
(but I don't think I can work on this in the near future)

@symat
Copy link
Contributor

symat commented Dec 12, 2019

FYI: I also tested the patch on windows and it builds / works fine

@ztzg
Copy link
Contributor Author

ztzg commented Dec 12, 2019

Hi @symat,

ZOOKEEPER-3646: Thanks! I am also keeping it in my TODO list—but with a low priority.

Windows: I had actually tested it on Windows, too :) But one additional data point does not hurt.

Cheers, -D

anmolnar
anmolnar approved these changes Jan 6, 2020
View reviewed changes
Copy link
Contributor

@anmolnar anmolnar left a comment

Choose a reason for hiding this comment

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

+1

@asfgit asfgit closed this in d7bc7b1 Jan 6, 2020
asfgit pushed a commit that referenced this pull request Jan 6, 2020
@ztzg @anmolnar
ZOOKEEPER-3640: Implement "batch mode" in cli_mt
d01dac1 
Batch mode never was implemented in `cli_mt`.  This patch seems to work, but:

1. There may be a cleaner way of waiting for the completion;
2. ~~`nanosleep` is POSIX; the Windows path should probably use `Sleep`~~ (DONE).

symat: Comments welcome.

Author: Damien Diederen <dd@crosstwine.com>

Reviewers: andor@apache.org

Closes #1173 from ztzg/ZOOKEEPER-3640-implement-batch-mode-in-cli-mt

(cherry picked from commit d7bc7b1)
Signed-off-by: Andor Molnar <andor@apache.org>
@anmolnar
Copy link
Contributor

anmolnar commented Jan 6, 2020

Committed to master and 3.6 branches.
Thanks @ztzg ! Are u able to assign Jira tickets yourself?

@ztzg
Copy link
Contributor Author

ztzg commented Jan 6, 2020

Hi @anmolnar,

Committed to master and 3.6 branches.

Thanks!

Are u able to assign Jira tickets yourself?

I don't think so. The assignee "widget" does not respond to mouse clicks, and I did not spot anything relevant in the "More >" menu or "Edit" dialog.

@anmolnar
Copy link
Contributor

anmolnar commented Jan 6, 2020

@ztzg No problem. I added you to the contributors list and assigned the Jira to you.
Thank you for your contributions.

@ztzg
Copy link
Contributor Author

ztzg commented Jan 9, 2020

@anmolnar: Thanks!

junyoungKimGit pushed a commit to junyoungKimGit/zookeeper that referenced this pull request Feb 7, 2020
@ztzg @junyoungKimGit
ZOOKEEPER-3640: Implement "batch mode" in cli_mt
8556d23 
Batch mode never was implemented in `cli_mt`.  This patch seems to work, but:

1. There may be a cleaner way of waiting for the completion;
2. ~~`nanosleep` is POSIX; the Windows path should probably use `Sleep`~~ (DONE).

symat: Comments welcome.

Author: Damien Diederen <dd@crosstwine.com>

Reviewers: andor@apache.org

Closes apache#1173 from ztzg/ZOOKEEPER-3640-implement-batch-mode-in-cli-mt
stickyhipp pushed a commit to stickyhipp/zookeeper that referenced this pull request Aug 19, 2020
@ztzg @stickyhipp
ZOOKEEPER-3640: Implement "batch mode" in cli_mt
3f13762 
Batch mode never was implemented in `cli_mt`.  This patch seems to work, but:

1. There may be a cleaner way of waiting for the completion;
2. ~~`nanosleep` is POSIX; the Windows path should probably use `Sleep`~~ (DONE).

symat: Comments welcome.

Author: Damien Diederen <dd@crosstwine.com>

Reviewers: andor@apache.org

Closes apache#1173 from ztzg/ZOOKEEPER-3640-implement-batch-mode-in-cli-mt
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Aug 31, 2022
@ztzg @RokLenarcic
ZOOKEEPER-3640: Implement "batch mode" in cli_mt
7d6be55 
Batch mode never was implemented in `cli_mt`.  This patch seems to work, but:

1. There may be a cleaner way of waiting for the completion;
2. ~~`nanosleep` is POSIX; the Windows path should probably use `Sleep`~~ (DONE).

symat: Comments welcome.

Author: Damien Diederen <dd@crosstwine.com>

Reviewers: andor@apache.org

Closes apache#1173 from ztzg/ZOOKEEPER-3640-implement-batch-mode-in-cli-mt
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Sep 3, 2022
@ztzg @RokLenarcic
ZOOKEEPER-3640: Implement "batch mode" in cli_mt
e6ad902 
Batch mode never was implemented in `cli_mt`.  This patch seems to work, but:

1. There may be a cleaner way of waiting for the completion;
2. ~~`nanosleep` is POSIX; the Windows path should probably use `Sleep`~~ (DONE).

symat: Comments welcome.

Author: Damien Diederen <dd@crosstwine.com>

Reviewers: andor@apache.org

Closes apache#1173 from ztzg/ZOOKEEPER-3640-implement-batch-mode-in-cli-mt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@symat symat symat approved these changes

@anmolnar anmolnar anmolnar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ztzg @asf-ci @symat @anmolnar