Alon Bar-Lev has posted comments on this change.
Change subject: packaging: setup: revert nfs config on cleanup
......................................................................
Patch Set 14:
(6 comments)
....................................................
File packaging/setup/ovirt_engine_setup/util.py
Line 110:
Line 111: if before is not None:
Line 112: re_before = re.compile(pattern=before)
Line 113: if after is not None:
Line 114: re_after = re.compile(pattern=after)
why not get pre-compiled as parameter?
Line 115:
Line 116: # Find params which appear in the input.
Line 117: uncommented = set()
Line 118: commented = set()
Line 193: else:
Line 194: action = None
Line 195: if before is not None and re_before.match(line):
Line 196: action = 'before'
Line 197: if after is not None and re_after.match(line):
you can check if re_after is not None, no need to drag variables.
I suggest the following function logical structure:
1. loop until 'after' is found, copy blindly lines.
2. loop until 'before' is found, perform subst logic.
3. loop until end, copy blindly lines.
This will make function much more readable and without edge conditions
(addcurrent_line is an example).
mycontent = content[:]
while (
mycontent and
re_after is not None and
not re_after.match(mycontent[0])
):
newcontent.append(mycontent.pop(0))
while (
mycontent and
(
re_before is None or
not re_before.match(mycontent[0])
)
):
line = mycontent.pop(0)
do logic of replacement
add parameters that are not found
while mycontent:
newcontent.append(mycontent.pop(0))
another option is to have state variable... however, as much as it first look
to be promising the result is more tightly coupled code.
(INITIAL, LOGIC, TERMINATION) = range(3)
state = INITIAL if re_after is None else LOGIC
for line in content:
if state == INITIAL:
newcontent.append(line)
if re_after.match(line):
state = LOGIC
elif state == LOGIC:
if re_before.match(line):
state = TERMINATION
add parameters that are not found
mycontent.append(line)
do logic of replacement
elif state == TERMINATION:
newcontent.append(line)
if state == LOGIC:
add parameters that are not found
Line 198: action = 'after'
Line 199: add_current_line = False
Line 200: newcontent.append(line)
Line 201: if action:
Line 197: if after is not None and re_after.match(line):
Line 198: action = 'after'
Line 199: add_current_line = False
Line 200: newcontent.append(line)
Line 201: if action:
is not None?
Line 202: for (param, value) in [
Line 203: (param, value)
Line 204: for param, value in params.items()
Line 205: if not param in foundparams
....................................................
File packaging/setup/plugins/ovirt-engine-setup/apache/ssl.py
Line 183: content=self._sslData.splitlines(),
Line 184: params=self._params,
Line 185: changed_lines=changed_lines,
Line 186: separator_re='\s+',
Line 187: new_line_tpl='{spaces}{param} {value}',
before the terminator? yes, I know this is impossible because of the above
validation, however, if someone removes it, we do want to keep file structure
intact.
BTW: I added the above validation only because I did not want to mess up with
the terminator :))
Line 188: ),
Line 189: )
Line 190: )
Line 191: self.environment[
....................................................
File packaging/setup/plugins/ovirt-engine-setup/provisioning/postgres.py
Line 135
Line 136
Line 137
Line 138
Line 139
I suggest to read content as is above and add function call here.
content=osetuputil.editConfigContent(...),
same to all bellow
Line 101: ):
Line 102: with open(filename, 'r') as f:
Line 103: content = osetuputil.editConfigContent(
Line 104: content=f.read().splitlines(),
Line 105: params={'max_connections': maxconn},
put each entry of dict in one line to ease future patch?
Line 106: )
Line 107:
Line 108: transaction.append(
Line 109: filetransaction.FileTransaction(
--
To view, visit http://gerrit.ovirt.org/19001
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74dd4c1556bd6479fcf10f85fcbe083f215e0854
Gerrit-PatchSet: 14
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sandro Bonazzola <sbona...@redhat.com>
Gerrit-Reviewer: Alex Lourie <alou...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Kiril Nesenko <knese...@redhat.com>
Gerrit-Reviewer: Ofer Schreiber <oschr...@redhat.com>
Gerrit-Reviewer: Ohad Basan <oba...@redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com>
Gerrit-Reviewer: Yedidyah Bar David <d...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
