)]}'
{"neutron/agent/common/config.py":[{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"7d262f77e12be8c7a41a6fbb0039c3b76f9008ee","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    conf.register_opts(USE_NAMESPACES_OPTS)"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"def get_root_helper(conf, legacy\u003dTrue):"},{"line_number":98,"context_line":"    from neutron.agent.linux import utils"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    root_helper \u003d conf.AGENT.root_helper"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F95I0%3D","line":97,"updated":"2014-04-01 11:53:22.000000000","message":"What\u0027s \"legacy\"? Could you add a doc string explaining what this param does?","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"61342d1c6eb900bc96b3061a17d06ec28892fa54","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    conf.register_opts(USE_NAMESPACES_OPTS)"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"def get_root_helper(conf, legacy\u003dTrue):"},{"line_number":98,"context_line":"    from neutron.agent.linux import utils"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    root_helper \u003d conf.AGENT.root_helper"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F9w5w%3D","line":97,"in_reply_to":"AAAAWH%2F93CQ%3D","updated":"2014-04-02 10:44:48.000000000","message":"Done","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"1461b27155a7c204a9d72f5aaa55490583e768a3","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    conf.register_opts(USE_NAMESPACES_OPTS)"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"def get_root_helper(conf, legacy\u003dTrue):"},{"line_number":98,"context_line":"    from neutron.agent.linux import utils"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    root_helper \u003d conf.AGENT.root_helper"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F93CQ%3D","line":97,"in_reply_to":"AAAAWH%2F95I0%3D","updated":"2014-04-01 16:05:18.000000000","message":"Will do.","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"7d262f77e12be8c7a41a6fbb0039c3b76f9008ee","unresolved":false,"context_lines":[{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"def get_root_helper(conf, legacy\u003dTrue):"},{"line_number":98,"context_line":"    from neutron.agent.linux import utils"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    root_helper \u003d conf.AGENT.root_helper"},{"line_number":101,"context_line":"    if legacy and root_helper \u003d\u003d \u0027sudo\u0027:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F95a0%3D","line":98,"updated":"2014-04-01 11:53:22.000000000","message":"why is the import here?","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"61342d1c6eb900bc96b3061a17d06ec28892fa54","unresolved":false,"context_lines":[{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"def get_root_helper(conf, legacy\u003dTrue):"},{"line_number":98,"context_line":"    from neutron.agent.linux import utils"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    root_helper \u003d conf.AGENT.root_helper"},{"line_number":101,"context_line":"    if legacy and root_helper \u003d\u003d \u0027sudo\u0027:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F9w5s%3D","line":98,"in_reply_to":"AAAAWH%2F93CA%3D","updated":"2014-04-02 10:44:48.000000000","message":"Done","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"1461b27155a7c204a9d72f5aaa55490583e768a3","unresolved":false,"context_lines":[{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"def get_root_helper(conf, legacy\u003dTrue):"},{"line_number":98,"context_line":"    from neutron.agent.linux import utils"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    root_helper \u003d conf.AGENT.root_helper"},{"line_number":101,"context_line":"    if legacy and root_helper \u003d\u003d \u0027sudo\u0027:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F93CA%3D","line":98,"in_reply_to":"AAAAWH%2F95a0%3D","updated":"2014-04-01 16:05:18.000000000","message":"To not require linux-specific module unless it is needed. I\u0027ll add a comment here.","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"130ee1c391712c705695bc869d9d472007bec44d","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    conf.register_opts(USE_NAMESPACES_OPTS)"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"def get_root_helper(conf, legacy\u003dTrue):"},{"line_number":98,"context_line":"    \"\"\"Returns appropriate root helper object based on parameters in config."},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    :param conf: config object"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAWH%2F9jQc%3D","line":97,"updated":"2014-04-03 20:16:52.000000000","message":"Consider changing the default of legacy to False.  The reason I say that is so that callers that wish to get the deprecated legacy behavior need to pass this option instead of the callers that want the new behavior.  It will serve as a reminder that it should be removed when the deprecated behavior is removed.","commit_id":"7850a99bac69329275a60780e5b76172d1721342"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"01b9242cb2e680fb16e18e253eb094d2c0b0aeeb","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    conf.register_opts(USE_NAMESPACES_OPTS)"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"def get_root_helper(conf, legacy\u003dTrue):"},{"line_number":98,"context_line":"    \"\"\"Returns appropriate root helper object based on parameters in config."},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    :param conf: config object"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAWH%2F9iIE%3D","line":97,"in_reply_to":"AAAAWH%2F9jQc%3D","updated":"2014-04-03 21:45:43.000000000","message":"I thought about that. It\u0027d require changes in way more places than I introduce call to get_root_helper to. I also will require to be changed back all over the place once deprecation period is finished. Are you sure that\u0027s not a bad approach?","commit_id":"7850a99bac69329275a60780e5b76172d1721342"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"130ee1c391712c705695bc869d9d472007bec44d","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        with deprecation warning in logs (defaults to True)"},{"line_number":103,"context_line":"    \"\"\""},{"line_number":104,"context_line":"    # Import linux-specific module here to support non-linux agents"},{"line_number":105,"context_line":"    from neutron.agent.linux import utils"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    root_helper \u003d conf.AGENT.root_helper"},{"line_number":108,"context_line":"    if legacy and root_helper \u003d\u003d \u0027sudo\u0027:"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAWH%2F9jNA%3D","line":105,"updated":"2014-04-03 20:16:52.000000000","message":"I\u0027m not sure I\u0027m comfortable with this.  get_root_helper was not strictly linux-only before this.  Adding this import makes it officially linux-only.\n\nAre there other platforms (e.g. bsd) which might\u0027ve used get_root_helper before that need to be considered?  I really don\u0027t know because I only work with OpenStack on the Linux platform.  So, I\u0027m posing the question.\n\nIs there a better way to handle platform differences?","commit_id":"7850a99bac69329275a60780e5b76172d1721342"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"01b9242cb2e680fb16e18e253eb094d2c0b0aeeb","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        with deprecation warning in logs (defaults to True)"},{"line_number":103,"context_line":"    \"\"\""},{"line_number":104,"context_line":"    # Import linux-specific module here to support non-linux agents"},{"line_number":105,"context_line":"    from neutron.agent.linux import utils"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    root_helper \u003d conf.AGENT.root_helper"},{"line_number":108,"context_line":"    if legacy and root_helper \u003d\u003d \u0027sudo\u0027:"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAWH%2F9iG8%3D","line":105,"in_reply_to":"AAAAWH%2F9jNA%3D","updated":"2014-04-03 21:45:43.000000000","message":"All that root_helper stuff is actually (almost) platform-agnostic. But the methods that use root_helper (create_process and execute) are defined in that module anyway, so caller will have to be Linux.\n\nI think it deserves some refactoring (moving common code out of linux package) but it\u0027s a matter for another change request.","commit_id":"7850a99bac69329275a60780e5b76172d1721342"},{"author":{"_account_id":2592,"name":"Mark McClain","email":"mark@mcclain.xyz","username":"markmcclain"},"change_message_id":"eaf4edcee1cdacc85c0c3e4c8035dc6e01b8cbeb","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        with deprecation warning in logs (defaults to True)"},{"line_number":103,"context_line":"    \"\"\""},{"line_number":104,"context_line":"    # Import linux-specific module here to support non-linux agents"},{"line_number":105,"context_line":"    from neutron.agent.linux import utils"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    root_helper \u003d conf.AGENT.root_helper"},{"line_number":108,"context_line":"    if legacy and root_helper \u003d\u003d \u0027sudo\u0027:"}],"source_content_type":"text/x-python","patch_set":20,"id":"1ae5cdf2_aa409307","line":105,"updated":"2014-06-05 17:48:51.000000000","message":"why an internal import? if it is to avoid loops, why not just move this method?","commit_id":"a1b7aceeb539003053627b30e72e5cdf60d8a4c7"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"18f01eb1c3dd66ae4ffe4c3e0c41086a0f66e198","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        with deprecation warning in logs (defaults to True)"},{"line_number":101,"context_line":"    \"\"\""},{"line_number":102,"context_line":"    # Import linux-specific module here to support non-linux agents"},{"line_number":103,"context_line":"    from neutron.agent.linux import utils"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    root_helper \u003d conf.AGENT.root_helper"},{"line_number":106,"context_line":"    if legacy and root_helper \u003d\u003d \u0027sudo\u0027:"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_ce3cdcc3","line":103,"updated":"2014-07-29 10:09:39.000000000","message":"why local import?","commit_id":"1a814b813624a8278e7bc4547e7500da10a3ade8"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"031200c4405c56081db8feee99d8a56a5b8ea7d2","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        with deprecation warning in logs (defaults to True)"},{"line_number":101,"context_line":"    \"\"\""},{"line_number":102,"context_line":"    # Import linux-specific module here to support non-linux agents"},{"line_number":103,"context_line":"    from neutron.agent.linux import utils"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    root_helper \u003d conf.AGENT.root_helper"},{"line_number":106,"context_line":"    if legacy and root_helper \u003d\u003d \u0027sudo\u0027:"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_04edd3f0","line":103,"in_reply_to":"baada198_642daf45","updated":"2014-07-29 11:17:09.000000000","message":"Ah, indeed.","commit_id":"1a814b813624a8278e7bc4547e7500da10a3ade8"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"0ce33746d5acded54cee5bdc45762ef8fb3c87f5","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        with deprecation warning in logs (defaults to True)"},{"line_number":101,"context_line":"    \"\"\""},{"line_number":102,"context_line":"    # Import linux-specific module here to support non-linux agents"},{"line_number":103,"context_line":"    from neutron.agent.linux import utils"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    root_helper \u003d conf.AGENT.root_helper"},{"line_number":106,"context_line":"    if legacy and root_helper \u003d\u003d \u0027sudo\u0027:"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_c40e7b33","line":103,"in_reply_to":"baada198_a9ffa265","updated":"2014-07-29 11:05:39.000000000","message":"Which OSes do you mean? AFAIK the only platform specific thing in utils.py is \u0027import fcntl\u0027, but it\u0027s avail on all Unices, right? Or do you expect that any agent will run outside Unix env?","commit_id":"1a814b813624a8278e7bc4547e7500da10a3ade8"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"ea0d4f9dde8c7962312f585163496175cf9ebab0","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        with deprecation warning in logs (defaults to True)"},{"line_number":101,"context_line":"    \"\"\""},{"line_number":102,"context_line":"    # Import linux-specific module here to support non-linux agents"},{"line_number":103,"context_line":"    from neutron.agent.linux import utils"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    root_helper \u003d conf.AGENT.root_helper"},{"line_number":106,"context_line":"    if legacy and root_helper \u003d\u003d \u0027sudo\u0027:"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_642daf45","line":103,"in_reply_to":"baada198_c40e7b33","updated":"2014-07-29 11:15:23.000000000","message":"We have Hyper-V agent that does import this module. I don\u0027t think it\u0027ll be happy with anything *nix-specific as it should run on Windows IIUC.","commit_id":"1a814b813624a8278e7bc4547e7500da10a3ade8"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"4ec17284723c58dc09b4d0620f8d8537640cbbe8","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        with deprecation warning in logs (defaults to True)"},{"line_number":101,"context_line":"    \"\"\""},{"line_number":102,"context_line":"    # Import linux-specific module here to support non-linux agents"},{"line_number":103,"context_line":"    from neutron.agent.linux import utils"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    root_helper \u003d conf.AGENT.root_helper"},{"line_number":106,"context_line":"    if legacy and root_helper \u003d\u003d \u0027sudo\u0027:"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_a9ffa265","line":103,"in_reply_to":"baada198_ce3cdcc3","updated":"2014-07-29 10:49:15.000000000","message":"See comment above. We don\u0027t want to import linux-specific modules in common code as it will break on other OSs.","commit_id":"1a814b813624a8278e7bc4547e7500da10a3ade8"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"9e508b1444c8c3733c7c42cecaffd54a2f40a221","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        with deprecation warning in logs (defaults to True)"},{"line_number":101,"context_line":"    \"\"\""},{"line_number":102,"context_line":"    # Import linux-specific module here to support non-linux agents"},{"line_number":103,"context_line":"    from neutron.agent.linux import utils"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    root_helper \u003d conf.AGENT.root_helper"},{"line_number":106,"context_line":"    if legacy and root_helper \u003d\u003d \u0027sudo\u0027:"}],"source_content_type":"text/x-python","patch_set":24,"id":"5a890539_7cd3a5d5","line":103,"updated":"2014-12-05 06:55:01.000000000","message":"actually this module, well, the code this function uses from the module, does not look so linux specific.","commit_id":"43d259600e591d9c90cba69245a81bafec90378a"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"41b933fcc4bce9ec2473e7bb2b95dc0d95520b1c","unresolved":false,"context_lines":[{"line_number":106,"context_line":"    if legacy and root_helper \u003d\u003d \u0027sudo\u0027:"},{"line_number":107,"context_line":"        root_helper \u003d conf.root_helper"},{"line_number":108,"context_line":"        if root_helper !\u003d \u0027sudo\u0027:"},{"line_number":109,"context_line":"            LOG.deprecated(_(\u0027DEFAULT.root_helper is deprecated! Please move \u0027"},{"line_number":110,"context_line":"                             \u0027root_helper configuration to [AGENT] section.\u0027))"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    return utils.get_root_helper_object(root_helper)"}],"source_content_type":"text/x-python","patch_set":24,"id":"3a961159_6d59605a","line":109,"updated":"2015-01-16 14:34:09.000000000","message":"Shouldn\u0027t it be handled by deprecated_group\u003d argument from oslo.config instead?","commit_id":"43d259600e591d9c90cba69245a81bafec90378a"}],"neutron/agent/linux/utils.py":[{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"7d262f77e12be8c7a41a6fbb0039c3b76f9008ee","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        return NoopRootHelper()"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"class NoopRootHelper(object):"},{"line_number":46,"context_line":"    def create_process(self, cmd, env):"},{"line_number":47,"context_line":"        LOG.debug(_(\"Running command: %s\"), cmd)"},{"line_number":48,"context_line":"        obj \u003d utils.subprocess_popen(cmd, shell\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F95Ps%3D","line":45,"updated":"2014-04-01 11:53:22.000000000","message":"The name is confusing. I\u0027d consider renaming it \"ProcessHelper\". \"BasicRootHelper\" could be renamed ad \"BasicRootProcessHelper\"","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"61342d1c6eb900bc96b3061a17d06ec28892fa54","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        return NoopRootHelper()"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"class NoopRootHelper(object):"},{"line_number":46,"context_line":"    def create_process(self, cmd, env):"},{"line_number":47,"context_line":"        LOG.debug(_(\"Running command: %s\"), cmd)"},{"line_number":48,"context_line":"        obj \u003d utils.subprocess_popen(cmd, shell\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F9w58%3D","line":45,"in_reply_to":"AAAAWH%2F92%2B8%3D","updated":"2014-04-02 10:44:48.000000000","message":"Done","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"1461b27155a7c204a9d72f5aaa55490583e768a3","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        return NoopRootHelper()"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"class NoopRootHelper(object):"},{"line_number":46,"context_line":"    def create_process(self, cmd, env):"},{"line_number":47,"context_line":"        LOG.debug(_(\"Running command: %s\"), cmd)"},{"line_number":48,"context_line":"        obj \u003d utils.subprocess_popen(cmd, shell\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F92%2B8%3D","line":45,"in_reply_to":"AAAAWH%2F95Ps%3D","updated":"2014-04-01 16:05:18.000000000","message":"I agree. I\u0027m not sure about \"ProcessHelper\" though. I\u0027ll think about it.","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"7d262f77e12be8c7a41a6fbb0039c3b76f9008ee","unresolved":false,"context_lines":[{"line_number":66,"context_line":"            greenthread.sleep(0)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def __eq__(self, other):"},{"line_number":69,"context_line":"        if isinstance(other, NoopRootHelper):"},{"line_number":70,"context_line":"            return True"},{"line_number":71,"context_line":"        else:"},{"line_number":72,"context_line":"            return NotImplemented"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F95Jg%3D","line":69,"updated":"2014-04-01 11:53:22.000000000","message":"This will return True if other is BasicRootHelper, since it\u0027s a subclass of NoopRootHelper is that OK?","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"61342d1c6eb900bc96b3061a17d06ec28892fa54","unresolved":false,"context_lines":[{"line_number":66,"context_line":"            greenthread.sleep(0)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def __eq__(self, other):"},{"line_number":69,"context_line":"        if isinstance(other, NoopRootHelper):"},{"line_number":70,"context_line":"            return True"},{"line_number":71,"context_line":"        else:"},{"line_number":72,"context_line":"            return NotImplemented"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F9w54%3D","line":69,"in_reply_to":"AAAAWH%2F92%2Bk%3D","updated":"2014-04-02 10:44:48.000000000","message":"Done","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"1461b27155a7c204a9d72f5aaa55490583e768a3","unresolved":false,"context_lines":[{"line_number":66,"context_line":"            greenthread.sleep(0)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def __eq__(self, other):"},{"line_number":69,"context_line":"        if isinstance(other, NoopRootHelper):"},{"line_number":70,"context_line":"            return True"},{"line_number":71,"context_line":"        else:"},{"line_number":72,"context_line":"            return NotImplemented"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F92%2Bk%3D","line":69,"in_reply_to":"AAAAWH%2F95Jg%3D","updated":"2014-04-01 16:05:18.000000000","message":"It\u0027s not a big deal as it\u0027s overridden in the child class anyway. I\u0027ll add more strict check here.","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"7d262f77e12be8c7a41a6fbb0039c3b76f9008ee","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        if isinstance(other, NoopRootHelper):"},{"line_number":70,"context_line":"            return True"},{"line_number":71,"context_line":"        else:"},{"line_number":72,"context_line":"            return NotImplemented"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"class BasicRootHelper(NoopRootHelper):"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F95Qc%3D","line":72,"updated":"2014-04-01 11:53:22.000000000","message":"Why NotImplemented? It\u0027s a comparison, shouldn\u0027t be \"False\"? Is that on purpose? If so, why?\n\nIf the answer to my comments on line 69 and here are both yes\nthen you should consider replacing lines from 69 to 72 with:\n   def __eq__(self, other):\n        return (isinstance(other, self.__class__))\n\nThen you can remove lines 83 up to 87 in the derived class","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"1461b27155a7c204a9d72f5aaa55490583e768a3","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        if isinstance(other, NoopRootHelper):"},{"line_number":70,"context_line":"            return True"},{"line_number":71,"context_line":"        else:"},{"line_number":72,"context_line":"            return NotImplemented"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"class BasicRootHelper(NoopRootHelper):"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F928k%3D","line":72,"in_reply_to":"AAAAWH%2F95Qc%3D","updated":"2014-04-01 16:05:18.000000000","message":"I don\u0027t want these objects to be comparable to any other object. As long as we return NotImplemented here, comparsions like\n\n    NoopRootHelper() \u003d\u003d 42\n\nwould raise an exception instead of silently returning False. This gives less surprising behavior and will allow to spot problems in tests early instead of hiding them.","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"130ee1c391712c705695bc869d9d472007bec44d","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        return ProcessHelper()"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"class ProcessHelper(object):"},{"line_number":46,"context_line":"    def create_process(self, cmd, env):"},{"line_number":47,"context_line":"        LOG.debug(_(\"Running command: %s\"), cmd)"},{"line_number":48,"context_line":"        obj \u003d utils.subprocess_popen(cmd, shell\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAWH%2F9iuc%3D","line":45,"updated":"2014-04-03 20:16:52.000000000","message":"You could think of this class as a case of WrappedProcessHelper where self.root_helper is [].  Inheritance might be overkill.","commit_id":"7850a99bac69329275a60780e5b76172d1721342"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"01b9242cb2e680fb16e18e253eb094d2c0b0aeeb","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        return ProcessHelper()"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"class ProcessHelper(object):"},{"line_number":46,"context_line":"    def create_process(self, cmd, env):"},{"line_number":47,"context_line":"        LOG.debug(_(\"Running command: %s\"), cmd)"},{"line_number":48,"context_line":"        obj \u003d utils.subprocess_popen(cmd, shell\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAWH%2F9h%2Fo%3D","line":45,"in_reply_to":"AAAAWH%2F9iuc%3D","updated":"2014-04-03 21:45:43.000000000","message":"I think although inheritance is an overkill here it provides better readability with almost no overhead.","commit_id":"7850a99bac69329275a60780e5b76172d1721342"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"130ee1c391712c705695bc869d9d472007bec44d","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        if type(self) is ProcessHelper and type(other) is ProcessHelper:"},{"line_number":70,"context_line":"            return True"},{"line_number":71,"context_line":"        else:"},{"line_number":72,"context_line":"            return NotImplemented"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"class WrappedProcessHelper(ProcessHelper):"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAWH%2F9i20%3D","line":72,"updated":"2014-04-03 20:16:52.000000000","message":"See comment on line 87.","commit_id":"7850a99bac69329275a60780e5b76172d1721342"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"130ee1c391712c705695bc869d9d472007bec44d","unresolved":false,"context_lines":[{"line_number":76,"context_line":"    def __init__(self, root_helper):"},{"line_number":77,"context_line":"        self.root_helper \u003d shlex.split(root_helper)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    def create_process(self, cmd, env\u003dNone):"},{"line_number":80,"context_line":"        return super(WrappedProcessHelper, self).create_process("},{"line_number":81,"context_line":"            self.root_helper + cmd, env)"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAWH%2F9iuE%3D","line":79,"updated":"2014-04-03 20:16:52.000000000","message":"Env does not default to None in the parent class.  They should be consistent.","commit_id":"7850a99bac69329275a60780e5b76172d1721342"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"fa84927af75684b932a9e24f4cb2b0247aec16d5","unresolved":false,"context_lines":[{"line_number":76,"context_line":"    def __init__(self, root_helper):"},{"line_number":77,"context_line":"        self.root_helper \u003d shlex.split(root_helper)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    def create_process(self, cmd, env\u003dNone):"},{"line_number":80,"context_line":"        return super(WrappedProcessHelper, self).create_process("},{"line_number":81,"context_line":"            self.root_helper + cmd, env)"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAWH%2F9ROc%3D","line":79,"in_reply_to":"AAAAWH%2F9h%2Fk%3D","updated":"2014-04-07 14:34:57.000000000","message":"Done","commit_id":"7850a99bac69329275a60780e5b76172d1721342"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"01b9242cb2e680fb16e18e253eb094d2c0b0aeeb","unresolved":false,"context_lines":[{"line_number":76,"context_line":"    def __init__(self, root_helper):"},{"line_number":77,"context_line":"        self.root_helper \u003d shlex.split(root_helper)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    def create_process(self, cmd, env\u003dNone):"},{"line_number":80,"context_line":"        return super(WrappedProcessHelper, self).create_process("},{"line_number":81,"context_line":"            self.root_helper + cmd, env)"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAWH%2F9h%2Fk%3D","line":79,"in_reply_to":"AAAAWH%2F9iuE%3D","updated":"2014-04-03 21:45:43.000000000","message":"Will fix that.","commit_id":"7850a99bac69329275a60780e5b76172d1721342"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"130ee1c391712c705695bc869d9d472007bec44d","unresolved":false,"context_lines":[{"line_number":84,"context_line":"        if isinstance(other, WrappedProcessHelper):"},{"line_number":85,"context_line":"            return other.root_helper \u003d\u003d self.root_helper"},{"line_number":86,"context_line":"        else:"},{"line_number":87,"context_line":"            return NotImplemented"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"def create_process(cmd, root_helper\u003dNone, addl_env\u003dNone):"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAWH%2F9i3Q%3D","line":87,"updated":"2014-04-03 20:16:52.000000000","message":"Do you mean to raise NotImplemented rather than return it?  One problem with returning NotImplemented is that it evaluates to True in a boolean context:\n\n\u003e\u003e\u003e bool(NotImplemented) returns True","commit_id":"7850a99bac69329275a60780e5b76172d1721342"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"01b9242cb2e680fb16e18e253eb094d2c0b0aeeb","unresolved":false,"context_lines":[{"line_number":84,"context_line":"        if isinstance(other, WrappedProcessHelper):"},{"line_number":85,"context_line":"            return other.root_helper \u003d\u003d self.root_helper"},{"line_number":86,"context_line":"        else:"},{"line_number":87,"context_line":"            return NotImplemented"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"def create_process(cmd, root_helper\u003dNone, addl_env\u003dNone):"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAWH%2F9h6k%3D","line":87,"in_reply_to":"AAAAWH%2F9i3Q%3D","updated":"2014-04-03 21:45:43.000000000","message":"It has to do with how Python handles such overloaded operators. One should return NotImplemented (not raise NotImplementedError) from comparison (or other) methods to signal that this set of operands is not supported and other approach should be taken.\n\nFrom https://docs.python.org/2.7/reference/datamodel.html#coercion-rules :\n\nFor objects x and y, first x.__op__(y) is tried. If this is not implemented or returns NotImplemented, y.__rop__(x) is tried. If this is also not implemented or returns NotImplemented, a TypeError exception is raised.","commit_id":"7850a99bac69329275a60780e5b76172d1721342"},{"author":{"_account_id":2592,"name":"Mark McClain","email":"mark@mcclain.xyz","username":"markmcclain"},"change_message_id":"eaf4edcee1cdacc85c0c3e4c8035dc6e01b8cbeb","unresolved":false,"context_lines":[{"line_number":66,"context_line":"            greenthread.sleep(0)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def __eq__(self, other):"},{"line_number":69,"context_line":"        if type(self) is ProcessHelper and type(other) is ProcessHelper:"},{"line_number":70,"context_line":"            return True"},{"line_number":71,"context_line":"        else:"},{"line_number":72,"context_line":"            return NotImplemented"}],"source_content_type":"text/x-python","patch_set":20,"id":"1ae5cdf2_4ab8ef10","line":69,"updated":"2014-06-05 17:48:51.000000000","message":"Why do we care about equality?","commit_id":"a1b7aceeb539003053627b30e72e5cdf60d8a4c7"},{"author":{"_account_id":2592,"name":"Mark McClain","email":"mark@mcclain.xyz","username":"markmcclain"},"change_message_id":"eaf4edcee1cdacc85c0c3e4c8035dc6e01b8cbeb","unresolved":false,"context_lines":[{"line_number":80,"context_line":"        return super(WrappedProcessHelper, self).create_process("},{"line_number":81,"context_line":"            self.root_helper + cmd, env)"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    def __eq__(self, other):"},{"line_number":84,"context_line":"        if isinstance(other, WrappedProcessHelper):"},{"line_number":85,"context_line":"            return other.root_helper \u003d\u003d self.root_helper"},{"line_number":86,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":20,"id":"1ae5cdf2_6a3a0b9a","line":83,"updated":"2014-06-05 17:48:51.000000000","message":"same question as above","commit_id":"a1b7aceeb539003053627b30e72e5cdf60d8a4c7"},{"author":{"_account_id":2592,"name":"Mark McClain","email":"mark@mcclain.xyz","username":"markmcclain"},"change_message_id":"eaf4edcee1cdacc85c0c3e4c8035dc6e01b8cbeb","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    list of command arguments used to create it."},{"line_number":95,"context_line":"    \"\"\""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    if root_helper is None:"},{"line_number":98,"context_line":"        root_helper \u003d ProcessHelper()"},{"line_number":99,"context_line":"    cmd \u003d map(str, cmd)"},{"line_number":100,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"1ae5cdf2_8a3d9791","line":97,"updated":"2014-06-05 17:48:51.000000000","message":"if not root_helper:","commit_id":"a1b7aceeb539003053627b30e72e5cdf60d8a4c7"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"528e20f552822d42226a1b4966031025a3e137b7","unresolved":false,"context_lines":[{"line_number":66,"context_line":"            greenthread.sleep(0)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def __eq__(self, other):"},{"line_number":69,"context_line":"        if type(self) is ProcessHelper and type(other) is ProcessHelper:"},{"line_number":70,"context_line":"            return True"},{"line_number":71,"context_line":"        else:"},{"line_number":72,"context_line":"            return NotImplemented"}],"source_content_type":"text/x-python","patch_set":23,"id":"1abeadc6_b3f8a67d","line":69,"updated":"2014-08-19 14:13:43.000000000","message":"testing type(self) is not required because WrappedProcessHelper overloads it","commit_id":"e47db7b43bb9c766f4c4c55e010705f3d8508c5b"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"9e508b1444c8c3733c7c42cecaffd54a2f40a221","unresolved":false,"context_lines":[{"line_number":65,"context_line":"            #               it two execute calls in a row hangs the second one"},{"line_number":66,"context_line":"            greenthread.sleep(0)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def __eq__(self, other):"},{"line_number":69,"context_line":"        if type(self) is ProcessHelper and type(other) is ProcessHelper:"},{"line_number":70,"context_line":"            return True"},{"line_number":71,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":24,"id":"5a890539_3cf03d66","line":68,"updated":"2014-12-05 06:55:01.000000000","message":"who uses these operators?","commit_id":"43d259600e591d9c90cba69245a81bafec90378a"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"41b933fcc4bce9ec2473e7bb2b95dc0d95520b1c","unresolved":false,"context_lines":[{"line_number":72,"context_line":"            return NotImplemented"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"class WrappedProcessHelper(ProcessHelper):"},{"line_number":76,"context_line":"    def __init__(self, root_helper):"},{"line_number":77,"context_line":"        self.root_helper \u003d shlex.split(root_helper)"},{"line_number":78,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"3a961159_8d1cccc3","line":75,"updated":"2015-01-16 14:34:09.000000000","message":"That seems quite similar to what we have in neutron/tests/functional/agent/linux/helpers.py. Have you considered reusing/unifying the code?","commit_id":"43d259600e591d9c90cba69245a81bafec90378a"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"41b933fcc4bce9ec2473e7bb2b95dc0d95520b1c","unresolved":false,"context_lines":[{"line_number":105,"context_line":"    return root_helper.create_process(cmd, env)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"def execute(cmd, root_helper\u003dNone, process_input\u003dNone, addl_env\u003dNone,"},{"line_number":109,"context_line":"            check_exit_code\u003dTrue, return_stderr\u003dFalse):"},{"line_number":110,"context_line":"    if root_helper is None:"},{"line_number":111,"context_line":"        root_helper \u003d ProcessHelper()"}],"source_content_type":"text/x-python","patch_set":24,"id":"3a961159_2de8387a","line":108,"updated":"2015-01-16 14:34:09.000000000","message":"(No action required)\n\nI wonder whether we would ever want to consume oslo.concurrency.processutils.execute() here (I think Sean Dague mentioned that before), and if so, how it may influence that.","commit_id":"43d259600e591d9c90cba69245a81bafec90378a"}],"neutron/plugins/bigswitch/agent/restproxy_agent.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"130ee1c391712c705695bc869d9d472007bec44d","unresolved":false,"context_lines":[{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    integ_br \u003d cfg.CONF.RESTPROXYAGENT.integration_bridge"},{"line_number":172,"context_line":"    polling_interval \u003d cfg.CONF.RESTPROXYAGENT.polling_interval"},{"line_number":173,"context_line":"    root_helper \u003d agent_config.get_root_helper(cfg.CONF, False)"},{"line_number":174,"context_line":"    bsnagent \u003d RestProxyAgent(integ_br, polling_interval, root_helper,"},{"line_number":175,"context_line":"                              cfg.CONF.RESTPROXYAGENT.virtual_switch_type)"},{"line_number":176,"context_line":"    bsnagent.daemon_loop()"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAWH%2F9qsI%3D","line":173,"updated":"2014-04-03 20:16:52.000000000","message":"pass legacy\u003dFalse for readability and consistency","commit_id":"7850a99bac69329275a60780e5b76172d1721342"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"fa84927af75684b932a9e24f4cb2b0247aec16d5","unresolved":false,"context_lines":[{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    integ_br \u003d cfg.CONF.RESTPROXYAGENT.integration_bridge"},{"line_number":172,"context_line":"    polling_interval \u003d cfg.CONF.RESTPROXYAGENT.polling_interval"},{"line_number":173,"context_line":"    root_helper \u003d agent_config.get_root_helper(cfg.CONF, False)"},{"line_number":174,"context_line":"    bsnagent \u003d RestProxyAgent(integ_br, polling_interval, root_helper,"},{"line_number":175,"context_line":"                              cfg.CONF.RESTPROXYAGENT.virtual_switch_type)"},{"line_number":176,"context_line":"    bsnagent.daemon_loop()"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAWH%2F9RNk%3D","line":173,"in_reply_to":"AAAAWH%2F9qsI%3D","updated":"2014-04-07 14:34:57.000000000","message":"Done","commit_id":"7850a99bac69329275a60780e5b76172d1721342"}],"neutron/plugins/linuxbridge/agent/linuxbridge_neutron_agent.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"130ee1c391712c705695bc869d9d472007bec44d","unresolved":false,"context_lines":[{"line_number":1026,"context_line":"    LOG.info(_(\"Interface mappings: %s\"), interface_mappings)"},{"line_number":1027,"context_line":""},{"line_number":1028,"context_line":"    polling_interval \u003d cfg.CONF.AGENT.polling_interval"},{"line_number":1029,"context_line":"    root_helper \u003d agent_config.get_root_helper(cfg.CONF, False)"},{"line_number":1030,"context_line":"    agent \u003d LinuxBridgeNeutronAgentRPC(interface_mappings,"},{"line_number":1031,"context_line":"                                       polling_interval,"},{"line_number":1032,"context_line":"                                       root_helper)"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAWH%2F9qhk%3D","line":1029,"updated":"2014-04-03 20:16:52.000000000","message":"pass legacy\u003dFalse for readability and consistency","commit_id":"7850a99bac69329275a60780e5b76172d1721342"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"fa84927af75684b932a9e24f4cb2b0247aec16d5","unresolved":false,"context_lines":[{"line_number":1026,"context_line":"    LOG.info(_(\"Interface mappings: %s\"), interface_mappings)"},{"line_number":1027,"context_line":""},{"line_number":1028,"context_line":"    polling_interval \u003d cfg.CONF.AGENT.polling_interval"},{"line_number":1029,"context_line":"    root_helper \u003d agent_config.get_root_helper(cfg.CONF, False)"},{"line_number":1030,"context_line":"    agent \u003d LinuxBridgeNeutronAgentRPC(interface_mappings,"},{"line_number":1031,"context_line":"                                       polling_interval,"},{"line_number":1032,"context_line":"                                       root_helper)"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAWH%2F9RNg%3D","line":1029,"in_reply_to":"AAAAWH%2F9qhk%3D","updated":"2014-04-07 14:34:57.000000000","message":"Done","commit_id":"7850a99bac69329275a60780e5b76172d1721342"}],"neutron/plugins/openvswitch/agent/ovs_neutron_agent.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"130ee1c391712c705695bc869d9d472007bec44d","unresolved":false,"context_lines":[{"line_number":1317,"context_line":"        tun_br\u003dconfig.OVS.tunnel_bridge,"},{"line_number":1318,"context_line":"        local_ip\u003dconfig.OVS.local_ip,"},{"line_number":1319,"context_line":"        bridge_mappings\u003dbridge_mappings,"},{"line_number":1320,"context_line":"        root_helper\u003dagent_config.get_root_helper(config, False),"},{"line_number":1321,"context_line":"        polling_interval\u003dconfig.AGENT.polling_interval,"},{"line_number":1322,"context_line":"        minimize_polling\u003dconfig.AGENT.minimize_polling,"},{"line_number":1323,"context_line":"        tunnel_types\u003dconfig.AGENT.tunnel_types,"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAWH%2F9qhg%3D","line":1320,"updated":"2014-04-03 20:16:52.000000000","message":"pass legacy\u003dFalse for readability and consistency","commit_id":"7850a99bac69329275a60780e5b76172d1721342"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"fa84927af75684b932a9e24f4cb2b0247aec16d5","unresolved":false,"context_lines":[{"line_number":1317,"context_line":"        tun_br\u003dconfig.OVS.tunnel_bridge,"},{"line_number":1318,"context_line":"        local_ip\u003dconfig.OVS.local_ip,"},{"line_number":1319,"context_line":"        bridge_mappings\u003dbridge_mappings,"},{"line_number":1320,"context_line":"        root_helper\u003dagent_config.get_root_helper(config, False),"},{"line_number":1321,"context_line":"        polling_interval\u003dconfig.AGENT.polling_interval,"},{"line_number":1322,"context_line":"        minimize_polling\u003dconfig.AGENT.minimize_polling,"},{"line_number":1323,"context_line":"        tunnel_types\u003dconfig.AGENT.tunnel_types,"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAWH%2F9RNY%3D","line":1320,"in_reply_to":"AAAAWH%2F9qhg%3D","updated":"2014-04-07 14:34:57.000000000","message":"Done","commit_id":"7850a99bac69329275a60780e5b76172d1721342"}],"neutron/tests/base.py":[{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"7d262f77e12be8c7a41a6fbb0039c3b76f9008ee","unresolved":false,"context_lines":[{"line_number":140,"context_line":"            return"},{"line_number":141,"context_line":"        self.fail(\u0027Execution of this test timed out\u0027)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def mock_root_helper(self, start\u003dTrue):"},{"line_number":144,"context_line":"        rh_patch \u003d mock.patch("},{"line_number":145,"context_line":"            \u0027neutron.agent.linux.utils.get_root_helper_object\u0027,"},{"line_number":146,"context_line":"            new\u003dlambda x: x)"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F95BM%3D","line":143,"updated":"2014-04-01 11:53:22.000000000","message":"what\u0027s the point of creating a mock.patch if you don\u0027t start it? I don\u0027t see why the Boolean start is useful. Actually I\u0027d remove this method in the base test class and start the patch(\u0027neutron.agent.linux.utils.get_root_helper_object\u0027) directly in the test that need it. What am I missing?","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"53e3d7d51c1a0757a31b935f4c51cc51e7a4b33b","unresolved":false,"context_lines":[{"line_number":140,"context_line":"            return"},{"line_number":141,"context_line":"        self.fail(\u0027Execution of this test timed out\u0027)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def mock_root_helper(self, start\u003dTrue):"},{"line_number":144,"context_line":"        rh_patch \u003d mock.patch("},{"line_number":145,"context_line":"            \u0027neutron.agent.linux.utils.get_root_helper_object\u0027,"},{"line_number":146,"context_line":"            new\u003dlambda x: x)"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F9wYo%3D","line":143,"in_reply_to":"AAAAWH%2F925s%3D","updated":"2014-04-02 11:12:22.000000000","message":"This allows to use patcher as context manager if you don\u0027t want to keep the method patched for the whole test. See test_iptables_driver.py for example.\n\n\u003e If you don\u0027t want to keep the method patched for the whole test  you should stop the patch but here setting start\u003dFalse is a no op, that\u0027s why I am confused","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"1461b27155a7c204a9d72f5aaa55490583e768a3","unresolved":false,"context_lines":[{"line_number":140,"context_line":"            return"},{"line_number":141,"context_line":"        self.fail(\u0027Execution of this test timed out\u0027)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def mock_root_helper(self, start\u003dTrue):"},{"line_number":144,"context_line":"        rh_patch \u003d mock.patch("},{"line_number":145,"context_line":"            \u0027neutron.agent.linux.utils.get_root_helper_object\u0027,"},{"line_number":146,"context_line":"            new\u003dlambda x: x)"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F925s%3D","line":143,"in_reply_to":"AAAAWH%2F95BM%3D","updated":"2014-04-01 16:05:18.000000000","message":"\u003e what\u0027s the point of creating a mock.patch if you don\u0027t start it?\n\nThis allows to use patcher as context manager if you don\u0027t want to keep the method patched for the whole test. See test_iptables_driver.py for example.\n\n\u003e Actually I\u0027d remove this method in the base test class and start the patch(\u0027...\u0027) directly in the test that need it.\n\nThis would add a lot of code repetition in the tests. I\u0027d like to avoid that.","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"4080eea0536fa87457fcc5bb719536b8aa8f2e2c","unresolved":false,"context_lines":[{"line_number":140,"context_line":"            return"},{"line_number":141,"context_line":"        self.fail(\u0027Execution of this test timed out\u0027)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def mock_root_helper(self, start\u003dTrue):"},{"line_number":144,"context_line":"        rh_patch \u003d mock.patch("},{"line_number":145,"context_line":"            \u0027neutron.agent.linux.utils.get_root_helper_object\u0027,"},{"line_number":146,"context_line":"            new\u003dlambda x: x)"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F9sy0%3D","line":143,"in_reply_to":"AAAAWH%2F9tJg%3D","updated":"2014-04-02 17:56:32.000000000","message":"Added clarifying docstring.","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"5418502c63b9b3a57dda6bcc216c2307663c6f46","unresolved":false,"context_lines":[{"line_number":140,"context_line":"            return"},{"line_number":141,"context_line":"        self.fail(\u0027Execution of this test timed out\u0027)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def mock_root_helper(self, start\u003dTrue):"},{"line_number":144,"context_line":"        rh_patch \u003d mock.patch("},{"line_number":145,"context_line":"            \u0027neutron.agent.linux.utils.get_root_helper_object\u0027,"},{"line_number":146,"context_line":"            new\u003dlambda x: x)"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F9tJg%3D","line":143,"in_reply_to":"AAAAWH%2F9wYo%3D","updated":"2014-04-02 17:19:47.000000000","message":"If you set start\u003dFalse, you should start and stop this patcher yourself. For example, in test_iptables_driver it\u0027s done using context manager protocol. That\u0027s the intended use of this option.","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"}],"neutron/tests/unit/bigswitch/test_restproxy_agent.py":[{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"7d262f77e12be8c7a41a6fbb0039c3b76f9008ee","unresolved":false,"context_lines":[{"line_number":38,"context_line":"    def setUp(self):"},{"line_number":39,"context_line":"        super(BaseAgentTestCase, self).setUp()"},{"line_number":40,"context_line":"        self.mod_agent \u003d importutils.import_module(AGENTMOD)"},{"line_number":41,"context_line":"        self.mock_root_helper()"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"class TestRestProxyAgentOVS(BaseAgentTestCase):"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F949M%3D","line":41,"updated":"2014-04-01 11:53:22.000000000","message":"consider starting the patch directly here instead of using this helper method as done on line 47 for example for other patches.","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"53e3d7d51c1a0757a31b935f4c51cc51e7a4b33b","unresolved":false,"context_lines":[{"line_number":38,"context_line":"    def setUp(self):"},{"line_number":39,"context_line":"        super(BaseAgentTestCase, self).setUp()"},{"line_number":40,"context_line":"        self.mod_agent \u003d importutils.import_module(AGENTMOD)"},{"line_number":41,"context_line":"        self.mock_root_helper()"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"class TestRestProxyAgentOVS(BaseAgentTestCase):"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F9wZk%3D","line":41,"in_reply_to":"AAAAWH%2F922Y%3D","updated":"2014-04-02 11:12:22.000000000","message":"\"Moreover starting patchers indefinitely is a very bad practice as it provides global state change between tests.\"\n\nYou can add a cleanUp\n\nActually your comment made me realize...this line shouldn\u0027t be here, it\u0027s the base class, it\u0027s better to put it in the derived classes so that we can have more control of the patches that are started","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"1461b27155a7c204a9d72f5aaa55490583e768a3","unresolved":false,"context_lines":[{"line_number":38,"context_line":"    def setUp(self):"},{"line_number":39,"context_line":"        super(BaseAgentTestCase, self).setUp()"},{"line_number":40,"context_line":"        self.mod_agent \u003d importutils.import_module(AGENTMOD)"},{"line_number":41,"context_line":"        self.mock_root_helper()"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"class TestRestProxyAgentOVS(BaseAgentTestCase):"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F922Y%3D","line":41,"in_reply_to":"AAAAWH%2F949M%3D","updated":"2014-04-01 16:05:18.000000000","message":"As I commented in neutron/tests/base.py, this allows to avoid lots of code repetition since this change would need to be done in other test modules as well.\n\nMoreover starting patchers indefinitely is a very bad practice as it provides global state change between tests. This might make some other tests working with such hidden methods while they won\u0027t work on their own.","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"4080eea0536fa87457fcc5bb719536b8aa8f2e2c","unresolved":false,"context_lines":[{"line_number":38,"context_line":"    def setUp(self):"},{"line_number":39,"context_line":"        super(BaseAgentTestCase, self).setUp()"},{"line_number":40,"context_line":"        self.mod_agent \u003d importutils.import_module(AGENTMOD)"},{"line_number":41,"context_line":"        self.mock_root_helper()"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"class TestRestProxyAgentOVS(BaseAgentTestCase):"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F9szg%3D","line":41,"in_reply_to":"AAAAWH%2F9tIg%3D","updated":"2014-04-02 17:56:32.000000000","message":"Done","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"5418502c63b9b3a57dda6bcc216c2307663c6f46","unresolved":false,"context_lines":[{"line_number":38,"context_line":"    def setUp(self):"},{"line_number":39,"context_line":"        super(BaseAgentTestCase, self).setUp()"},{"line_number":40,"context_line":"        self.mod_agent \u003d importutils.import_module(AGENTMOD)"},{"line_number":41,"context_line":"        self.mock_root_helper()"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"class TestRestProxyAgentOVS(BaseAgentTestCase):"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAWH%2F9tIg%3D","line":41,"in_reply_to":"AAAAWH%2F9wZk%3D","updated":"2014-04-02 17:19:47.000000000","message":"Yes, you have to add cleanup method. That\u0027s why I decided it\u0027s better to couple them in one place.\n\nI don\u0027t like adding this call to such general place too. I\u0027ll see if it can be shifted to child classes and their tests.","commit_id":"0b7c0618aadf0c53604a9bb551b7c519f9bb095d"}]}