Ryan Barry has posted comments on this change.

Change subject: ovirt-auto-install: fix imports
......................................................................


Patch Set 1: Code-Review-1

(2 comments)

https://gerrit.ovirt.org/#/c/43175/1/scripts/ovirt-auto-install.py
File scripts/ovirt-auto-install.py:

Line 28: import sys
Line 29: import os
Line 30: import time
Line 31: 
Line 32: from ovirtnode.install import Install
Please import this conditionally under PerformInstallation
Line 33: from ovirtnode.ovirtfunctions import is_iscsi_install, is_stateless
Line 34: 
Line 35: OVIRT_VARS = defaults.NodeConfigFile().get_dict()
Line 36: 


Line 30: import time
Line 31: 
Line 32: from ovirtnode.install import Install
Line 33: from ovirtnode.ovirtfunctions import is_iscsi_install, is_stateless
Line 34: 
is_iscsi_install and is_stateless are extremely small functions which would be 
easy to re-implement locally in ovirt-auto-install.

Yes, this is code duplication, which is bad. But ovirtnode.* does so many 
global things that it's probably better for maintainability (especially with 
any changes with augeas) to not import ovirtnode.* at the top level unless we 
absolutely need to.
Line 35: OVIRT_VARS = defaults.NodeConfigFile().get_dict()
Line 36: 
Line 37: 
Line 38: class PrepareInstallation(Transaction.Element):


-- 
To view, visit https://gerrit.ovirt.org/43175
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbe43cc316540752ed9710fb2eca96f4c2cf8670
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ryan Barry <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
node-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/node-patches

Reply via email to