This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/203/ |
config/roles/domaincontroller/role.py (Diff revision 2) | |||
---|---|---|---|
249 | result = yield async.subprocess_future(ipa_install_args) |
260 | # result = yield async.subprocess_future(ipa_install_args) |
261 | # |
||
262 | # if result.status: |
||
263 | # # If the subprocess returned non-zero, raise an exception |
||
264 | # raise RolekitError(COMMAND_FAILED, "%d" % result.status) |
Do you want to resurrect this code later? If not, these lines should just go away.
config/roles/domaincontroller/role.py (Diff revision 2) | |||
---|---|---|---|
251 | if result.status: |
266 | raise NotImplementedError |
252 | # If the subprocess returned non-zero, raise an exception |
||
253 | raise RolekitError(COMMAND_FAILED, "%d" % result.status) |
||
254 | 267 | ||
255 | # Create the systemd target definition |
268 | # Create the systemd target definition |
256 | target = {'Role': 'domaincontroller', |
269 | target = {'Role': 'domaincontroller', |
257 | 'Instance': self.get_name(), |
270 | 'Instance': self.get_name(), |
258 | 'Description': "Domain Controller Role - %s" % |
271 | 'Description': "Domain Controller Role - %s" % |
The
raise
in line 266 prevents any later lines from being executed, should they just go away, or is this in the wrong place?
config/roles/domaincontroller/role.py (Diff revision 2) | |||
---|---|---|---|
476 | allowed = re.compile("(?!-)[A-Z\d-]{1,63}(?<!-)$", re.IGNORECASE) |
||
477 | return all(allowed.match(x) for x in fqdn.split(".")) |
The compiled regex object is used and immediately thrown away here -- which is not a problem, just odd.
Not quoting the regex string with
r"..."
isn't a problem right now unless someone fiddles with it in the wrong way (i.e. when the backslash becomes a quote for the next character of the string and won't be passed on to the regex compiler)... but it's bad practice ;).You could just do this instead:
return all(re.match( r"(?!-)[A-Z\d-]{1,63}(?<!-)$", x, re.IGNORECASE) for x in fqdn.split("."))Or alternatively compile the regex object once and store it as a class member, of course.
- Nils Philippsen
On September 2nd, 2015, 4:14 nachm. CEST, Stephen Gallagher wrote:
Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen Gallagher, and Thomas Woerner.
By Stephen Gallagher.
Updated Sept. 2, 2015, 4:14 nachm.
Bugs:
43
Repository:
rolekit
Description
Testing
Diffs
|