)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"cb7a6dd319df5c3e984f109a08fdc5f1f38c279a","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Not like this:"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"    from nova.privsep import libvirt"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"This is because it makes it obvious at the caller that a priviledged"},{"line_number":18,"context_line":"operation is occuring:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5fc1f717_803e44b7","line":15,"updated":"2019-04-01 19:47:55.000000000","message":"++ this is a great idea, for all the reasons you state.\n\nIs it documented somewhere?","commit_id":"b5df2d428e9b4da1b0736ead2b98c320005e8287"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0f364fcf8bebd8e3abbc91b5cc28b7ed1613f9fa","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Not like this:"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"    from nova.privsep import libvirt"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"This is because it makes it obvious at the caller that a priviledged"},{"line_number":18,"context_line":"operation is occuring:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5fc1f717_ab3b21c6","line":15,"in_reply_to":"5fc1f717_6099d877","updated":"2019-04-01 21:01:16.000000000","message":"https://review.openstack.org/649188","commit_id":"b5df2d428e9b4da1b0736ead2b98c320005e8287"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"bb0cb651523e64e55bee681d280d919627cf12be","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Not like this:"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"    from nova.privsep import libvirt"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"This is because it makes it obvious at the caller that a priviledged"},{"line_number":18,"context_line":"operation is occuring:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5fc1f717_a06ee093","line":15,"in_reply_to":"5fc1f717_803e44b7","updated":"2019-04-01 20:02:55.000000000","message":"I do not think it is documented anywhere. Certainly a quick grep doesn\u0027t show it. I\u0027m not opposed to documenting it, but is a hacking rule more appropriate? Do we even still use hacking?","commit_id":"b5df2d428e9b4da1b0736ead2b98c320005e8287"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"beef016364c005df790fdf6733538ccba1085afd","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Not like this:"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"    from nova.privsep import libvirt"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"This is because it makes it obvious at the caller that a priviledged"},{"line_number":18,"context_line":"operation is occuring:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5fc1f717_6099d877","line":15,"in_reply_to":"5fc1f717_a06ee093","updated":"2019-04-01 20:13:37.000000000","message":"We do. A hacking rule would be perfect.","commit_id":"b5df2d428e9b4da1b0736ead2b98c320005e8287"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"204904aec79d614f493cf1aa5a58e532c1831085","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Style corrections for privsep usage."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We always import privsep modules like this:"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"    import nova.privsep.libvirt"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Not like this:"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"    from nova.privsep import libvirt"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"This is because it makes it obvious at the caller that a priviledged"},{"line_number":18,"context_line":"operation is occuring:"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"    nova.privsep.libvirt.destroy_root_filesystem()"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Not just:"},{"line_number":23,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5fc1f717_264f6893","line":20,"range":{"start_line":9,"start_character":0,"end_line":20,"end_character":50},"updated":"2019-04-01 23:04:04.000000000","message":"i covered this in https://review.openstack.org/#/c/649188/1//COMMIT_MSG@22\n\nbut i believe the general pattern of having a \u003cproject\u003e.privsep.* \nmodule structure encourages misuse of privsep and requires\nconventions like not abrivating the imports to work around the fact that it is not obvious it is a privileged function\n\n\na better patter in my opinion is to add privileged to the function name and to group the privileged in the same module as the unprivileged functions that call them.\n\nthis encourages targeted privileged function instead of generic privileged functions.\n\nthat said while we have all function grouped under nova.privsep i think this does improve visablity but\nit really fixing a problem of our own making.","commit_id":"96d74b8e42f7a409661239e600e7656f4167d993"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"399e971d1c013153240b7a72fc9d6fe7af7d9d5b","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Style corrections for privsep usage."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We always import privsep modules like this:"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"    import nova.privsep.libvirt"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Not like this:"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"    from nova.privsep import libvirt"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"This is because it makes it obvious at the caller that a priviledged"},{"line_number":18,"context_line":"operation is occuring:"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"    nova.privsep.libvirt.destroy_root_filesystem()"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Not just:"},{"line_number":23,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5fc1f717_5f7cb1ea","line":20,"range":{"start_line":9,"start_character":0,"end_line":20,"end_character":50},"in_reply_to":"5fc1f717_264f6893","updated":"2019-04-02 10:42:02.000000000","message":"I don\u0027t necessarily disagree with you, Sean, but this series is a step on the road, not the last word. IMO a big benefit to the way this series is setting  things up is discoverability: making it easier for the next stage - which might do things just as you say - to identify all the places that need to be touched.","commit_id":"96d74b8e42f7a409661239e600e7656f4167d993"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f67d72a78da76b06808d265e064d72cdf539d993","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Style corrections for privsep usage."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We always import privsep modules like this:"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"    import nova.privsep.libvirt"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Not like this:"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"    from nova.privsep import libvirt"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"This is because it makes it obvious at the caller that a priviledged"},{"line_number":18,"context_line":"operation is occuring:"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"    nova.privsep.libvirt.destroy_root_filesystem()"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Not just:"},{"line_number":23,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5fc1f717_a21c9ac1","line":20,"range":{"start_line":9,"start_character":0,"end_line":20,"end_character":50},"in_reply_to":"5fc1f717_5f7cb1ea","updated":"2019-04-02 11:30:21.000000000","message":"oh yes i know which is why i did not -1 this patch.\n\ngiven this is how we already do thing at present then the visablity aspect of this change are definetly worth it.\n\ni -1\u0027d the hacking change as that was codefying a check for something i think is a design/architecural miss step and i wanted to make sure we atleast talked about it.\n\nbut im fine with actuly doing this for now provided as you said this si not the endstate and we contiue to improve it.","commit_id":"96d74b8e42f7a409661239e600e7656f4167d993"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"c36c472f92f44a555ee7b356a4bc91a669a8d213","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Style corrections for privsep usage."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We always import privsep modules like this:"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"    import nova.privsep.libvirt"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Not like this:"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"    from nova.privsep import libvirt"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"This is because it makes it obvious at the caller that a priviledged"},{"line_number":18,"context_line":"operation is occuring:"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"    nova.privsep.libvirt.destroy_root_filesystem()"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Not just:"},{"line_number":23,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5fc1f717_6ebd3901","line":20,"range":{"start_line":9,"start_character":0,"end_line":20,"end_character":50},"in_reply_to":"5fc1f717_a21c9ac1","updated":"2019-04-02 13:51:16.000000000","message":"++\n\nThe new version of the hacking check patch is here [1] in case you want to -1 it as well :)  IMO it still has merit for the same reasons of strictness and discoverability for the future. But I guess we can discuss there.\n\n[1] https://review.openstack.org/#/c/649190/","commit_id":"96d74b8e42f7a409661239e600e7656f4167d993"}]}