[node-patches] Change in ovirt-node[master]: config: Fix logrotate migration

[fabiand at redhat.com]

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 <fabiand at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes