Alon Bar-Lev has posted comments on this change.

Change subject: Log setup events
......................................................................


Patch Set 2: (7 inline comments)

Please avoid puting +1 for your own code... :)

....................................................
File packaging/fedora/setup/ovirt-engine-log-setup-event.sh.in
Line 1: #!/bin/sh
please move it out of setup and put it into packaging/bin, fedora/setup will be 
removed soon.
Line 2: # ovirt-engine-log-setup-event.sh - log an event during 
setup/upgrade/cleanup
Line 3: #
Line 4: # It logs to a file which should not rotate and allow getting a clear 
view of
Line 5: # the entire line of versions installed/upgraded/cleaned/etc.


....................................................
File packaging/setup/plugins/ovirt-engine-remove/core/misc.py
Line 104
Line 105
Line 106
Line 107
Line 108
please move it to distro-rpm, within its own plugin, and make sure you test for 
distribution.

please put this in common, so that we have this running at remove and setup.


Line 115:             ),
Line 116:         )
Line 117: 
Line 118:     @plugin.event(
Line 119:         stage=plugin.Stages.STAGE_INIT,
please put this at SETUP (at least), I think more appropriate place is at 
STAGE_TRANSACTION_BEGIN, so we have this when we truly going to do something.
Line 120:     )
Line 121:     def _log_setup_event_init(self):
Line 122:         rc, stdout, stderr = self.execute(
Line 123:             (


Line 118:     @plugin.event(
Line 119:         stage=plugin.Stages.STAGE_INIT,
Line 120:     )
Line 121:     def _log_setup_event_init(self):
Line 122:         rc, stdout, stderr = self.execute(
if you do not use rc, stdout, stderr do not specify...
Line 123:             (
Line 124:                 osetupcons.FileLocations.OVIRT_ENGINE_LOG_SETUP_EVENT,
Line 125:                 '--notes=Start of engine-cleanup (otopi)'
Line 126:             )


Line 121:     def _log_setup_event_init(self):
Line 122:         rc, stdout, stderr = self.execute(
Line 123:             (
Line 124:                 osetupcons.FileLocations.OVIRT_ENGINE_LOG_SETUP_EVENT,
Line 125:                 '--notes=Start of engine-cleanup (otopi)'
please always put comma at end of statements so if we add more line we have 
smaller diff.
Line 126:             )
Line 127:         )
Line 128: 
Line 129:     @plugin.event(


Line 122:         rc, stdout, stderr = self.execute(
Line 123:             (
Line 124:                 osetupcons.FileLocations.OVIRT_ENGINE_LOG_SETUP_EVENT,
Line 125:                 '--notes=Start of engine-cleanup (otopi)'
Line 126:             )
same.
Line 127:         )
Line 128: 
Line 129:     @plugin.event(
Line 130:         stage=plugin.Stages.STAGE_CLOSEUP,


Line 126:             )
Line 127:         )
Line 128: 
Line 129:     @plugin.event(
Line 130:         stage=plugin.Stages.STAGE_CLOSEUP,
should be in cleanup to execute always.
Line 131:     )
Line 132:     def _log_setup_event_closeup(self):
Line 133:         rc, stdout, stderr = self.execute(
Line 134:             (


-- 
To view, visit http://gerrit.ovirt.org/15863
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iae455ff0ef6475f00243906738474134f6f4e6e3
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yedidyah Bar David <[email protected]>
Gerrit-Reviewer: Alex Lourie <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Kiril Nesenko <[email protected]>
Gerrit-Reviewer: Moran Goldboim <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Yedidyah Bar David <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to