This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/219/ |
On September 22nd, 2015, 3 nachm. CEST, Stephen Gallagher wrote:
config/roles/databaseserver/role.py (Diff revision 2) 51 def tweak_lines(lines_iterable, tweaking_rules, append_if_missing=False):52 for rule in tweaking_rules:53 regex = rule['regex']54 if isinstance(regex, str):55 rule['regex'] = re.compile(regex)56 rule.setdefault('global', True)57 rule.setdefault('append_if_missing', append_if_missing)58 59 found_regexes = set()60 ignore_regexes = set()61 62 for line in lines_iterable:63 lines_to_append = []64 for rule in tweaking_rules:65 regex = rule['regex']66 if regex not in ignore_regexes:67 m = regex.search(line)68 if m:69 found_regexes.add(regex)70 if not rule['global']:71 ignore_regexes.add(regex)72 if 'replace' in rule:73 line = regex.sub(rule['replace'], line)74 if 'append' in rule:75 lines_to_append.append(rule['append'])76 yield line77 for l in lines_to_append:78 yield l + "\n"79 80 for rule in tweaking_rules:81 if (82 rule['append_if_missing'] and83 rule['regex'] not in found_regexes):84 if 'replace' in rule:85 yield rule['replace'] + "\n"86 if 'append' in rule:87 yield rule['append'] + "\n"88 89 This function is very difficult to follow. Please add many comments.
I don't like it very much myself ;), it's too unwieldy. It's basically a substitute for the functions of sed we used, you can match and
replace
orappend
, and optionally append at the end of the file if the regex never matched (for instance if future versions of the config file don't come with the matched directive so defaults get used). I thought about how to make this a bit more lean and easy to use so it's generally usable, but so far haven't come up with some concrete idea.
On September 22nd, 2015, 3 nachm. CEST, Stephen Gallagher wrote:
config/roles/databaseserver/role.py (Diff revision 2) 66 if regex not in ignore_regexes:67 m = regex.search(line)68 if m:69 found_regexes.add(regex)70 if not rule['global']:71 ignore_regexes.add(regex)How are we defining "global" regexes here? Per-file or per-line? That needs explanation. Right now, it looks like it's per-file.
The
global
option specifies that replacing/appending is done more than once, everytime the regex matches. Regex replacements are always done "globally" in a line.
On September 22nd, 2015, 3 nachm. CEST, Stephen Gallagher wrote:
config/roles/databaseserver/role.py (Diff revision 2) 81 if (82 rule['append_if_missing'] and83 rule['regex'] not in found_regexes):This whitespace usage is uncommon and hard to read.
Yeah. I think this is a place where backslash continuation is acceptable, see: https://www.python.org/dev/peps/pep-0008/#maximum-line-length
On September 22nd, 2015, 3 nachm. CEST, Stephen Gallagher wrote:
config/roles/databaseserver/role.py (Diff revision 2) 305 linkfile(conffile, bakfile)Can you add a comment about linkfile() here? Is this creating a hard-link? (And if so, is it also going to be overwritten by overwrite_safely()?)
Can you add a comment about linkfile() here? Is this creating a hard-link?
The
linkfile()
function is documented:slip.util.files.linkfile = linkfile(srcpath, dstpath) Hardlink srcpath to dstpath. Attempt to atomically replace dstpath if it exists.I think documentation about a function should be with the function, not where it's used.
(And if so, is it also going to be overwritten by overwrite_safely()?)
It's not a function to "scrub" a file containing secret data, if you mean that. It's just something to overwrite a file safely with new content:
slip.util.files.overwrite_safely = overwrite_safely(path, content, preserve_mode=True, preserve_context=True, preserve_ownership=True) Safely overwrite a file by creating a temporary file in the same directory, writing it, moving it over the original file, eventually preserving file mode, SELinux context and ownership.
- Nils
On September 19th, 2015, 3:04 vorm. CEST, Nils Philippsen wrote:
Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen Gallagher, and Thomas Woerner.
By Nils Philippsen.
Updated Sept. 19, 2015, 3:04 vorm.
Repository:
rolekit
Description
Testing
Diffs
|