Change in vdsm[master]: vdsm: verifyingTransport testing
oVirt Jenkins CI Server has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 13: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/10015/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10800/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc20_created/80/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc19_created/112/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-el6_created/91/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10957/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
Nir Soffer has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 13: Stop this dependency craziness! There is no reason this patch should depend on anything - it should be rebase on last master and never change until we finish the review. The way it is works now is all your reviewers are getting mail whenever you change the dependent patch and post all the chain back to gerrit. -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
oVirt Jenkins CI Server has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 14: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/10025/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10810/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc20_created/82/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc19_created/114/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-el6_created/93/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10967/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
Dan Kenigsberg has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 14: Code-Review+2 raising score. Thanks a lot for making this test work again (thanks for authoring and reviewing) -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
Dan Kenigsberg has submitted this change and it was merged. Change subject: vdsm: verifyingTransport testing .. vdsm: verifyingTransport testing Changing unused command line test to unit test. SecureXMLRPCServer is not used by vdsm code base and it was moved to verifyingTransportTests to be used to make sure that Verifing* classes work. Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Signed-off-by: pkliczewski piotr.kliczew...@gmail.com Reviewed-on: http://gerrit.ovirt.org/28858 Reviewed-by: Nir Soffer nsof...@redhat.com Reviewed-by: Dan Kenigsberg dan...@redhat.com --- M .gitignore M debian/vdsm-python.install M lib/vdsm/Makefile.am D lib/vdsm/SecureXMLRPCServer.py M lib/vdsm/sslutils.py M lib/vdsm/vdscli.py M tests/Makefile.am M tests/jsonRpcHelper.py M tests/makecert.sh M tests/run_tests_local.sh.in M tests/sslTests.py A tests/sslhelper.py M vdsm.spec.in M vdsm/kaxmlrpclib.py 14 files changed, 253 insertions(+), 234 deletions(-) Approvals: Piotr Kliczewski: Verified Nir Soffer: Looks good to me, but someone else must approve Dan Kenigsberg: Looks good to me, approved -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
oVirt Jenkins CI Server has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 15: Build Successful http://jenkins.ovirt.org/job/vdsm_master_create-rpms_merged/1585/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5570/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3728/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
oVirt Jenkins CI Server has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 12: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9948/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10733/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc20_created/69/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc19_created/102/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-el6_created/81/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10890/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
oVirt Jenkins CI Server has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 9: Build Successful http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9850/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10635/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc20_created/63/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc19_created/96/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-el6_created/75/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10792/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
Nir Soffer has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 9: (1 comment) Nice - but there is duplicate test. http://gerrit.ovirt.org/#/c/28858/9/tests/sslTests.py File tests/sslTests.py: Line 122: with verifyingclient(KEY_FILE, OTHER_CRT_FILE) as client: Line 123: with self.assertRaises(ssl.SSLError): Line 124: client.add(2, 3) Line 125: Line 126: def test_key_do_not_match_with_cert(self): This is the same as test_cert_do_not_match_with_key - did you mean to use different combination here? Line 127: with verifyingclient(KEY_FILE, OTHER_CRT_FILE) as client: Line 128: with self.assertRaises(ssl.SSLError): Line 129: client.add(2, 3) Line 130: -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
oVirt Jenkins CI Server has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 10: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9879/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10664/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc20_created/64/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc19_created/97/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-el6_created/76/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10821/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
oVirt Jenkins CI Server has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 11: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9882/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10667/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc20_created/65/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc19_created/98/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-el6_created/77/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10824/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
Nir Soffer has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 11: Code-Review+1 Nice! -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
oVirt Jenkins CI Server has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 8: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9768/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10553/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc20_created/33/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc19_created/66/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-el6_created/45/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10711/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
Nir Soffer has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 8: (2 comments) http://gerrit.ovirt.org/#/c/28858/8/tests/sslTests.py File tests/sslTests.py: Line 96: @contextmanager Line 97: def verifyingclient(key_file, crt_file): Line 98: server = TestServer() Line 99: try: Line 100: server.start() This must be before the try. Line 101: client = VerifyingClient(server.port, key_file, crt_file) Line 102: try: Line 103: yield client Line 104: finally: Line 115: Line 116: def test_invalid(self): Line 117: with verifyingclient(OTHER_KEY_FILE, OTHER_CRT_FILE) as client: Line 118: with self.assertRaises(ssl.SSLError): Line 119: self.assertEquals(client.add(2, 3), 5) So other combinations (e.g. KEY_FILE, OTHER_CRT_FILE) are not interesting? I also assume that OTHER_KEY_FILE and OTHER_CRT_FILE are not valid combination - because the names do not suggest this. One can expect that OTHER_KEY with OTHER_CRT is a valid combination. Line 120: Line 121: Line 122: class SSLServerThread(threading.Thread): Line 123: A very simple server thread. -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
Nir Soffer has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 8: Code-Review-1 Little error in the try block, and question about the invalid test(s?). -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
oVirt Jenkins CI Server has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 6: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9719/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10504/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc20_created/29/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc19_created/62/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-el6_created/41/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10661/ : FAILURE -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
Nir Soffer has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 6: (4 comments) http://gerrit.ovirt.org/#/c/28858/6/tests/sslTests.py File tests/sslTests.py: Line 64: def serve_forever(self): Line 65: try: Line 66: self.server.serve_forever() Line 67: except: Line 68: # we want to hide server side exception Do we? why? Line 69: pass Line 70: Line 71: def stop(self): Line 72: self.server.shutdown() Line 93: Line 94: class VerifyingTransportTests(TestCaseBase): Line 95: Line 96: def test_valid(self): Line 97: server = TestServer() try should be here - the server starts when created. Line 98: client = VerifingClient(server.port, KEY_FILE, CRT_FILE) Line 99: try: Line 100: self.assertEquals(client.add(2, 3), 5) Line 101: finally: Line 102: server.stop() Line 103: client.close() Line 104: Line 105: def test_invalid(self): Line 106: server = TestServer() Why this test does not need try finally? What you need here is a single check method, that run the same flow with different arguments: def check(self, key_file, crt_file): server = TestServer() try: client = VerifyingClient(server.port, key_fie, crt_file) try: return client.add(2, 3) finally: client.close() finally: server.stop() And then: def test_valid(self): self.assertEquals(self.check(KEY_FILE, CRT_FILE), 5) def test_invalid_crt(self): self.assertRaises... And so on, all needed combinations. Another option is to write a context mananger that starts a server and returns a client that you can use to send messages: @contextmanager def verifingclient(key_file, crt_file): server = TestServer() try: client = VerifyingClient(server.port, key_fie, crt_file) try: yield client finally: client.close() finally: server.stop() If it works, the tests can be nicer: with verifyingclient(...) as client: self.assertEquals(client.add(2, 3), 5) I'm not sure this effort is need for this module, but we can use what we learn here to make other tests nicer and easier to maintain. Line 107: client = VerifingClient(server.port, OTHER_KEY_FILE, OTHER_CRT_FILE) Line 108: Line 109: with self.assertRaises(ssl.SSLError): Line 110: self.assertEquals(client.add(2, 3), 5) Line 109: with self.assertRaises(ssl.SSLError): Line 110: self.assertEquals(client.add(2, 3), 5) Line 111: Line 112: server.stop() Line 113: client.close() Should we test also other combinations, like same KEY_FILE, other CRT_FILE? I don't know much abut the details of the verification. Line 114: Line 115: Line 116: class SSLServerThread(threading.Thread): Line 117: A very simple server thread. -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
Nir Soffer has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 6: (1 comment) http://gerrit.ovirt.org/#/c/28858/6/tests/run_tests_local.sh.in File tests/run_tests_local.sh.in: Line 3: PYTHON_EXE=@PYTHON@ Line 4: fi Line 5: Line 6: if [ ! -f @top_srcdir@/tests/server.crt ] || [ ! -f @top_srcdir@/tests/server.csr ] || [ ! -f @top_srcdir@/tests/server.key ] || [ ! -f @top_srcdir@/tests/other.crt ] || [ ! -f @top_srcdir@/tests/other.csr ] || [ ! -f @top_srcdir@/tests/other.key ]; then Line 7: @top_srcdir@/tests/makecert.sh In another patch, maybe move this to make target instead? Line 8: fi Line 9: -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
oVirt Jenkins CI Server has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 7: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9728/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10513/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc20_created/30/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc19_created/63/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-el6_created/42/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10670/ : FAILURE -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
Dan Kenigsberg has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 7: Code-Review-1 (2 comments) http://gerrit.ovirt.org/#/c/28858/7/tests/sslTests.py File tests/sslTests.py: Line 64: def serve_forever(self): Line 65: try: Line 66: self.server.serve_forever() Line 67: except: Line 68: # we want to hide server side exception could you catch only the expected exceptions here? and if you keep the comment, please explain *why* we want to hide the errors. Line 69: pass Line 70: Line 71: def stop(self): Line 72: self.server.shutdown() Line 93: Line 94: class VerifyingTransportTests(TestCaseBase): Line 95: Line 96: def test_valid(self): Line 97: server = TestServer() I do not see Nir's comments acted upon - shouldn't they? Line 98: client = VerifingClient(server.port, KEY_FILE, CRT_FILE) Line 99: try: Line 100: self.assertEquals(client.add(2, 3), 5) Line 101: finally: -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
Nir Soffer has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 7: (2 comments) http://gerrit.ovirt.org/#/c/28858/7/tests/sslTests.py File tests/sslTests.py: Line 28: import tempfile Line 29: import threading Line 30: import xmlrpclib Line 31: Line 32: from sslHelper import KEY_FILE, CRT_FILE, OTHER_KEY_FILE, OTHER_CRT_FILE can you rename this to sslhelper ? modules names should not use mixedCase. We have such modules (and Python standard library also) but pep8 recommend lowercase module names. Line 33: from testrunner import VdsmTestCase as TestCaseBase Line 34: from vdsm.sslutils import SSLServerSocket Line 35: from vdsm.sslutils import VerifyingSafeTransport Line 36: Line 64: def serve_forever(self): Line 65: try: Line 66: self.server.serve_forever() Line 67: except: Line 68: # we want to hide server side exception could you catch only the expected exceptions here? and if you keep the comm Please replace the comment with with the text in your reply in version 6: http://gerrit.ovirt.org/#/c/28858/6/tests/sslTests.py The current comment does not add any new information. I can tell by looking at the code that you want to hide server side exception. What I cannot tell is why. Line 69: pass Line 70: Line 71: def stop(self): Line 72: self.server.shutdown() -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
oVirt Jenkins CI Server has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 5: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9625/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10410/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10567/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5491/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3649/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc19_created/30/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc20_created/10/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-el6_created/11/ : FAILURE -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
Nir Soffer has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 5: (4 comments) Nice! but you forgot to address two comments. http://gerrit.ovirt.org/#/c/28858/5/lib/vdsm/verifyingtransport.py File lib/vdsm/verifyingtransport.py: Line 27: Line 28: import httplib Line 29: import socket Line 30: import ssl Line 31: import xmlrpclib Maybe move these 3 small classes to sslutils? Line 32: Line 33: Line 34: class VerifyingHTTPSConnection(httplib.HTTPSConnection): Line 35: def __init__(self, host, port=None, key_file=None, cert_file=None, http://gerrit.ovirt.org/#/c/28858/5/tests/verifyingTransportTests.py File tests/verifyingTransportTests.py: Line 24: Line 25: from vdsm.verifyingtransport import VerifyingSafeTransport Line 26: from vdsm.sslutils import SSLServerSocket Line 27: from testrunner import VdsmTestCase as TestCaseBase Line 28: from jsonRpcHelper import KEY_FILE, CRT_FILE Don't import from jsonRpcHelpler - see my comment in http://gerrit.ovirt.org/#/c/28858/3/tests/verifyingTransportTests.py Line 29: Line 30: CACERT = None Line 31: HOST = '127.0.0.1' Line 32: Line 45: self.server.socket = SSLServerSocket(raw=self.server.socket, Line 46: keyfile=KEY_FILE, Line 47: certfile=CRT_FILE, Line 48: ca_certs=CACERT) Line 49: _, self.port = self.server.socket.getsockname() Nice Line 50: self.server.register_instance(MathService()) Line 51: self.start() Line 52: Line 53: def start(self): Line 85: try: Line 86: self.assertEquals(client.add(2, 3), 5) Line 87: finally: Line 88: server.stop() Line 89: client.close() And where are the tests for invalid certificate/key? See my comment in http://gerrit.ovirt.org/#/c/28858/3/tests/verifyingTransportTests.py -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
Nir Soffer has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 5: (1 comment) http://gerrit.ovirt.org/#/c/28858/5/tests/verifyingTransportTests.py File tests/verifyingTransportTests.py: Line 85: try: Line 86: self.assertEquals(client.add(2, 3), 5) Line 87: finally: Line 88: server.stop() Line 89: client.close() Ok will add. Thanks! -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
oVirt Jenkins CI Server has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 4: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9544/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/777/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10328/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10485/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5410/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3568/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc19_created/17/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
oVirt Jenkins CI Server has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 3: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9491/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/773/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10275/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10431/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5357/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3515/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
Nir Soffer has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 3: Code-Review-1 (10 comments) Nice patch, can be simplified a lot and need some fixes. http://gerrit.ovirt.org/#/c/28858/3/tests/verifyingTransportTests.py File tests/verifyingTransportTests.py: Line 20: import threading Line 21: import logging Line 22: import socket Line 23: import ssl Line 24: import xmlrpclib Sort imports Line 25: Line 26: import SimpleXMLRPCServer Line 27: Line 28: from vdsm.verifyingtransport import VerifyingSafeTransport Line 21: import logging Line 22: import socket Line 23: import ssl Line 24: import xmlrpclib Line 25: Remove this unneeded line Line 26: import SimpleXMLRPCServer Line 27: Line 28: from vdsm.verifyingtransport import VerifyingSafeTransport Line 29: from vdsm.sslutils import SSLServerSocket Line 26: import SimpleXMLRPCServer Line 27: Line 28: from vdsm.verifyingtransport import VerifyingSafeTransport Line 29: from vdsm.sslutils import SSLServerSocket Line 30: from vdsm.utils import IPXMLRPCRequestHandler Not needed after deletion of unneeded clases. Line 31: from testrunner import VdsmTestCase as TestCaseBase Line 32: import jsonRpcHelper Line 33: Line 34: KEYFILE = jsonRpcHelper.KEY_FILE Line 28: from vdsm.verifyingtransport import VerifyingSafeTransport Line 29: from vdsm.sslutils import SSLServerSocket Line 30: from vdsm.utils import IPXMLRPCRequestHandler Line 31: from testrunner import VdsmTestCase as TestCaseBase Line 32: import jsonRpcHelper Don't import jsonRpcHelper, it imports too much, which casue the test to fail becuase of relative imports when running using ./run_tests_local.sh. Since jsonRpcHelper's KEY_FILe and CRT_FILE are used by multiple modules, they should be in another module imported by both this module and jsonRpcHelper. This can be testrunner.py or maybe new sslhelper module. This means also that the files cannot be named jsonrpc-tests.server.crt, they should be simply named server.crt. Line 33: Line 34: KEYFILE = jsonRpcHelper.KEY_FILE Line 35: CERTFILE = jsonRpcHelper.CRT_FILE Line 36: CACERT = None Line 34: KEYFILE = jsonRpcHelper.KEY_FILE Line 35: CERTFILE = jsonRpcHelper.CRT_FILE Line 36: CACERT = None Line 37: HOST = '127.0.0.1' Line 38: PORT = 8443 What if this port is used? better use port=0, let the kernel select unused port for you, and then use the selected port in the client: _, port = self.server.socket.getsockname() Line 39: Line 40: Line 41: class SimpleIPXMLRPCServer(SimpleXMLRPCServer.SimpleXMLRPCServer): Line 42: # Create daemon threads when mixed with SocketServer.ThreadingMixIn Line 37: HOST = '127.0.0.1' Line 38: PORT = 8443 Line 39: Line 40: Line 41: class SimpleIPXMLRPCServer(SimpleXMLRPCServer.SimpleXMLRPCServer): We don't need this class, delete it. Line 42: # Create daemon threads when mixed with SocketServer.ThreadingMixIn Line 43: daemon_threads = True Line 44: Line 45: def __init__(self, addr, requestHandler=IPXMLRPCRequestHandler, Line 51: allow_none=allow_none, encoding=encoding, Line 52: bind_and_activate=bind_and_activate) Line 53: Line 54: Line 55: class SecureXMLRPCServer(SimpleIPXMLRPCServer): This class is not needed as well, delete it. Line 56: def __init__(self, addr, Line 57: requestHandler=IPXMLRPCRequestHandler, Line 58: logRequests=True, allow_none=False, encoding=None, Line 59: bind_and_activate=True, Line 88: class TestServer(): Line 89: Line 90: def __init__(self): Line 91: self.server = SecureXMLRPCServer((HOST, PORT), keyfile=KEYFILE, Line 92: certfile=CERTFILE, ca_certs=CACERT) Since we are not testing the server, Python builtin server is just fine: self.server = SimpleXMLRPCServer.SimpleXMLRPCServer((HOST, PORT), logRequests=False) To create a secure server, wrap it's socket: self.server.socket = SSLServerSocket(raw=self.server.socket, keyfile=KEYFILE, certfile=CERTFILE, ca_certs=CACERT) Line 93: self.server.register_instance(MathService()) Line 94: self.start() Line 95: Line 96: def start(self): Line 115: def add(self, numer1, number2): Line 116: return self.proxy.add(numer1, number2) Line 117: Line 118: def stop(self): Line 119: self.transport.close() First, this is not stop - client are not started. Rename this to close. From http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9491/console: AttributeError: VerifyingSafeTransport instance has no attribute 'close' This look like a difference between Python 2.7 (has close) and 2.6 (no close). You can do: if hasattr(self.transport, 'close'): self.transport.close() Line 120: Line 121: Line 122: class VerifyingTransportTests(TestCaseBase): Line 123: Line
Change in vdsm[master]: vdsm: verifyingTransport testing
Nir Soffer has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 3: (1 comment) http://gerrit.ovirt.org/#/c/28858/3/tests/verifyingTransportTests.py File tests/verifyingTransportTests.py: Line 120: Line 121: Line 122: class VerifyingTransportTests(TestCaseBase): Line 123: Line 124: def test_verifing_transport(self): You just repeat the class name here. Should be named to test_valid. And you must add test that actually test that the trasport is verying by using different certfile/keyfile. Line 125: server = TestServer() Line 126: client = VerifingClient() Line 127: self.assertEquals(client.add(2, 3), 5) Line 128: client.stop() -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: verifyingTransport testing
oVirt Jenkins CI Server has posted comments on this change. Change subject: vdsm: verifyingTransport testing .. Patch Set 2: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9433/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/759/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10216/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10372/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5298/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3456/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/28858 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a706d4d90fdf446b06530288d947d96934e45f2 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
