)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"7361a28f24f1f3f4a8b4e1e9ecbe4e81dacee9e0","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add rootwrap daemon mode support"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add a configuration option \u0027use_rootwrap_daemon\u0027 to"},{"line_number":10,"context_line":"switch on the rootwrap daemon mode support. This"},{"line_number":11,"context_line":"option is switched off by default to keep the"},{"line_number":12,"context_line":"current behavior as is."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"9a0c5dc1_47fd3c39","line":9,"updated":"2015-05-13 16:03:51.000000000","message":"Can you please add a DocImpact for this","commit_id":"5b885688c1e799f96092977d122835be062f0331"}],"nova/tests/unit/test_utils.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ee42be18ef63d896f54b2241ca7c3408e4f5a0c7","unresolved":false,"context_lines":[{"line_number":220,"context_line":"        cmd \u003d utils._get_root_helper()"},{"line_number":221,"context_line":"        self.assertEqual(\u0027sudo nova-rootwrap foo\u0027, cmd)"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    @mock.patch(\u0027nova.utils.CONF\u0027)"},{"line_number":224,"context_line":"    @mock.patch(\u0027nova.utils.RootwrapProcessHelper\u0027)"},{"line_number":225,"context_line":"    def test_get_root_helper_proc(self, mock_proc_helper, mock_conf):"},{"line_number":226,"context_line":"        mock_conf.rootwrap_config \u003d \u0027/path/to/conf\u0027"}],"source_content_type":"text/x-python","patch_set":31,"id":"ba3cc151_dd92dff5","line":223,"updated":"2015-07-02 15:01:14.000000000","message":"Don\u0027t patch CONF :) Instead, simply do this:\n\n @mock.patch(\u0027nova.utils.RootwrapProcessHelper\u0027)\n def test_get_root_helper_proc(self, mock_proc_helper):\n     self.flags(use_root_wrapdaemon\u003dFalse)\n     self.flags(rootwrap_config\u003d\"/path/to/conf\")\n     utils._get_rootwrap_helper()\n     mock_proc_helper.assert_called_once_with()\n\nDitto for all below..","commit_id":"fb025e068406b6e76c9e3f71fd075212f5ed5bd3"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"e41fb800ea79fda7061ebe993b91133d5f146693","unresolved":false,"context_lines":[{"line_number":220,"context_line":"        cmd \u003d utils._get_root_helper()"},{"line_number":221,"context_line":"        self.assertEqual(\u0027sudo nova-rootwrap foo\u0027, cmd)"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    @mock.patch(\u0027nova.utils.CONF\u0027)"},{"line_number":224,"context_line":"    @mock.patch(\u0027nova.utils.RootwrapProcessHelper\u0027)"},{"line_number":225,"context_line":"    def test_get_root_helper_proc(self, mock_proc_helper, mock_conf):"},{"line_number":226,"context_line":"        mock_conf.rootwrap_config \u003d \u0027/path/to/conf\u0027"}],"source_content_type":"text/x-python","patch_set":31,"id":"ba3cc151_9e852ad0","line":223,"in_reply_to":"ba3cc151_dd92dff5","updated":"2015-07-02 16:46:08.000000000","message":"Done","commit_id":"fb025e068406b6e76c9e3f71fd075212f5ed5bd3"}],"nova/utils.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"7361a28f24f1f3f4a8b4e1e9ecbe4e81dacee9e0","unresolved":false,"context_lines":[{"line_number":302,"context_line":"                    cmd, process_input)"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"                end_time \u003d time.time() - start_time"},{"line_number":305,"context_line":"                LOG.log(loglevel, \u0027CMD \"%s\" returned: %s in %0.3fs\u0027 %"},{"line_number":306,"context_line":"                        (sanitized_cmd, returncode, end_time))"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"                if not ignore_exit_code and returncode not in check_exit_code:"}],"source_content_type":"text/x-python","patch_set":17,"id":"9a0c5dc1_077aa4f1","line":305,"updated":"2015-05-13 16:03:51.000000000","message":"can we use a , instead of the %\n\nmissing translation\n\nshould be dict format and not %s and %0.3fs","commit_id":"5b885688c1e799f96092977d122835be062f0331"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ee42be18ef63d896f54b2241ca7c3408e4f5a0c7","unresolved":false,"context_lines":[{"line_number":33,"context_line":"import struct"},{"line_number":34,"context_line":"import sys"},{"line_number":35,"context_line":"import tempfile"},{"line_number":36,"context_line":"import threading"},{"line_number":37,"context_line":"import time"},{"line_number":38,"context_line":"from xml.sax import saxutils"},{"line_number":39,"context_line":""}],"source_content_type":"text/x-python","patch_set":31,"id":"ba3cc151_cf2c669b","line":36,"updated":"2015-07-02 15:01:14.000000000","message":"This needs to be eventlet-patched, no?","commit_id":"fb025e068406b6e76c9e3f71fd075212f5ed5bd3"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"e41fb800ea79fda7061ebe993b91133d5f146693","unresolved":false,"context_lines":[{"line_number":33,"context_line":"import struct"},{"line_number":34,"context_line":"import sys"},{"line_number":35,"context_line":"import tempfile"},{"line_number":36,"context_line":"import threading"},{"line_number":37,"context_line":"import time"},{"line_number":38,"context_line":"from xml.sax import saxutils"},{"line_number":39,"context_line":""}],"source_content_type":"text/x-python","patch_set":31,"id":"ba3cc151_b93a8897","line":36,"in_reply_to":"ba3cc151_cf2c669b","updated":"2015-07-02 16:46:08.000000000","message":"nova/cmd/__init__.py is the single spot we are patching eventlet for all of nova. so we should be good","commit_id":"fb025e068406b6e76c9e3f71fd075212f5ed5bd3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ee42be18ef63d896f54b2241ca7c3408e4f5a0c7","unresolved":false,"context_lines":[{"line_number":236,"context_line":""},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"class RootwrapProcessHelper(object):"},{"line_number":239,"context_line":"    def trycmd(self, *args, **kwargs):"},{"line_number":240,"context_line":"        discard_warnings \u003d kwargs.pop(\u0027discard_warnings\u0027, False)"},{"line_number":241,"context_line":"        try:"},{"line_number":242,"context_line":"            out, err \u003d self.execute(*args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":31,"id":"ba3cc151_af15629c","line":239,"updated":"2015-07-02 15:01:14.000000000","message":"I don\u0027t see the trycmd() method used in the new unit tests. Is this method used anywhere? If not, perhaps good to just leave it out of the patch?","commit_id":"fb025e068406b6e76c9e3f71fd075212f5ed5bd3"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"e41fb800ea79fda7061ebe993b91133d5f146693","unresolved":false,"context_lines":[{"line_number":236,"context_line":""},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"class RootwrapProcessHelper(object):"},{"line_number":239,"context_line":"    def trycmd(self, *args, **kwargs):"},{"line_number":240,"context_line":"        discard_warnings \u003d kwargs.pop(\u0027discard_warnings\u0027, False)"},{"line_number":241,"context_line":"        try:"},{"line_number":242,"context_line":"            out, err \u003d self.execute(*args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":31,"id":"ba3cc151_b41b6d80","line":239,"in_reply_to":"ba3cc151_af15629c","updated":"2015-07-02 16:46:08.000000000","message":"Good point, i\u0027ll add a couple of tests for trycmd.","commit_id":"fb025e068406b6e76c9e3f71fd075212f5ed5bd3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ee42be18ef63d896f54b2241ca7c3408e4f5a0c7","unresolved":false,"context_lines":[{"line_number":330,"context_line":"                    format \u003d _(\u0027%(desc)r\\ncommand: %(cmd)r\\n\u0027"},{"line_number":331,"context_line":"                               \u0027exit code: %(code)r\\nstdout: %(stdout)r\\n\u0027"},{"line_number":332,"context_line":"                               \u0027stderr: %(stderr)r\u0027)"},{"line_number":333,"context_line":"                    LOG.log(loglevel, format, {\"desc\": err.description,"},{"line_number":334,"context_line":"                                               \"cmd\": err.cmd,"},{"line_number":335,"context_line":"                                               \"code\": err.exit_code,"},{"line_number":336,"context_line":"                                               \"stdout\": err.stdout,"}],"source_content_type":"text/x-python","patch_set":31,"id":"ba3cc151_0fb9ee14","line":333,"updated":"2015-07-02 15:01:14.000000000","message":"loglevel should be std_logging.ERROR in this case, no? I feel it\u0027s kind of odd that the user supplies a log level to this function, and we log all errors as well as debugging information at that requested level... If this is how processutils.execute() works, then meh, OK :) But I still think it\u0027s weird :)","commit_id":"fb025e068406b6e76c9e3f71fd075212f5ed5bd3"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"e41fb800ea79fda7061ebe993b91133d5f146693","unresolved":false,"context_lines":[{"line_number":330,"context_line":"                    format \u003d _(\u0027%(desc)r\\ncommand: %(cmd)r\\n\u0027"},{"line_number":331,"context_line":"                               \u0027exit code: %(code)r\\nstdout: %(stdout)r\\n\u0027"},{"line_number":332,"context_line":"                               \u0027stderr: %(stderr)r\u0027)"},{"line_number":333,"context_line":"                    LOG.log(loglevel, format, {\"desc\": err.description,"},{"line_number":334,"context_line":"                                               \"cmd\": err.cmd,"},{"line_number":335,"context_line":"                                               \"code\": err.exit_code,"},{"line_number":336,"context_line":"                                               \"stdout\": err.stdout,"}],"source_content_type":"text/x-python","patch_set":31,"id":"ba3cc151_b96128ab","line":333,"in_reply_to":"ba3cc151_0fb9ee14","updated":"2015-07-02 16:46:08.000000000","message":"yes, this is the existing functionality in processutils.execute() so keeping the \"feature\"","commit_id":"fb025e068406b6e76c9e3f71fd075212f5ed5bd3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ee42be18ef63d896f54b2241ca7c3408e4f5a0c7","unresolved":false,"context_lines":[{"line_number":331,"context_line":"                               \u0027exit code: %(code)r\\nstdout: %(stdout)r\\n\u0027"},{"line_number":332,"context_line":"                               \u0027stderr: %(stderr)r\u0027)"},{"line_number":333,"context_line":"                    LOG.log(loglevel, format, {\"desc\": err.description,"},{"line_number":334,"context_line":"                                               \"cmd\": err.cmd,"},{"line_number":335,"context_line":"                                               \"code\": err.exit_code,"},{"line_number":336,"context_line":"                                               \"stdout\": err.stdout,"},{"line_number":337,"context_line":"                                               \"stderr\": err.stderr})"}],"source_content_type":"text/x-python","patch_set":31,"id":"ba3cc151_4f95b684","line":334,"updated":"2015-07-02 15:01:14.000000000","message":"The cmd needs to be santized with mask_password(), yes?","commit_id":"fb025e068406b6e76c9e3f71fd075212f5ed5bd3"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"e41fb800ea79fda7061ebe993b91133d5f146693","unresolved":false,"context_lines":[{"line_number":331,"context_line":"                               \u0027exit code: %(code)r\\nstdout: %(stdout)r\\n\u0027"},{"line_number":332,"context_line":"                               \u0027stderr: %(stderr)r\u0027)"},{"line_number":333,"context_line":"                    LOG.log(loglevel, format, {\"desc\": err.description,"},{"line_number":334,"context_line":"                                               \"cmd\": err.cmd,"},{"line_number":335,"context_line":"                                               \"code\": err.exit_code,"},{"line_number":336,"context_line":"                                               \"stdout\": err.stdout,"},{"line_number":337,"context_line":"                                               \"stderr\": err.stderr})"}],"source_content_type":"text/x-python","patch_set":31,"id":"ba3cc151_34175d49","line":334,"in_reply_to":"ba3cc151_4f95b684","updated":"2015-07-02 16:46:08.000000000","message":"see line 317, it\u0027s already sanitized.","commit_id":"fb025e068406b6e76c9e3f71fd075212f5ed5bd3"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"ae06b54e411851a5592d0c46b7122cb45c8c518a","unresolved":false,"context_lines":[{"line_number":79,"context_line":"               help\u003d\u0027Time period to generate instance usages for.  \u0027"},{"line_number":80,"context_line":"                    \u0027Time period must be hour, day, month or year\u0027),"},{"line_number":81,"context_line":"    cfg.BoolOpt(\u0027use_rootwrap_daemon\u0027, default\u003dFalse,"},{"line_number":82,"context_line":"                help\u003d\"Use rootwrap daemon mode.\"),"},{"line_number":83,"context_line":"    cfg.StrOpt(\u0027rootwrap_config\u0027,"},{"line_number":84,"context_line":"               default\u003d\"/etc/nova/rootwrap.conf\","},{"line_number":85,"context_line":"               help\u003d\u0027Path to the rootwrap configuration file to use for \u0027"}],"source_content_type":"text/x-python","patch_set":34,"id":"ba3cc151_f509b085","line":82,"updated":"2015-07-10 09:53:51.000000000","message":"I\u0027d really like to have a help which is more descriptive. This is another \"adjusting screw\" for a cloud operator. This help will be exported to the \"configuration reference\" manual and provides in its current form no hint what the impact is. It neither states which service uses this option, which could lead to a confusion which of my (assumed) many hosts need to have this config option in its \"nova.conf\".","commit_id":"3bf343a9c8875749a24224e89730ecceee63765b"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"c0248685570c3c237ab73dee7402d85d5e325993","unresolved":false,"context_lines":[{"line_number":79,"context_line":"               help\u003d\u0027Time period to generate instance usages for.  \u0027"},{"line_number":80,"context_line":"                    \u0027Time period must be hour, day, month or year\u0027),"},{"line_number":81,"context_line":"    cfg.BoolOpt(\u0027use_rootwrap_daemon\u0027, default\u003dFalse,"},{"line_number":82,"context_line":"                help\u003d\"Use rootwrap daemon mode.\"),"},{"line_number":83,"context_line":"    cfg.StrOpt(\u0027rootwrap_config\u0027,"},{"line_number":84,"context_line":"               default\u003d\"/etc/nova/rootwrap.conf\","},{"line_number":85,"context_line":"               help\u003d\u0027Path to the rootwrap configuration file to use for \u0027"}],"source_content_type":"text/x-python","patch_set":34,"id":"9a41bdd9_0e2bf383","line":82,"in_reply_to":"ba3cc151_f509b085","updated":"2015-07-11 03:13:03.000000000","message":"I tried something better, please suggest alternatives if you still think we need to enhance it. thanks Markus.","commit_id":"3bf343a9c8875749a24224e89730ecceee63765b"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"ae06b54e411851a5592d0c46b7122cb45c8c518a","unresolved":false,"context_lines":[{"line_number":376,"context_line":""},{"line_number":377,"context_line":"def trycmd(*args, **kwargs):"},{"line_number":378,"context_line":"    \"\"\"Convenience wrapper around oslo\u0027s trycmd() method.\"\"\""},{"line_number":379,"context_line":"    if \u0027run_as_root\u0027 in kwargs and kwargs.get(\u0027run_as_root\u0027):"},{"line_number":380,"context_line":"        if CONF.use_rootwrap_daemon:"},{"line_number":381,"context_line":"            return RootwrapDaemonHelper(CONF.rootwrap_config).trycmd("},{"line_number":382,"context_line":"                *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":34,"id":"ba3cc151_5829b388","line":379,"updated":"2015-07-10 09:53:51.000000000","message":"nit: could be:\n\n    if kwargs.get(\u0027run_as_root\u0027, False):","commit_id":"3bf343a9c8875749a24224e89730ecceee63765b"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"c0248685570c3c237ab73dee7402d85d5e325993","unresolved":false,"context_lines":[{"line_number":376,"context_line":""},{"line_number":377,"context_line":"def trycmd(*args, **kwargs):"},{"line_number":378,"context_line":"    \"\"\"Convenience wrapper around oslo\u0027s trycmd() method.\"\"\""},{"line_number":379,"context_line":"    if \u0027run_as_root\u0027 in kwargs and kwargs.get(\u0027run_as_root\u0027):"},{"line_number":380,"context_line":"        if CONF.use_rootwrap_daemon:"},{"line_number":381,"context_line":"            return RootwrapDaemonHelper(CONF.rootwrap_config).trycmd("},{"line_number":382,"context_line":"                *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":34,"id":"9a41bdd9_33066021","line":379,"in_reply_to":"ba3cc151_5829b388","updated":"2015-07-11 03:13:03.000000000","message":"Done","commit_id":"3bf343a9c8875749a24224e89730ecceee63765b"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"2ad1e982a81a24e6e407d34bdbe05e200bb37cb9","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                help\u003d\"Start and use a daemon that can run the commands that \""},{"line_number":83,"context_line":"                     \"need to be run with root privileges. This option is \""},{"line_number":84,"context_line":"                     \"usually enabled on nodes that run nova compute \""},{"line_number":85,"context_line":"                     \"processes\"),"},{"line_number":86,"context_line":"    cfg.StrOpt(\u0027rootwrap_config\u0027,"},{"line_number":87,"context_line":"               default\u003d\"/etc/nova/rootwrap.conf\","},{"line_number":88,"context_line":"               help\u003d\u0027Path to the rootwrap configuration file to use for \u0027"}],"source_content_type":"text/x-python","patch_set":36,"id":"9a41bdd9_e9043f7f","line":85,"updated":"2015-07-13 16:38:06.000000000","message":"Yes, this is already a lot better. Sorry that I missed to make a suggestion the last time. I would suggest this:\n\n    Some commands which get executed by the nova-compute service have\n    to be executed as root. This is usually done via a rootwrap for \n    each call. This can result in a performance penalty. If this option\n    is enabled, it is possible to use a deamon which mitigates the  \n    penalty. The default value honors backwards compatibility before\n    the \u0027Liberty\u0027 release.","commit_id":"3fb037ccd828eb23ed9a55e79b06aabd67cfc53d"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"be8f7661f93e93fe71d24fd8964e8f83250e0ceb","unresolved":false,"context_lines":[{"line_number":249,"context_line":""},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"def _get_root_helper():"},{"line_number":252,"context_line":"    if CONF.workarounds.disable_rootwrap:"},{"line_number":253,"context_line":"        cmd \u003d \u0027sudo\u0027"},{"line_number":254,"context_line":"    else:"},{"line_number":255,"context_line":"        cmd \u003d \u0027sudo nova-rootwrap %s\u0027 % CONF.rootwrap_config"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa1b9901_c11d9f74","line":252,"updated":"2015-08-18 11:45:28.000000000","message":"should that not also disable the rootwrap deamon?","commit_id":"df1b1e3a666c4d6a7a3c446c324b08172b244354"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"fd5839a9aef6b733b54f7e1b5e3121ea6c55f700","unresolved":false,"context_lines":[{"line_number":249,"context_line":""},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"def _get_root_helper():"},{"line_number":252,"context_line":"    if CONF.workarounds.disable_rootwrap:"},{"line_number":253,"context_line":"        cmd \u003d \u0027sudo\u0027"},{"line_number":254,"context_line":"    else:"},{"line_number":255,"context_line":"        cmd \u003d \u0027sudo nova-rootwrap %s\u0027 % CONF.rootwrap_config"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa1b9901_e208d32f","line":252,"in_reply_to":"fa1b9901_baa0d4f5","updated":"2015-08-18 16:49:20.000000000","message":"OK, seems a bit odd, but OK.","commit_id":"df1b1e3a666c4d6a7a3c446c324b08172b244354"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"301ed7c5206d8d43aa0ed4e8c2e6eddb262d3f58","unresolved":false,"context_lines":[{"line_number":249,"context_line":""},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"def _get_root_helper():"},{"line_number":252,"context_line":"    if CONF.workarounds.disable_rootwrap:"},{"line_number":253,"context_line":"        cmd \u003d \u0027sudo\u0027"},{"line_number":254,"context_line":"    else:"},{"line_number":255,"context_line":"        cmd \u003d \u0027sudo nova-rootwrap %s\u0027 % CONF.rootwrap_config"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa1b9901_baa0d4f5","line":252,"in_reply_to":"fa1b9901_c11d9f74","updated":"2015-08-18 12:35:06.000000000","message":"So, disable_rootwrap means use sudo directly, when disable_rootwrap is false then we have a choice of either rootwrap directly or via daemon. so this logic is correct as we use nova-rootwrap command line in either case.","commit_id":"df1b1e3a666c4d6a7a3c446c324b08172b244354"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"be8f7661f93e93fe71d24fd8964e8f83250e0ceb","unresolved":false,"context_lines":[{"line_number":267,"context_line":"    def trycmd(self, *args, **kwargs):"},{"line_number":268,"context_line":"        discard_warnings \u003d kwargs.pop(\u0027discard_warnings\u0027, False)"},{"line_number":269,"context_line":"        try:"},{"line_number":270,"context_line":"            out, err \u003d self.execute(*args, **kwargs)"},{"line_number":271,"context_line":"            failed \u003d False"},{"line_number":272,"context_line":"        except processutils.ProcessExecutionError as exn:"},{"line_number":273,"context_line":"            out, err \u003d \u0027\u0027, six.text_type(exn)"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa1b9901_a42679b8","line":270,"updated":"2015-08-18 11:45:28.000000000","message":"Why does this not use processutils.trycmd like it did before?\n\nFeels like this is only needed in the RootwrapDaemonHelper class?","commit_id":"df1b1e3a666c4d6a7a3c446c324b08172b244354"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"fd5839a9aef6b733b54f7e1b5e3121ea6c55f700","unresolved":false,"context_lines":[{"line_number":267,"context_line":"    def trycmd(self, *args, **kwargs):"},{"line_number":268,"context_line":"        discard_warnings \u003d kwargs.pop(\u0027discard_warnings\u0027, False)"},{"line_number":269,"context_line":"        try:"},{"line_number":270,"context_line":"            out, err \u003d self.execute(*args, **kwargs)"},{"line_number":271,"context_line":"            failed \u003d False"},{"line_number":272,"context_line":"        except processutils.ProcessExecutionError as exn:"},{"line_number":273,"context_line":"            out, err \u003d \u0027\u0027, six.text_type(exn)"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa1b9901_1d21ac9f","line":270,"in_reply_to":"fa1b9901_5a6df802","updated":"2015-08-18 16:49:20.000000000","message":"sure, I get that, but why not keep the old behavior for the RootwrapProcessHelper, and just add this code to RootwrapDaemonHelper?","commit_id":"df1b1e3a666c4d6a7a3c446c324b08172b244354"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"301ed7c5206d8d43aa0ed4e8c2e6eddb262d3f58","unresolved":false,"context_lines":[{"line_number":267,"context_line":"    def trycmd(self, *args, **kwargs):"},{"line_number":268,"context_line":"        discard_warnings \u003d kwargs.pop(\u0027discard_warnings\u0027, False)"},{"line_number":269,"context_line":"        try:"},{"line_number":270,"context_line":"            out, err \u003d self.execute(*args, **kwargs)"},{"line_number":271,"context_line":"            failed \u003d False"},{"line_number":272,"context_line":"        except processutils.ProcessExecutionError as exn:"},{"line_number":273,"context_line":"            out, err \u003d \u0027\u0027, six.text_type(exn)"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa1b9901_5a6df802","line":270,"in_reply_to":"fa1b9901_a42679b8","updated":"2015-08-18 12:35:06.000000000","message":"When we switch rootwrap daemon on and existing code calls trycmd, we would like it to call the execute method of the RootwrapDaemonHelper","commit_id":"df1b1e3a666c4d6a7a3c446c324b08172b244354"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"be8f7661f93e93fe71d24fd8964e8f83250e0ceb","unresolved":false,"context_lines":[{"line_number":282,"context_line":"        return processutils.execute(*cmd, **kwargs)"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"class RootwrapDaemonHelper(RootwrapProcessHelper):"},{"line_number":286,"context_line":"    _clients \u003d {}"},{"line_number":287,"context_line":"    _clients_mutex \u003d threading.Lock()"},{"line_number":288,"context_line":""}],"source_content_type":"text/x-python","patch_set":48,"id":"fa1b9901_0432c5f1","line":285,"updated":"2015-08-18 11:45:28.000000000","message":"This seems quite complex, feels like it would be handy if this was in common code eventually, but I guess its OK for now.","commit_id":"df1b1e3a666c4d6a7a3c446c324b08172b244354"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"301ed7c5206d8d43aa0ed4e8c2e6eddb262d3f58","unresolved":false,"context_lines":[{"line_number":282,"context_line":"        return processutils.execute(*cmd, **kwargs)"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"class RootwrapDaemonHelper(RootwrapProcessHelper):"},{"line_number":286,"context_line":"    _clients \u003d {}"},{"line_number":287,"context_line":"    _clients_mutex \u003d threading.Lock()"},{"line_number":288,"context_line":""}],"source_content_type":"text/x-python","patch_set":48,"id":"fa1b9901_9a4ab082","line":285,"in_reply_to":"fa1b9901_0432c5f1","updated":"2015-08-18 12:35:06.000000000","message":"right the more we learn with daemon adoption more we can move to oslo.rootwrap","commit_id":"df1b1e3a666c4d6a7a3c446c324b08172b244354"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"be8f7661f93e93fe71d24fd8964e8f83250e0ceb","unresolved":false,"context_lines":[{"line_number":288,"context_line":""},{"line_number":289,"context_line":"    @classmethod"},{"line_number":290,"context_line":"    def _get_client(cls, rootwrap_config):"},{"line_number":291,"context_line":"        with cls._clients_mutex:"},{"line_number":292,"context_line":"            try:"},{"line_number":293,"context_line":"                return cls._clients[rootwrap_config]"},{"line_number":294,"context_line":"            except KeyError:"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa1b9901_a4a199ab","line":291,"updated":"2015-08-18 11:45:28.000000000","message":"can\u0027t we just use our usual nova synchronised decorator here?","commit_id":"df1b1e3a666c4d6a7a3c446c324b08172b244354"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"301ed7c5206d8d43aa0ed4e8c2e6eddb262d3f58","unresolved":false,"context_lines":[{"line_number":288,"context_line":""},{"line_number":289,"context_line":"    @classmethod"},{"line_number":290,"context_line":"    def _get_client(cls, rootwrap_config):"},{"line_number":291,"context_line":"        with cls._clients_mutex:"},{"line_number":292,"context_line":"            try:"},{"line_number":293,"context_line":"                return cls._clients[rootwrap_config]"},{"line_number":294,"context_line":"            except KeyError:"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa1b9901_da06e81f","line":291,"in_reply_to":"fa1b9901_a4a199ab","updated":"2015-08-18 12:35:06.000000000","message":"yes, but see above ^^ (eventually we should be taking what we learn here to oslo.rootwrap)","commit_id":"df1b1e3a666c4d6a7a3c446c324b08172b244354"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"fd5839a9aef6b733b54f7e1b5e3121ea6c55f700","unresolved":false,"context_lines":[{"line_number":288,"context_line":""},{"line_number":289,"context_line":"    @classmethod"},{"line_number":290,"context_line":"    def _get_client(cls, rootwrap_config):"},{"line_number":291,"context_line":"        with cls._clients_mutex:"},{"line_number":292,"context_line":"            try:"},{"line_number":293,"context_line":"                return cls._clients[rootwrap_config]"},{"line_number":294,"context_line":"            except KeyError:"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa1b9901_5d75e495","line":291,"in_reply_to":"fa1b9901_da06e81f","updated":"2015-08-18 16:49:20.000000000","message":"But now Nova has to maintain two methods, that could be fixed as part of the olso move.","commit_id":"df1b1e3a666c4d6a7a3c446c324b08172b244354"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"be8f7661f93e93fe71d24fd8964e8f83250e0ceb","unresolved":false,"context_lines":[{"line_number":293,"context_line":"                return cls._clients[rootwrap_config]"},{"line_number":294,"context_line":"            except KeyError:"},{"line_number":295,"context_line":"                from oslo_rootwrap import client"},{"line_number":296,"context_line":"                new_client \u003d client.Client(["},{"line_number":297,"context_line":"                    \"sudo\", \"nova-rootwrap-daemon\", rootwrap_config])"},{"line_number":298,"context_line":"                cls._clients[rootwrap_config] \u003d new_client"},{"line_number":299,"context_line":"                return new_client"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa1b9901_247d891c","line":296,"updated":"2015-08-18 11:45:28.000000000","message":"why do we need to bother with all this here? why not create a new client each time? Why is there not a client factory to deal with all this in oslo?\n\nMaybe keeping a reference to this whole class would be better?","commit_id":"df1b1e3a666c4d6a7a3c446c324b08172b244354"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"301ed7c5206d8d43aa0ed4e8c2e6eddb262d3f58","unresolved":false,"context_lines":[{"line_number":293,"context_line":"                return cls._clients[rootwrap_config]"},{"line_number":294,"context_line":"            except KeyError:"},{"line_number":295,"context_line":"                from oslo_rootwrap import client"},{"line_number":296,"context_line":"                new_client \u003d client.Client(["},{"line_number":297,"context_line":"                    \"sudo\", \"nova-rootwrap-daemon\", rootwrap_config])"},{"line_number":298,"context_line":"                cls._clients[rootwrap_config] \u003d new_client"},{"line_number":299,"context_line":"                return new_client"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa1b9901_b52b0582","line":296,"in_reply_to":"fa1b9901_247d891c","updated":"2015-08-18 12:35:06.000000000","message":"agree, we should move some of this to oslo.rootwrap eventually.","commit_id":"df1b1e3a666c4d6a7a3c446c324b08172b244354"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"fd5839a9aef6b733b54f7e1b5e3121ea6c55f700","unresolved":false,"context_lines":[{"line_number":293,"context_line":"                return cls._clients[rootwrap_config]"},{"line_number":294,"context_line":"            except KeyError:"},{"line_number":295,"context_line":"                from oslo_rootwrap import client"},{"line_number":296,"context_line":"                new_client \u003d client.Client(["},{"line_number":297,"context_line":"                    \"sudo\", \"nova-rootwrap-daemon\", rootwrap_config])"},{"line_number":298,"context_line":"                cls._clients[rootwrap_config] \u003d new_client"},{"line_number":299,"context_line":"                return new_client"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa1b9901_1da68c10","line":296,"in_reply_to":"fa1b9901_b52b0582","updated":"2015-08-18 16:49:20.000000000","message":"But why do we need this? Can\u0027t we just create a new one each time?","commit_id":"df1b1e3a666c4d6a7a3c446c324b08172b244354"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"be8f7661f93e93fe71d24fd8964e8f83250e0ceb","unresolved":false,"context_lines":[{"line_number":301,"context_line":"    def __init__(self, rootwrap_config):"},{"line_number":302,"context_line":"        self.client \u003d self._get_client(rootwrap_config)"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"    def execute(self, *cmd, **kwargs):"},{"line_number":305,"context_line":"        cmd \u003d [str(c) for c in cmd]"},{"line_number":306,"context_line":"        loglevel \u003d kwargs.pop(\u0027loglevel\u0027, std_logging.DEBUG)"},{"line_number":307,"context_line":"        log_errors \u003d kwargs.pop(\u0027log_errors\u0027, None)"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa1b9901_040165ae","line":304,"updated":"2015-08-18 11:45:28.000000000","message":"Might be worth a comment saying this is here to honor the processutils.execute interface.\n\nJust not sure why thats not part of the oslo client, I guess. That or we should remove our users of retry in our code, I guess?","commit_id":"df1b1e3a666c4d6a7a3c446c324b08172b244354"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"301ed7c5206d8d43aa0ed4e8c2e6eddb262d3f58","unresolved":false,"context_lines":[{"line_number":301,"context_line":"    def __init__(self, rootwrap_config):"},{"line_number":302,"context_line":"        self.client \u003d self._get_client(rootwrap_config)"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"    def execute(self, *cmd, **kwargs):"},{"line_number":305,"context_line":"        cmd \u003d [str(c) for c in cmd]"},{"line_number":306,"context_line":"        loglevel \u003d kwargs.pop(\u0027loglevel\u0027, std_logging.DEBUG)"},{"line_number":307,"context_line":"        log_errors \u003d kwargs.pop(\u0027log_errors\u0027, None)"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa1b9901_fac5ec5a","line":304,"in_reply_to":"fa1b9901_040165ae","updated":"2015-08-18 12:35:06.000000000","message":"Ack will add a comment. we want either daemon or direct rootwrap to honor the same set of flags in kwargs essentially.","commit_id":"df1b1e3a666c4d6a7a3c446c324b08172b244354"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"fd5839a9aef6b733b54f7e1b5e3121ea6c55f700","unresolved":false,"context_lines":[{"line_number":301,"context_line":"    def __init__(self, rootwrap_config):"},{"line_number":302,"context_line":"        self.client \u003d self._get_client(rootwrap_config)"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"    def execute(self, *cmd, **kwargs):"},{"line_number":305,"context_line":"        cmd \u003d [str(c) for c in cmd]"},{"line_number":306,"context_line":"        loglevel \u003d kwargs.pop(\u0027loglevel\u0027, std_logging.DEBUG)"},{"line_number":307,"context_line":"        log_errors \u003d kwargs.pop(\u0027log_errors\u0027, None)"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa1b9901_3d803046","line":304,"in_reply_to":"fa1b9901_fac5ec5a","updated":"2015-08-18 16:49:20.000000000","message":"yeah, I get that after reading through the commit message, was a bit of mystery to me without that hint.","commit_id":"df1b1e3a666c4d6a7a3c446c324b08172b244354"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"be8f7661f93e93fe71d24fd8964e8f83250e0ceb","unresolved":false,"context_lines":[{"line_number":378,"context_line":"    \"\"\"Convenience wrapper around oslo\u0027s execute() method.\"\"\""},{"line_number":379,"context_line":"    if \u0027run_as_root\u0027 in kwargs and kwargs.get(\u0027run_as_root\u0027):"},{"line_number":380,"context_line":"        if CONF.use_rootwrap_daemon:"},{"line_number":381,"context_line":"            return RootwrapDaemonHelper(CONF.rootwrap_config).execute("},{"line_number":382,"context_line":"                *cmd, **kwargs)"},{"line_number":383,"context_line":"        else:"},{"line_number":384,"context_line":"            return RootwrapProcessHelper().execute(*cmd, **kwargs)"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa1b9901_010157ae","line":381,"updated":"2015-08-18 11:45:28.000000000","message":"I was thinking we should just keeping a reference to these, rather than creating objects every time, but thats probably silly, and this is better for config reloads and things.","commit_id":"df1b1e3a666c4d6a7a3c446c324b08172b244354"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"301ed7c5206d8d43aa0ed4e8c2e6eddb262d3f58","unresolved":false,"context_lines":[{"line_number":378,"context_line":"    \"\"\"Convenience wrapper around oslo\u0027s execute() method.\"\"\""},{"line_number":379,"context_line":"    if \u0027run_as_root\u0027 in kwargs and kwargs.get(\u0027run_as_root\u0027):"},{"line_number":380,"context_line":"        if CONF.use_rootwrap_daemon:"},{"line_number":381,"context_line":"            return RootwrapDaemonHelper(CONF.rootwrap_config).execute("},{"line_number":382,"context_line":"                *cmd, **kwargs)"},{"line_number":383,"context_line":"        else:"},{"line_number":384,"context_line":"            return RootwrapProcessHelper().execute(*cmd, **kwargs)"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa1b9901_fa52ccdc","line":381,"in_reply_to":"fa1b9901_010157ae","updated":"2015-08-18 12:35:06.000000000","message":"right.","commit_id":"df1b1e3a666c4d6a7a3c446c324b08172b244354"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"a26d0f6d6795a2c3606bbe2daed6c2791b778886","unresolved":false,"context_lines":[{"line_number":247,"context_line":"    return (identifier \u003d\u003d 0x40 and client_sess \u003d\u003d session_id)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"def get_root_helper():"},{"line_number":251,"context_line":"    if CONF.workarounds.disable_rootwrap:"},{"line_number":252,"context_line":"        cmd \u003d \u0027sudo\u0027"},{"line_number":253,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":49,"id":"fa1b9901_8bd94bf5","line":250,"updated":"2015-08-21 13:02:39.000000000","message":"Nit: tempting to move this as a method of RootwrapProcessHelper, but I do like the diff more with this staying here.","commit_id":"43e7de419d5138938d992289611bce6084b21114"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"1a4413811459af302f6d5cb4685481af95b8633a","unresolved":false,"context_lines":[{"line_number":247,"context_line":"    return (identifier \u003d\u003d 0x40 and client_sess \u003d\u003d session_id)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"def get_root_helper():"},{"line_number":251,"context_line":"    if CONF.workarounds.disable_rootwrap:"},{"line_number":252,"context_line":"        cmd \u003d \u0027sudo\u0027"},{"line_number":253,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":49,"id":"fa1b9901_d6428a0e","line":250,"in_reply_to":"fa1b9901_8bd94bf5","updated":"2015-08-21 13:06:28.000000000","message":"John, this is used by other modules, it was just changed from _get_root_helper() to get_root_helper in the previous commit.","commit_id":"43e7de419d5138938d992289611bce6084b21114"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ee12ec37333d02460572245c55d89e2c1d546a87","unresolved":false,"context_lines":[{"line_number":247,"context_line":"    return (identifier \u003d\u003d 0x40 and client_sess \u003d\u003d session_id)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"def get_root_helper():"},{"line_number":251,"context_line":"    if CONF.workarounds.disable_rootwrap:"},{"line_number":252,"context_line":"        cmd \u003d \u0027sudo\u0027"},{"line_number":253,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":49,"id":"fa1b9901_56ebdadb","line":250,"in_reply_to":"fa1b9901_d6428a0e","updated":"2015-08-21 13:07:22.000000000","message":"ah, even better reason for it to stay here, OK.","commit_id":"43e7de419d5138938d992289611bce6084b21114"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"a26d0f6d6795a2c3606bbe2daed6c2791b778886","unresolved":false,"context_lines":[{"line_number":265,"context_line":"class RootwrapProcessHelper(object):"},{"line_number":266,"context_line":"    def trycmd(self, *cmd, **kwargs):"},{"line_number":267,"context_line":"        kwargs[\u0027root_helper\u0027] \u003d get_root_helper()"},{"line_number":268,"context_line":"        return processutils.trycmd(*cmd, **kwargs)"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    def execute(self, *cmd, **kwargs):"},{"line_number":271,"context_line":"        kwargs[\u0027root_helper\u0027] \u003d get_root_helper()"}],"source_content_type":"text/x-python","patch_set":49,"id":"fa1b9901_6b2957e5","line":268,"updated":"2015-08-21 13:02:39.000000000","message":"sorry to be a pendent, but this looks much better to my eyes.","commit_id":"43e7de419d5138938d992289611bce6084b21114"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"1a4413811459af302f6d5cb4685481af95b8633a","unresolved":false,"context_lines":[{"line_number":265,"context_line":"class RootwrapProcessHelper(object):"},{"line_number":266,"context_line":"    def trycmd(self, *cmd, **kwargs):"},{"line_number":267,"context_line":"        kwargs[\u0027root_helper\u0027] \u003d get_root_helper()"},{"line_number":268,"context_line":"        return processutils.trycmd(*cmd, **kwargs)"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    def execute(self, *cmd, **kwargs):"},{"line_number":271,"context_line":"        kwargs[\u0027root_helper\u0027] \u003d get_root_helper()"}],"source_content_type":"text/x-python","patch_set":49,"id":"fa1b9901_365426c8","line":268,"in_reply_to":"fa1b9901_6b2957e5","updated":"2015-08-21 13:06:28.000000000","message":"no worries. i agree.","commit_id":"43e7de419d5138938d992289611bce6084b21114"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"a26d0f6d6795a2c3606bbe2daed6c2791b778886","unresolved":false,"context_lines":[{"line_number":319,"context_line":"            ignore_exit_code \u003d not check_exit_code"},{"line_number":320,"context_line":"            check_exit_code \u003d [0]"},{"line_number":321,"context_line":"        elif isinstance(check_exit_code, int):"},{"line_number":322,"context_line":"            check_exit_code \u003d [check_exit_code]"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        sanitized_cmd \u003d strutils.mask_password(\u0027 \u0027.join(cmd))"},{"line_number":325,"context_line":"        LOG.info(_LI(\u0027Executing RootwrapDaemonHelper.execute \u0027"}],"source_content_type":"text/x-python","patch_set":49,"id":"fa1b9901_8b0aab24","line":322,"updated":"2015-08-21 13:02:39.000000000","message":"I am not seeing unit test coverage for all of these options here.","commit_id":"43e7de419d5138938d992289611bce6084b21114"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"f645c64667f03280c337ac13a37c52c28c88dad4","unresolved":false,"context_lines":[{"line_number":319,"context_line":"            ignore_exit_code \u003d not check_exit_code"},{"line_number":320,"context_line":"            check_exit_code \u003d [0]"},{"line_number":321,"context_line":"        elif isinstance(check_exit_code, int):"},{"line_number":322,"context_line":"            check_exit_code \u003d [check_exit_code]"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        sanitized_cmd \u003d strutils.mask_password(\u0027 \u0027.join(cmd))"},{"line_number":325,"context_line":"        LOG.info(_LI(\u0027Executing RootwrapDaemonHelper.execute \u0027"}],"source_content_type":"text/x-python","patch_set":49,"id":"fa1b9901_b6a91692","line":322,"in_reply_to":"fa1b9901_8b0aab24","updated":"2015-08-21 13:07:57.000000000","message":"will add something that checks all those process utils args get passed through","commit_id":"43e7de419d5138938d992289611bce6084b21114"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"a26d0f6d6795a2c3606bbe2daed6c2791b778886","unresolved":false,"context_lines":[{"line_number":326,"context_line":"                     \u0027cmd\u003d[%(cmd)r] kwargs\u003d[%(kwargs)r]\u0027),"},{"line_number":327,"context_line":"                 {\u0027cmd\u0027: sanitized_cmd, \u0027kwargs\u0027: kwargs})"},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"        while attempts \u003e 0:"},{"line_number":330,"context_line":"            attempts -\u003d 1"},{"line_number":331,"context_line":"            try:"},{"line_number":332,"context_line":"                start_time \u003d time.time()"}],"source_content_type":"text/x-python","patch_set":49,"id":"fa1b9901_8b238bbc","line":329,"updated":"2015-08-21 13:02:39.000000000","message":"I can\u0027t see a unit test for this retry loop.","commit_id":"43e7de419d5138938d992289611bce6084b21114"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"1a4413811459af302f6d5cb4685481af95b8633a","unresolved":false,"context_lines":[{"line_number":326,"context_line":"                     \u0027cmd\u003d[%(cmd)r] kwargs\u003d[%(kwargs)r]\u0027),"},{"line_number":327,"context_line":"                 {\u0027cmd\u0027: sanitized_cmd, \u0027kwargs\u0027: kwargs})"},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"        while attempts \u003e 0:"},{"line_number":330,"context_line":"            attempts -\u003d 1"},{"line_number":331,"context_line":"            try:"},{"line_number":332,"context_line":"                start_time \u003d time.time()"}],"source_content_type":"text/x-python","patch_set":49,"id":"fa1b9901_163242a7","line":329,"in_reply_to":"fa1b9901_8b238bbc","updated":"2015-08-21 13:06:28.000000000","message":"ack. will add an assert for the retry loop","commit_id":"43e7de419d5138938d992289611bce6084b21114"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"a26d0f6d6795a2c3606bbe2daed6c2791b778886","unresolved":false,"context_lines":[{"line_number":354,"context_line":"                        cmd\u003dsanitized_cmd)"},{"line_number":355,"context_line":"                return (out, err)"},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"            except processutils.ProcessExecutionError as err:"},{"line_number":358,"context_line":"                # if we want to always log the errors or if this is"},{"line_number":359,"context_line":"                # the final attempt that failed and we want to log that."},{"line_number":360,"context_line":"                if log_errors \u003d\u003d processutils.LOG_ALL_ERRORS or ("}],"source_content_type":"text/x-python","patch_set":49,"id":"fa1b9901_56175a44","line":357,"updated":"2015-08-21 13:02:39.000000000","message":"I see most of this appears to be covered by unit tests, which is great.","commit_id":"43e7de419d5138938d992289611bce6084b21114"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"1a4413811459af302f6d5cb4685481af95b8633a","unresolved":false,"context_lines":[{"line_number":354,"context_line":"                        cmd\u003dsanitized_cmd)"},{"line_number":355,"context_line":"                return (out, err)"},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"            except processutils.ProcessExecutionError as err:"},{"line_number":358,"context_line":"                # if we want to always log the errors or if this is"},{"line_number":359,"context_line":"                # the final attempt that failed and we want to log that."},{"line_number":360,"context_line":"                if log_errors \u003d\u003d processutils.LOG_ALL_ERRORS or ("}],"source_content_type":"text/x-python","patch_set":49,"id":"fa1b9901_166482b6","line":357,"in_reply_to":"fa1b9901_56175a44","updated":"2015-08-21 13:06:28.000000000","message":"thanks","commit_id":"43e7de419d5138938d992289611bce6084b21114"}]}