Douglas Schilling Landgraf has posted comments on this change. Change subject: vdsm-tool: Add register verb ......................................................................
Patch Set 1: (9 comments) https://gerrit.ovirt.org/#/c/40966/1/lib/vdsm/tool/register.py File lib/vdsm/tool/register.py: Line 16: # also available at http://www.gnu.org/copyleft/gpl.html. Line 17: import argparse Line 18: import getpass Line 19: import logging Line 20: import M2Crypto > M2Crypto and selinux are not part of python's stdlib. import them in anothe ok Line 21: import os Line 22: import pwd Line 23: import requests Line 24: import selinux Line 177: Print and log a message Line 178: """ Line 179: print(msg) Line 180: Line 181: if level == "error": > logging.log(level, msg) accepts a (numeric) level, which makes this long if nice to know, going to check the print and log in logging module. Line 182: self.logger.error(msg, exc_info=True) Line 183: elif level == "debug": Line 184: self.logger.debug(msg) Line 185: elif level == "warning": Line 188: self.logger.critical(msg) Line 189: elif level == "info": Line 190: self.logger.info(msg) Line 191: Line 192: def __set_logger(self): > we don't use these uber-private double-underscore in new vdsm code. It's no Okay, I will remove the uber private. The point was to only make public methods that are really required to be. Line 193: """ Line 194: The logging settings Line 195: Saving log in: /var/log/vdsm/register.log Line 196: """ Line 193: """ Line 194: The logging settings Line 195: Saving log in: /var/log/vdsm/register.log Line 196: """ Line 197: # Avoid node side effects of logging module not been working > could you explain this a bit better? what's the node issue you are trying t The issue is that log is not created at all when using from ovirt.node.utils.fs import Config in the top of source, this is a issue that ovirt-node team must to address. After the reload the log file is created correctly. I have just created the bug: https://bugzilla.redhat.com/show_bug.cgi?id=1222236 I will make a reference into the source. Line 198: logging.shutdown() Line 199: reload(logging) Line 200: Line 201: logging.basicConfig( Line 204: format='%(asctime)s %(message)s', Line 205: datefmt='%m/%d/%Y %I:%M:%S %p', Line 206: ) Line 207: Line 208: if sys.version_info >= (2, 7): > we now support only python>=2.7, so this can be dropped. Done Line 209: logging.captureWarnings(True) Line 210: Line 211: return logging.getLogger(__name__) Line 212: Line 262: with open(cert, 'r') as f: Line 263: cert = f.read() Line 264: Line 265: x509 = M2Crypto.X509.load_cert_string(cert, M2Crypto.X509.FORMAT_PEM) Line 266: fp = x509.get_fingerprint('sha1') > please use ok. Line 267: fp = ':'.join(fp[pos:pos + 2] for pos in range(0, len(fp), 2)) Line 268: Line 269: return fp Line 270: Line 271: def download_ca(self): Line 272: """ Line 273: Download CA from Engine and save self.ca_engine Line 274: """ Line 275: if self.reg_protocol == "legacy": > vdsm_id-handling code must not in its own function - we "download ca" shoul "must not"? I understand you are saying vdsm_id hanlding requires it's own method. Line 276: # The legacy version uses the format: UUID_MACADDRESS Line 277: self.uuid = getHostUUID(legacy=True) Line 278: self.url_reg += "&vds_unique_id={u}".format(u=self.uuid) Line 279: else: Line 488: - In case, there is no UUID, use auxiliary function from VDSM Line 489: to generate it and store in /etc/vdsm/vdsm.id Line 490: Line 491: Legacy reg: Line 492: ============ > Which Engine versions require this Legacy reg? Done Line 493: - Process UUID Line 494: Line 495: - Download CA via Line 496: https://ENGINE_FQDN Line 499: https://ENGINE_FQDN/engine.ssh.key.txt Line 500: Line 501: - Register via URL: Line 502: (Original .NET version and earlier Linux versions) Line 503: https://ENGINE_FQDN/RHEVManagerWeb/VdsAutoRegistration.aspx?vds_ip=NODE_FQDN_OR_IP&vds_name=NODE_NAME&vds_unique_id=NODE_UUID&port=54321 > Please break this line. pep8-1.4.6 does not like it, and frankly - neither Done Line 504: Line 505: or Line 506: Line 507: https://ENGINE_FQDN/OvirtEngineWeb/register?vds_ip=NODE_FQDN_OR_IP&vds_name=NODE_NAME&vds_unique_id=NODE_UUID&port=54321 -- To view, visit https://gerrit.ovirt.org/40966 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ica800027beec1e5a20165bb5e1e78baf9283c1da Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Douglas Schilling Landgraf <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
