Ryan Barry has posted comments on this change. Change subject: If miss custom Syslog port, using default syslog port "514" to set ......................................................................
Patch Set 1: Code-Review+1 (3 comments) http://gerrit.ovirt.org/#/c/28346/1/src/ovirtnode/log.py File src/ovirtnode/log.py: Line 146: def syslog_auto(): Line 147: host = "" Line 148: port = "" Line 149: if (not "OVIRT_SYSLOG_SERVER" in _functions.OVIRT_VARS and Line 150: not "OVIRT_SYSLOG_PORT" in _functions.OVIRT_VARS): Changing this bars users from using DNS discovery through SRV records (_functions.find_srv(...)), which appears to be all this if statement is doing. No change should be needed here Line 151: logger.info("Attempting to locate remote syslog server...") Line 152: try: Line 153: host, port = _functions.find_srv("syslog", "udp") Line 154: except: Line 152: try: Line 153: host, port = _functions.find_srv("syslog", "udp") Line 154: except: Line 155: pass Line 156: if not host is "" and not port is "": I haven't tested, but I'm guessing things are going wrong here. find_srv returns False if it can't find it, so neither will be empty strings and this check will pass. Changing this to "if not host and not port" may resolve. Line 157: logger.info("Found! Using syslog server " + host + ":" + port) Line 158: ovirt_rsyslog(host, port, "udp") Line 159: return True Line 160: else: Line 161: logger.warn("Syslog server not found!") Line 162: return False Line 163: elif ("OVIRT_SYSLOG_SERVER" in _functions.OVIRT_VARS and Line 164: not "OVIRT_SYSLOG_PORT" in _functions.OVIRT_VARS): Line 165: logger.info("Using custom syslog server and default port" + For consistency, we're ok just saying "Using default syslog port" without any mention of a specified server Line 166: _functions.OVIRT_VARS["OVIRT_SYSLOG_SERVER"] + ":" + Line 167: "514" + ".") Line 168: ovirt_rsyslog(_functions.OVIRT_VARS["OVIRT_SYSLOG_SERVER"], Line 169: "514", "udp") -- To view, visit http://gerrit.ovirt.org/28346 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id1eadbac0749d630f67a3bcd60f2f7791fffd5f2 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-node Gerrit-Branch: master Gerrit-Owner: hadong <[email protected]> Gerrit-Reviewer: Ryan Barry <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ node-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/node-patches
