[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-07 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 @anmolnar thanks for merging this :) I've rebased #679, #680, and #681 on top of master. Let's get those in soon :) ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-06 Thread anmolnar
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/678 Merged to 3.5 and master branches. Thanks @ivmaykov ! ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-06 Thread anmolnar
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/678 Now it's green. I'll merge it. Previously the findbugs subprocess has been killed for some reason. ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-06 Thread asfgit
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/678 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2604/ ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-06 Thread enixon
Github user enixon commented on the issue: https://github.com/apache/zookeeper/pull/678 Yeah, there aren't findbugs reports in the artifacts as I would expect https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2599/artifact/patchprocess/ , it's unclear what went wrong

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-06 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 @anmolnar the latest attempt does not have test failures, but claims a findbugs failure. >[exec] -1 overall. GitHub Pull Request Build >[exec] >[exec] >

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-06 Thread anmolnar
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/678 It's not findbugs, core and contrib tests are failing, but I cannot see why: ``` [exec] -1 core tests. The patch failed core unit tests. [exec] -1 contrib

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-06 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 Jenkins claims there is a findbugs failure, but there is no actual output of findbugs failures, and findbugs passes on my machine. ¯\_(ツ)_/¯ ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-06 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 I don't understand why this PR started failing Jenkins builds all of a sudden. The next 2 PRs stacked on this one (#678 and #680) also fail, but the last one (#681) passes. ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-06 Thread asfgit
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/678 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2599/ ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-06 Thread anmolnar
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/678 @ivmaykov It's in: `zookeeper-server/src/test/resources/test-github-pr.sh` ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-06 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/678 Maybe you should use docker. As soon as possible I will copy the command line we are using in Jenkins job. Maybe it is already available on the job log ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-06 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 @anmolnar is there a way to reproduce the exact same steps that jenkins runs on my macbook? ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-06 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 @anmolnar it works for me as far as I can tell, I think the contbuild is flaky? The contbuild on #681, which includes the same commit, passes. ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-05 Thread anmolnar
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/678 What's wrong with this build? @ivmaykov Does it work for you locally? ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-05 Thread asfgit
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/678 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2589/ ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-05 Thread anmolnar
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/678 retest this please ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-05 Thread asfgit
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/678 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2584/ ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-02 Thread asfgit
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/678 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2563/ ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-01 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 @anmolnar on my machine it worked, but on Jenkins it could not resolve the import - not sure why. It's not a big deal I think, we don't need to use it. ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-01 Thread anmolnar
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/678 @ivmaykov What was the problem with using `FileNameUtils`? ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-01 Thread asfgit
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/678 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2559/ ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-01 Thread asfgit
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/678 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2555/ ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-01 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 @eolivelli got it, thanks! I will use that terminology from now on :) ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-01 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/678 'Non binding' means that I am not a 'committer' so I can't merge the patch by myself. Btw the other guys which are reviewing this patch are committers so I think this patch will be merged

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-01 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 @eolivelli what does a "(non binding)" +1 mean? Can we merge this to upstream/master at this point? ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-01 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 Revert `FileNameUtils` change as it seems to be breaking contbuild ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-31 Thread asfgit
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/678 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2548/ ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-31 Thread asfgit
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/678 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2544/ ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-31 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/678 For Apache httpcomponents there is nothing to write, the NOTICE already covers it, as it is an Apache Project ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-30 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 Refactored common code shared by a bunch of tests that use `X509TestContext` into a new base class, `BaseX509ParameterizedTestCase`. ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-30 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 @eolivelli capitalized "Airlift" in NOTICE.txt @anmolnar use `FileNameUtils.getExtension()` for file type detection everyone: fixed some copy-paste bugs in PEMFileLoaderTest and

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-30 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 @anmolnar it's certainly easier to keep track of the copied code if we don't modify it much. Moving the logic into PEMFileLoader and making it non-static would make it harder to trace the

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-29 Thread asfgit
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/678 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2536/ ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-29 Thread asfgit
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/678 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2532/ ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-29 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 @anmolnar added KeyStoreLoader classes @eolivelli updated NOTICE file and PemReader.java ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-27 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 I should mention that this code has been internally reviewed at Facebook, has been landed on our internal fork, and has been running in production for weeks. ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-27 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/678 @ivmaykov The code can stay where you put it. About the NOTICE file I will try to help past the weekend ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-26 Thread asfgit
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/678 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2521/ ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-26 Thread asfgit
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/678 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2525/ ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-26 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 @eolivelli should the copied code be put into the zookeeper-contrib subproject? Or can it still live in zookeeper-server? I could also use help with wording the message in the NOTICE file.

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-26 Thread asfgit
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/678 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2514/ ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-25 Thread hanm
Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/678 >> is there a way to trigger a Jenkins build re-run? @ivmaykov Check out https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute, search 'Jenkins Pre-commit Check'. The

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-25 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/678 retest this please ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-25 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 The test failures don't look to be related to my changes. @anmolnar @hanm is there a way to trigger a Jenkins build re-run? ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-24 Thread asfgit
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/678 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2500/ ---