Fabian Deutsch has posted comments on this change. Change subject: config: Fix logrotate migration ......................................................................
Patch Set 1: (3 comments) https://gerrit.ovirt.org/#/c/41347/1/src/ovirt/node/config/migrate.py File src/ovirt/node/config/migrate.py: Line 119: Line 120: def translate_logrotate(self): Line 121: if self.__is_persisted("/etc/logrotate.d/ovirt-node"): Line 122: logrotate_size = ovirtfunctions.get_logrotate_size() Line 123: if logrotate_size and logrotate_size is not 1024: > I would use logrotate_size != 1024 or even set the 1024 as in presets.py an != is a good idea. presets is a good idea, but I prefer to do that in a different commit. To have one safe commit for 3.5.z and one commit who fixes it more broadly. Line 124: self.aug.set( Line 125: "/files/etc/default/ovirt/OVIRT_LOGROTATE_MAX_SIZE", Line 126: str(logrotate_size) or "") Line 127: https://gerrit.ovirt.org/#/c/41347/1/src/ovirtnode/ovirtfunctions.py File src/ovirtnode/ovirtfunctions.py: Line 1599: return (accepted, message) Line 1600: Line 1601: def get_logrotate_size(): Line 1602: size = augtool_get("/files/etc/logrotate.d/ovirt-node/rule/size") Line 1603: if not size: > I think the validation is much faster if: size can either be None or "", to have _one_ return value I'm returning None. In your case we could return either None or "", which si not to nice. Line 1604: return None Line 1605: if "m" in size.lower(): Line 1606: multiplier = 1024 Line 1607: else: Line 1602: size = augtool_get("/files/etc/logrotate.d/ovirt-node/rule/size") Line 1603: if not size: Line 1604: return None Line 1605: if "m" in size.lower(): Line 1606: multiplier = 1024 > presets? :) :) But this time the constant is for unit conversion, so having 1024 hardcoded here is fine. Line 1607: else: Line 1608: multiplier = 1 Line 1609: size = size.lower().rstrip("kmb") Line 1610: size = int(size) * multiplier -- To view, visit https://gerrit.ovirt.org/41347 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ida64eafd93e6c678f8f718f494f396830661b7aa Gerrit-PatchSet: 1 Gerrit-Project: ovirt-node Gerrit-Branch: master Gerrit-Owner: Fabian Deutsch <[email protected]> Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]> Gerrit-Reviewer: Fabian Deutsch <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ node-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/node-patches
