Yedidyah Bar David has posted comments on this change. Change subject: packaging: setup: revert nfs config on cleanup ......................................................................
Patch Set 9: (4 comments) .................................................... File packaging/setup/ovirt_engine_setup/util.py Line 43: def editConfigContent( Line 44: content, Line 45: params, Line 46: changed_lines=None, Line 47: comment_re='[#]*\s*', I want to keep also the whitespace after the comment, and I do not see a reason to make it a separate group. Line 48: comment='# ', Line 49: separator_re='\s*=\s*', Line 50: separator=' = ', Line 51: ): Line 44: content, Line 45: params, Line 46: changed_lines=None, Line 47: comment_re='[#]*\s*', Line 48: comment='# ', Makes sense. Line 49: separator_re='\s*=\s*', Line 50: separator=' = ', Line 51: ): Line 52: """Return edited content of a config file. Line 46: changed_lines=None, Line 47: comment_re='[#]*\s*', Line 48: comment='# ', Line 49: separator_re='\s*=\s*', Line 50: separator=' = ', Makes sense. Line 51: ): Line 52: """Return edited content of a config file. Line 53: Line 54: Keyword arguments: Line 69: Params that appear uncommented in the input, are commented, and new Line 70: values are added after the commented lines. Params that appear only Line 71: commented in the input, the comments are copied as-is, and new lines Line 72: are added after the comments. Params that do not appear in the input Line 73: are added in the end. Current ssl.conf code assumes that the params we need are already there, perhaps commented. If they are not there, it practically means that the admin edited the file, and we can't know what to do without full parsing - hopefully she will choose not to edit the file during setup, but we probably need to refuse trying to edit. Consider e.g. multiple virtual hosts with different keys/certs etc. For xml I think it's much more healthy to use an xml parser. I do not claim that this function can't do other things and that we should not add 'below=', I just think this can be delayed. Practically only host below local is already needing this functionality, and this case is not really natural for this function because we simply do not have there a "param" and a "value". Line 74: """ Line 75: # TODO: Add more options that will allow using this function Line 76: # in other places. Examples: Line 77: # setup/plugins/ovirt-engine-setup/apache/ssl.py -- 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: 9 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Sandro Bonazzola <sbona...@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
