Keith Robertson has posted comments on this change. Change subject: packaging: Fixed creating objects in an insecure way ......................................................................
Patch Set 2: No score; No score (1 inline comment) .................................................... File src/engine-iso-uploader.py Line 413: if self.configuration.get("insecure"): Line 414: params["insecure"] = True Line 415: # Otherwise, use ca_file Line 416: else: Line 417: params["ca_file"] = self.configuration.get("engine_ca") Supplying insecure && ca_file. If they are both set, which way will the API go? It is *not* clear and not documented. Further they are mutually exclusive arguments because you can't both be "insecure" and ask the API to validate the server. IMO the API needs to throw an exception but, if you really want to do some sort of preferential policy ... then fine. Finally, what if I gave the API http://someserver and supplied ca_file? What would the API do? IMO that is also inconsistent and should constitute an error condition. Line 418: Line 419: # Create an API object Line 420: self.api = API(**params) Line 421: -- To view, visit http://gerrit.ovirt.org/10815 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I015c6b5441f0d1e33bb3aae378a59ad8557f9da5 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-iso-uploader Gerrit-Branch: master Gerrit-Owner: Alex Lourie <alou...@redhat.com> Gerrit-Reviewer: Alex Lourie <alou...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Keith Robertson <krobe...@redhat.com> Gerrit-Reviewer: Kiril Nesenko <knese...@redhat.com> Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com> Gerrit-Reviewer: Ofer Schreiber <oschr...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches