)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"5fd8c65f51ad2efad4223a6aa80a485d5e038500","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Aleks Chirko \u003cachirko@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2013-11-26 16:29:47 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Bugfix and refactoring for ovs_lib.OVSBridge flow managment methods"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Fixes bug: 1255058"},{"line_number":10,"context_line":"Fixes bug: 1240572"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"AAAATn%2F%2BZRc%3D","line":7,"updated":"2013-11-26 15:32:25.000000000","message":"It is better to describe how the bug is fixed briefly. I don\u0027t think it is a good idea the code describes it :-(","commit_id":"c6e9bd6601a867eb8ea7a32b4f6e23d8384167c7"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"700e86f826529c2e58fb9b6d3ec39968c91cb275","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Bugfix and refactoring for ovs_lib.OVSBridge flow managment methods"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Fixes bug: 1255058"},{"line_number":10,"context_line":"Fixes bug: 1240572"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ic89221d006a626aa2fc40314a9acffc0ea6fd61c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"AAAATn%2F%2BZi4%3D","line":10,"updated":"2013-11-26 14:39:28.000000000","message":"Following review for this bug was proposed earlier: https://review.openstack.org/#/c/58308/","commit_id":"c6e9bd6601a867eb8ea7a32b4f6e23d8384167c7"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"8f7cc5270e332192b927dd4c415900af3ccb754f","unresolved":false,"context_lines":[{"line_number":16,"context_line":"string for \u0027add_flow\u0027 and \u0027mod_flow\u0027 methods."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Fixes bug: 1255058"},{"line_number":19,"context_line":"Fixes bug: 1240572"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: Ic89221d006a626aa2fc40314a9acffc0ea6fd61c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"AAAAT3%2F%2F%2Bc4%3D","line":19,"updated":"2013-12-26 16:39:29.000000000","message":"The bugs are not getting linked correctly. Please use the format described here: https://wiki.openstack.org/wiki/GitCommitMessages#Including_external_references","commit_id":"2e5bad5e1587e99345e614e8462f3098231929d6"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"23276dcd75d7e5e58cbcec6b4938ead40f1b6ec6","unresolved":false,"context_lines":[{"line_number":16,"context_line":"string for \u0027add_flow\u0027 and \u0027mod_flow\u0027 methods."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Fixes bug: 1255058"},{"line_number":19,"context_line":"Fixes bug: 1240572"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: Ic89221d006a626aa2fc40314a9acffc0ea6fd61c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"AAAAT3%2F%2F6Ug%3D","line":19,"in_reply_to":"AAAAT3%2F%2F%2Bc4%3D","updated":"2013-12-27 11:35:47.000000000","message":"Done","commit_id":"2e5bad5e1587e99345e614e8462f3098231929d6"}],"neutron/agent/linux/ovs_lib.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"5fd8c65f51ad2efad4223a6aa80a485d5e038500","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        flow_expr_str \u003d \u0027\u0027"},{"line_number":162,"context_line":"        actions_str \u003d \u0027\u0027"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        if action is \u0027add\u0027:"},{"line_number":165,"context_line":"            prefix \u003d (\",hard_timeout\u003d%s,idle_timeout\u003d%s,priority\u003d%s\" %"},{"line_number":166,"context_line":"                     (flow_dict.pop(\u0027hard_timeout\u0027, \u00270\u0027),"},{"line_number":167,"context_line":"                      flow_dict.pop(\u0027idle_timeout\u0027, \u00270\u0027),"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BZQg%3D","line":164,"updated":"2013-11-26 15:32:25.000000000","message":"\"\u003d\u003d\" should be used. \"is\" operator checks object equality.","commit_id":"c6e9bd6601a867eb8ea7a32b4f6e23d8384167c7"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"5fd8c65f51ad2efad4223a6aa80a485d5e038500","unresolved":false,"context_lines":[{"line_number":168,"context_line":"                      flow_dict.pop(\u0027priority\u0027, \u00271\u0027)))"},{"line_number":169,"context_line":"            flow_expr_str +\u003d prefix"},{"line_number":170,"context_line":"        elif \u0027priority\u0027 in flow_dict:"},{"line_number":171,"context_line":"            raise Exception(_(\"Cannot match priority on flow deletion\""},{"line_number":172,"context_line":"                              \" or modification\"))"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"        if action is not \u0027del\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BZP8%3D","line":171,"updated":"2013-11-26 15:32:25.000000000","message":"Exception is too generic. It is better to define a new exception.","commit_id":"c6e9bd6601a867eb8ea7a32b4f6e23d8384167c7"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"5fd8c65f51ad2efad4223a6aa80a485d5e038500","unresolved":false,"context_lines":[{"line_number":171,"context_line":"            raise Exception(_(\"Cannot match priority on flow deletion\""},{"line_number":172,"context_line":"                              \" or modification\"))"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"        if action is not \u0027del\u0027:"},{"line_number":175,"context_line":"            if \"actions\" not in flow_dict:"},{"line_number":176,"context_line":"                raise Exception(_(\"Must specify one or more actions\"))"},{"line_number":177,"context_line":"            actions_str \u003d \",actions\u003d%s\" % flow_dict.pop(\u0027actions\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BZQI%3D","line":174,"updated":"2013-11-26 15:32:25.000000000","message":"Use \"!\u003d\"","commit_id":"c6e9bd6601a867eb8ea7a32b4f6e23d8384167c7"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"5fd8c65f51ad2efad4223a6aa80a485d5e038500","unresolved":false,"context_lines":[{"line_number":173,"context_line":""},{"line_number":174,"context_line":"        if action is not \u0027del\u0027:"},{"line_number":175,"context_line":"            if \"actions\" not in flow_dict:"},{"line_number":176,"context_line":"                raise Exception(_(\"Must specify one or more actions\"))"},{"line_number":177,"context_line":"            actions_str \u003d \",actions\u003d%s\" % flow_dict.pop(\u0027actions\u0027)"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"        if \u0027proto\u0027 in flow_dict:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BZP0%3D","line":176,"updated":"2013-11-26 15:32:25.000000000","message":"Same as the above.","commit_id":"c6e9bd6601a867eb8ea7a32b4f6e23d8384167c7"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"5fd8c65f51ad2efad4223a6aa80a485d5e038500","unresolved":false,"context_lines":[{"line_number":180,"context_line":"            flow_expr_str +\u003d \",%s\" % flow_dict.pop(\u0027proto\u0027)"},{"line_number":181,"context_line":" "},{"line_number":182,"context_line":"        for key, value in flow_dict.iteritems():"},{"line_number":183,"context_line":"            flow_expr_str +\u003d \",%s\u003d%s\" % (key, value)"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        if flow_expr_str:"},{"line_number":186,"context_line":"            flow_expr_str \u003d flow_expr_str[1:]  # strip leading comma"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BZPM%3D","line":183,"updated":"2013-11-26 15:32:25.000000000","message":"Who checks \"key\" is valid? Is no check intentional? (depending on ovs-ofctl validation)","commit_id":"c6e9bd6601a867eb8ea7a32b4f6e23d8384167c7"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"5fd8c65f51ad2efad4223a6aa80a485d5e038500","unresolved":false,"context_lines":[{"line_number":183,"context_line":"            flow_expr_str +\u003d \",%s\u003d%s\" % (key, value)"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        if flow_expr_str:"},{"line_number":186,"context_line":"            flow_expr_str \u003d flow_expr_str[1:]  # strip leading comma"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"        if actions_str:"},{"line_number":189,"context_line":"            flow_expr_str +\u003d actions_str"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BZOY%3D","line":186,"updated":"2013-11-26 15:32:25.000000000","message":"The alternative approach is to use a list instead of a string and join with \u0027,\u0027 when returning a value below.","commit_id":"c6e9bd6601a867eb8ea7a32b4f6e23d8384167c7"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"5fd8c65f51ad2efad4223a6aa80a485d5e038500","unresolved":false,"context_lines":[{"line_number":186,"context_line":"            flow_expr_str \u003d flow_expr_str[1:]  # strip leading comma"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"        if actions_str:"},{"line_number":189,"context_line":"            flow_expr_str +\u003d actions_str"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        return flow_expr_str"},{"line_number":192,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BZOM%3D","line":189,"updated":"2013-11-26 15:32:25.000000000","message":"If L.174-177 is moved here, actions_str is no longer needed.","commit_id":"c6e9bd6601a867eb8ea7a32b4f6e23d8384167c7"},{"author":{"_account_id":2592,"name":"Mark McClain","email":"mark@mcclain.xyz","username":"markmcclain"},"change_message_id":"55fe142318fc156683117255ad1d28acd0a3a563","unresolved":false,"context_lines":[{"line_number":158,"context_line":"                               self.br_name, \u0027datapath_id\u0027).strip(\u0027\"\u0027)"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"    def _build_flow_expr_str(self, flow_dict, action):"},{"line_number":161,"context_line":"        flow_expr_str \u003d \u0027\u0027"},{"line_number":162,"context_line":"        actions_str \u003d \u0027\u0027"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        if action \u003d\u003d \u0027add\u0027:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F96J0%3D","line":161,"updated":"2013-12-04 16:15:34.000000000","message":"Why are we using string concatenation here?  We should build a list of tuples and then use .join(). Consider:\n\nflow_expr \u003d []\n\nflow_expr.append(\u0027key\u0027, \u003cvalue\u003e)\n\nreturn \u0027,\u0027.join(\u0027%s\u003d%s\u0027 % item for item in flow_expr)","commit_id":"7945bc683741e7f413e482a96077309644c6c63c"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"84f6ba24a2ba576bd57b1f02437ec563b869d930","unresolved":false,"context_lines":[{"line_number":158,"context_line":"                               self.br_name, \u0027datapath_id\u0027).strip(\u0027\"\u0027)"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"    def _build_flow_expr_str(self, flow_dict, action):"},{"line_number":161,"context_line":"        flow_expr_str \u003d \u0027\u0027"},{"line_number":162,"context_line":"        actions_str \u003d \u0027\u0027"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        if action \u003d\u003d \u0027add\u0027:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F957w%3D","line":161,"in_reply_to":"AAAATn%2F96J0%3D","updated":"2013-12-04 17:17:16.000000000","message":"Good idea, thank you. Will do!","commit_id":"7945bc683741e7f413e482a96077309644c6c63c"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"5d619cb22cd93f54513e3d790286be4879807dbc","unresolved":false,"context_lines":[{"line_number":158,"context_line":"                               self.br_name, \u0027datapath_id\u0027).strip(\u0027\"\u0027)"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"    def _build_flow_expr_str(self, flow_dict, action):"},{"line_number":161,"context_line":"        flow_expr_str \u003d \u0027\u0027"},{"line_number":162,"context_line":"        actions_str \u003d \u0027\u0027"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        if action \u003d\u003d \u0027add\u0027:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F9Wvc%3D","line":161,"in_reply_to":"AAAATn%2F96J0%3D","updated":"2013-12-12 10:22:08.000000000","message":"OK, it could be a good idea if every flow parameter was a key/value combination. But we have \u0027proto\u0027 field which is just a string (\u0027ip\u0027, \u0027tcp\u0027 etc), and I can\u0027t just get rid of \u0027proto\u0027 because it won\u0027t be backward compatible.","commit_id":"7945bc683741e7f413e482a96077309644c6c63c"},{"author":{"_account_id":2592,"name":"Mark McClain","email":"mark@mcclain.xyz","username":"markmcclain"},"change_message_id":"55fe142318fc156683117255ad1d28acd0a3a563","unresolved":false,"context_lines":[{"line_number":180,"context_line":"            flow_expr_str +\u003d \",%s\" % flow_dict.pop(\u0027proto\u0027)"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        for key, value in flow_dict.iteritems():"},{"line_number":183,"context_line":"            flow_expr_str +\u003d \",%s\u003d%s\" % (key, value)"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        if flow_expr_str:"},{"line_number":186,"context_line":"            flow_expr_str \u003d flow_expr_str[1:]  # strip leading comma"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F96LA%3D","line":183,"updated":"2013-12-04 16:15:34.000000000","message":"This is not equivalent to the old logic of replacing \u0027\u0027 when the value is None.","commit_id":"7945bc683741e7f413e482a96077309644c6c63c"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"84f6ba24a2ba576bd57b1f02437ec563b869d930","unresolved":false,"context_lines":[{"line_number":180,"context_line":"            flow_expr_str +\u003d \",%s\" % flow_dict.pop(\u0027proto\u0027)"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        for key, value in flow_dict.iteritems():"},{"line_number":183,"context_line":"            flow_expr_str +\u003d \",%s\u003d%s\" % (key, value)"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        if flow_expr_str:"},{"line_number":186,"context_line":"            flow_expr_str \u003d flow_expr_str[1:]  # strip leading comma"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F952w%3D","line":183,"in_reply_to":"AAAATn%2F96LA%3D","updated":"2013-12-04 17:17:16.000000000","message":"OK, in my code if \u003cflow param name\u003e is not in flow_dict, it won\u0027t be packed into flow string.\nIn the old logic, if \u003chardcoded flow param name\u003e is not in kwargs - assign empty string. Then when \u0027match\u0027 string is build, empty strings will have no effect.\nSo logic differs, but the end result is the same.","commit_id":"7945bc683741e7f413e482a96077309644c6c63c"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"8f7cc5270e332192b927dd4c415900af3ccb754f","unresolved":false,"context_lines":[{"line_number":159,"context_line":"        return self.db_get_val(\u0027Bridge\u0027,"},{"line_number":160,"context_line":"                               self.br_name, \u0027datapath_id\u0027).strip(\u0027\"\u0027)"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    def _build_flow_expr_str(self, flow_dict, action):"},{"line_number":163,"context_line":"        flow_expr_arr \u003d []"},{"line_number":164,"context_line":"        actions \u003d None"},{"line_number":165,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAT3%2F%2F8V8%3D","line":162,"updated":"2013-12-26 16:39:29.000000000","message":"This can be a @staticmethod","commit_id":"2e5bad5e1587e99345e614e8462f3098231929d6"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"23276dcd75d7e5e58cbcec6b4938ead40f1b6ec6","unresolved":false,"context_lines":[{"line_number":159,"context_line":"        return self.db_get_val(\u0027Bridge\u0027,"},{"line_number":160,"context_line":"                               self.br_name, \u0027datapath_id\u0027).strip(\u0027\"\u0027)"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    def _build_flow_expr_str(self, flow_dict, action):"},{"line_number":163,"context_line":"        flow_expr_arr \u003d []"},{"line_number":164,"context_line":"        actions \u003d None"},{"line_number":165,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAT3%2F%2F6MA%3D","line":162,"in_reply_to":"AAAAT3%2F%2F8V8%3D","updated":"2013-12-27 11:35:47.000000000","message":"@staticmethod has no use except when class is going to be inherited, so I\u0027ll make it just a function.","commit_id":"2e5bad5e1587e99345e614e8462f3098231929d6"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"8f7cc5270e332192b927dd4c415900af3ccb754f","unresolved":false,"context_lines":[{"line_number":170,"context_line":"                                  flow_dict.pop(\u0027idle_timeout\u0027, \u00270\u0027)))"},{"line_number":171,"context_line":"            flow_expr_arr.append((\u0027priority\u0027, \u0027\u003d\u0027,"},{"line_number":172,"context_line":"                                  flow_dict.pop(\u0027priority\u0027, \u00271\u0027)))"},{"line_number":173,"context_line":"        elif \u0027priority\u0027 in flow_dict:"},{"line_number":174,"context_line":"            raise Exception(_(\"Cannot match priority on flow deletion\""},{"line_number":175,"context_line":"                              \" or modification\"))"},{"line_number":176,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAT3%2F%2F8Uo%3D","line":173,"updated":"2013-12-26 16:39:29.000000000","message":"I don\u0027t see a unit test covering this path?","commit_id":"2e5bad5e1587e99345e614e8462f3098231929d6"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"23276dcd75d7e5e58cbcec6b4938ead40f1b6ec6","unresolved":false,"context_lines":[{"line_number":170,"context_line":"                                  flow_dict.pop(\u0027idle_timeout\u0027, \u00270\u0027)))"},{"line_number":171,"context_line":"            flow_expr_arr.append((\u0027priority\u0027, \u0027\u003d\u0027,"},{"line_number":172,"context_line":"                                  flow_dict.pop(\u0027priority\u0027, \u00271\u0027)))"},{"line_number":173,"context_line":"        elif \u0027priority\u0027 in flow_dict:"},{"line_number":174,"context_line":"            raise Exception(_(\"Cannot match priority on flow deletion\""},{"line_number":175,"context_line":"                              \" or modification\"))"},{"line_number":176,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAT3%2F%2F6L8%3D","line":173,"in_reply_to":"AAAAT3%2F%2F8Uo%3D","updated":"2013-12-27 11:35:47.000000000","message":"Done","commit_id":"2e5bad5e1587e99345e614e8462f3098231929d6"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"cf445ac45599c4fec2410f3eff2ceb6ae6d5dc43","unresolved":false,"context_lines":[{"line_number":171,"context_line":"            flow_expr_arr.append((\u0027priority\u0027, \u0027\u003d\u0027,"},{"line_number":172,"context_line":"                                  flow_dict.pop(\u0027priority\u0027, \u00271\u0027)))"},{"line_number":173,"context_line":"        elif \u0027priority\u0027 in flow_dict:"},{"line_number":174,"context_line":"            raise Exception(_(\"Cannot match priority on flow deletion\""},{"line_number":175,"context_line":"                              \" or modification\"))"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"        if action !\u003d \u0027del\u0027:"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAT3%2F%2F8SY%3D","line":174,"updated":"2013-12-26 16:54:31.000000000","message":"This breaks OVSNeutronAgent, which does several calls to mod_flow() with priority set.","commit_id":"2e5bad5e1587e99345e614e8462f3098231929d6"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"23276dcd75d7e5e58cbcec6b4938ead40f1b6ec6","unresolved":false,"context_lines":[{"line_number":171,"context_line":"            flow_expr_arr.append((\u0027priority\u0027, \u0027\u003d\u0027,"},{"line_number":172,"context_line":"                                  flow_dict.pop(\u0027priority\u0027, \u00271\u0027)))"},{"line_number":173,"context_line":"        elif \u0027priority\u0027 in flow_dict:"},{"line_number":174,"context_line":"            raise Exception(_(\"Cannot match priority on flow deletion\""},{"line_number":175,"context_line":"                              \" or modification\"))"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"        if action !\u003d \u0027del\u0027:"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAT3%2F%2F6UE%3D","line":174,"in_reply_to":"AAAAT3%2F%2F8SY%3D","updated":"2013-12-27 11:35:47.000000000","message":"This is for https://bugs.launchpad.net/neutron/+bug/1255058","commit_id":"2e5bad5e1587e99345e614e8462f3098231929d6"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"8f7cc5270e332192b927dd4c415900af3ccb754f","unresolved":false,"context_lines":[{"line_number":172,"context_line":"                                  flow_dict.pop(\u0027priority\u0027, \u00271\u0027)))"},{"line_number":173,"context_line":"        elif \u0027priority\u0027 in flow_dict:"},{"line_number":174,"context_line":"            raise Exception(_(\"Cannot match priority on flow deletion\""},{"line_number":175,"context_line":"                              \" or modification\"))"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"        if action !\u003d \u0027del\u0027:"},{"line_number":178,"context_line":"            if \"actions\" not in flow_dict:"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAT3%2F%2F8VU%3D","line":175,"updated":"2013-12-26 16:39:29.000000000","message":"This is just an opinion, but I think the following sounds a little better: \"Cannot specify priority when deleting or modifying flows.\"","commit_id":"2e5bad5e1587e99345e614e8462f3098231929d6"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"8f7cc5270e332192b927dd4c415900af3ccb754f","unresolved":false,"context_lines":[{"line_number":175,"context_line":"                              \" or modification\"))"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"        if action !\u003d \u0027del\u0027:"},{"line_number":178,"context_line":"            if \"actions\" not in flow_dict:"},{"line_number":179,"context_line":"                raise Exception(_(\"Must specify one or more actions\"))"},{"line_number":180,"context_line":"            actions \u003d (\u0027actions\u0027, \u0027\u003d\u0027, flow_dict.pop(\u0027actions\u0027))"},{"line_number":181,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAT3%2F%2F8Ug%3D","line":178,"updated":"2013-12-26 16:39:29.000000000","message":"I don\u0027t see a unit test covering this path?","commit_id":"2e5bad5e1587e99345e614e8462f3098231929d6"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"23276dcd75d7e5e58cbcec6b4938ead40f1b6ec6","unresolved":false,"context_lines":[{"line_number":175,"context_line":"                              \" or modification\"))"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"        if action !\u003d \u0027del\u0027:"},{"line_number":178,"context_line":"            if \"actions\" not in flow_dict:"},{"line_number":179,"context_line":"                raise Exception(_(\"Must specify one or more actions\"))"},{"line_number":180,"context_line":"            actions \u003d (\u0027actions\u0027, \u0027\u003d\u0027, flow_dict.pop(\u0027actions\u0027))"},{"line_number":181,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAT3%2F%2F6LY%3D","line":178,"in_reply_to":"AAAAT3%2F%2F8Ug%3D","updated":"2013-12-27 11:35:47.000000000","message":"Done","commit_id":"2e5bad5e1587e99345e614e8462f3098231929d6"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"8f7cc5270e332192b927dd4c415900af3ccb754f","unresolved":false,"context_lines":[{"line_number":176,"context_line":""},{"line_number":177,"context_line":"        if action !\u003d \u0027del\u0027:"},{"line_number":178,"context_line":"            if \"actions\" not in flow_dict:"},{"line_number":179,"context_line":"                raise Exception(_(\"Must specify one or more actions\"))"},{"line_number":180,"context_line":"            actions \u003d (\u0027actions\u0027, \u0027\u003d\u0027, flow_dict.pop(\u0027actions\u0027))"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        for key, value in flow_dict.iteritems():"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAT3%2F%2F8Vo%3D","line":179,"updated":"2013-12-26 16:39:29.000000000","message":"\"Must specify one or more actions when adding or modifying flows.\"","commit_id":"2e5bad5e1587e99345e614e8462f3098231929d6"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"23276dcd75d7e5e58cbcec6b4938ead40f1b6ec6","unresolved":false,"context_lines":[{"line_number":176,"context_line":""},{"line_number":177,"context_line":"        if action !\u003d \u0027del\u0027:"},{"line_number":178,"context_line":"            if \"actions\" not in flow_dict:"},{"line_number":179,"context_line":"                raise Exception(_(\"Must specify one or more actions\"))"},{"line_number":180,"context_line":"            actions \u003d (\u0027actions\u0027, \u0027\u003d\u0027, flow_dict.pop(\u0027actions\u0027))"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        for key, value in flow_dict.iteritems():"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAT3%2F%2F6Is%3D","line":179,"in_reply_to":"AAAAT3%2F%2F8Vo%3D","updated":"2013-12-27 11:35:47.000000000","message":"Done","commit_id":"2e5bad5e1587e99345e614e8462f3098231929d6"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"6ef6cf7b9870bbb29bc247ec70e2fed93ab4762f","unresolved":false,"context_lines":[{"line_number":407,"context_line":"                              flow_dict.pop(\u0027priority\u0027, \u00271\u0027)))"},{"line_number":408,"context_line":"    elif \u0027priority\u0027 in flow_dict:"},{"line_number":409,"context_line":"        msg \u003d _(\"Cannot match priority on flow deletion or modification\")"},{"line_number":410,"context_line":"        raise exceptions.InvalidInput(error_message\u003dmsg)"},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"    if action !\u003d \u0027del\u0027:"},{"line_number":413,"context_line":"        if \"actions\" not in flow_dict:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAT3%2F%2F5is%3D","line":410,"updated":"2013-12-27 15:16:36.000000000","message":"I still say this will break OVSNeutronAgent. See for example https://github.com/openstack/neutron/blob/master/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py#L476\n\nI think as part of this patch you need to remove priority from all calls to mod_flow().","commit_id":"0aa928e16e7f1a713d1b5dbcd23424031108155c"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"43706ab611afb05bbabc2b9ddda5483ef991e0de","unresolved":false,"context_lines":[{"line_number":407,"context_line":"                              flow_dict.pop(\u0027priority\u0027, \u00271\u0027)))"},{"line_number":408,"context_line":"    elif \u0027priority\u0027 in flow_dict:"},{"line_number":409,"context_line":"        msg \u003d _(\"Cannot match priority on flow deletion or modification\")"},{"line_number":410,"context_line":"        raise exceptions.InvalidInput(error_message\u003dmsg)"},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"    if action !\u003d \u0027del\u0027:"},{"line_number":413,"context_line":"        if \"actions\" not in flow_dict:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAT3%2F%2F4Ns%3D","line":410,"in_reply_to":"AAAAT3%2F%2F4uc%3D","updated":"2013-12-28 11:45:51.000000000","message":"mod_flow() method doesn\u0027t work in general - see https://bugs.launchpad.net/neutron/+bug/1255058. But even after addressing this issue, ovs agent mod_flow() call will still fail because \u0027priority\u0027 is not a matching field for flow - we can\u0027t delete or modify flows based on it\u0027s priority.","commit_id":"0aa928e16e7f1a713d1b5dbcd23424031108155c"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"9040a934c6033472986d79a9e6f40e3fa2aa0624","unresolved":false,"context_lines":[{"line_number":407,"context_line":"                              flow_dict.pop(\u0027priority\u0027, \u00271\u0027)))"},{"line_number":408,"context_line":"    elif \u0027priority\u0027 in flow_dict:"},{"line_number":409,"context_line":"        msg \u003d _(\"Cannot match priority on flow deletion or modification\")"},{"line_number":410,"context_line":"        raise exceptions.InvalidInput(error_message\u003dmsg)"},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"    if action !\u003d \u0027del\u0027:"},{"line_number":413,"context_line":"        if \"actions\" not in flow_dict:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAT3%2F%2F4uc%3D","line":410,"in_reply_to":"AAAAT3%2F%2F5bU%3D","updated":"2013-12-28 01:09:45.000000000","message":"Let me first try to understand the issue a little better. Are you saying that even currently (before this patch), the ovs_neutron_agent calls to mod_flow() fail?","commit_id":"0aa928e16e7f1a713d1b5dbcd23424031108155c"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"f0b40bcd24954c36904592f5469807505910960f","unresolved":false,"context_lines":[{"line_number":407,"context_line":"                              flow_dict.pop(\u0027priority\u0027, \u00271\u0027)))"},{"line_number":408,"context_line":"    elif \u0027priority\u0027 in flow_dict:"},{"line_number":409,"context_line":"        msg \u003d _(\"Cannot match priority on flow deletion or modification\")"},{"line_number":410,"context_line":"        raise exceptions.InvalidInput(error_message\u003dmsg)"},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"    if action !\u003d \u0027del\u0027:"},{"line_number":413,"context_line":"        if \"actions\" not in flow_dict:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAT3%2F%2F5bU%3D","line":410,"in_reply_to":"AAAAT3%2F%2F5is%3D","updated":"2013-12-27 15:48:15.000000000","message":"This patch addresses ovs_lib bugs. I can create another bug which will describe ovs_neutron_agent bug and create another patch which will be dependant on this one. Is this will be OK with you?","commit_id":"0aa928e16e7f1a713d1b5dbcd23424031108155c"},{"author":{"_account_id":2592,"name":"Mark McClain","email":"mark@mcclain.xyz","username":"markmcclain"},"change_message_id":"e2fc048d507cd1076eb962ff1d59f845b9c0a4d8","unresolved":false,"context_lines":[{"line_number":439,"context_line":"    if actions:"},{"line_number":440,"context_line":"        flow_expr_arr.append(actions)"},{"line_number":441,"context_line":""},{"line_number":442,"context_line":"    return \u0027,\u0027.join(\u0027%s%s%s\u0027 % item for item in flow_expr_arr)"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUH%2F97RE%3D","line":442,"updated":"2014-02-01 00:28:36.000000000","message":"return \u0027,\u0027.join(\u0027\u0027.join(item) for item in flow_expr_arr)","commit_id":"d3f24c4fa94a85afa6debf7d21c0cd1806b72b37"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"f672cc1a45281072c5866ec157cd836f22116ede","unresolved":false,"context_lines":[{"line_number":439,"context_line":"    if actions:"},{"line_number":440,"context_line":"        flow_expr_arr.append(actions)"},{"line_number":441,"context_line":""},{"line_number":442,"context_line":"    return \u0027,\u0027.join(\u0027%s%s%s\u0027 % item for item in flow_expr_arr)"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUH%2F92TY%3D","line":442,"in_reply_to":"AAAAUH%2F97RE%3D","updated":"2014-02-03 11:03:58.000000000","message":"Done. It turned out to be more code because now we have to convert every \u0027value\u0027 in flow_expr_arr tuples to string.","commit_id":"d3f24c4fa94a85afa6debf7d21c0cd1806b72b37"},{"author":{"_account_id":105,"name":"Kyle Mestery","email":"mestery@mestery.com","username":"mestery"},"change_message_id":"4766912e3748ddf9e6159f4690d9cb7e9803c92d","unresolved":false,"context_lines":[{"line_number":413,"context_line":"    actions \u003d None"},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"    if action \u003d\u003d \u0027add\u0027:"},{"line_number":416,"context_line":"        flow_expr_arr.append((\u0027hard_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":417,"context_line":"                              str(flow_dict.pop(\u0027hard_timeout\u0027, 0))))"},{"line_number":418,"context_line":"        flow_expr_arr.append((\u0027idle_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":419,"context_line":"                              str(flow_dict.pop(\u0027idle_timeout\u0027, 0))))"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAUX%2F%2FXzg%3D","line":416,"updated":"2014-02-13 16:56:16.000000000","message":"What happens if callers of _build_flow_expr_str() don\u0027t pass in the required arguments here? Won\u0027t this fail? Should we be checking for these arguments before assuming they exist in the flow_dict being passed in?","commit_id":"da85391e44bebbaca42c7bbcb529bf3157c1ed0e"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"553e0e7610027cb5476a65b96300e46d8a371299","unresolved":false,"context_lines":[{"line_number":413,"context_line":"    actions \u003d None"},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"    if action \u003d\u003d \u0027add\u0027:"},{"line_number":416,"context_line":"        flow_expr_arr.append((\u0027hard_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":417,"context_line":"                              str(flow_dict.pop(\u0027hard_timeout\u0027, 0))))"},{"line_number":418,"context_line":"        flow_expr_arr.append((\u0027idle_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":419,"context_line":"                              str(flow_dict.pop(\u0027idle_timeout\u0027, 0))))"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAUX%2F%2B4mg%3D","line":416,"in_reply_to":"AAAAUX%2F%2FXzg%3D","updated":"2014-02-18 12:43:53.000000000","message":"Cases mentioned by you are covered by unit tests - check them out. In the last patch I\u0027ve added a couple more (tests).","commit_id":"da85391e44bebbaca42c7bbcb529bf3157c1ed0e"},{"author":{"_account_id":333,"name":"Isaku Yamahata","email":"isaku.yamahata@gmail.com","username":"yamahata"},"change_message_id":"52bab7875a03fffe16502e3983b9f9a1b4f2f8e4","unresolved":false,"context_lines":[{"line_number":414,"context_line":""},{"line_number":415,"context_line":"    if action \u003d\u003d \u0027add\u0027:"},{"line_number":416,"context_line":"        flow_expr_arr.append((\u0027hard_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":417,"context_line":"                              str(flow_dict.pop(\u0027hard_timeout\u0027, 0))))"},{"line_number":418,"context_line":"        flow_expr_arr.append((\u0027idle_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":419,"context_line":"                              str(flow_dict.pop(\u0027idle_timeout\u0027, 0))))"},{"line_number":420,"context_line":"        flow_expr_arr.append((\u0027priority\u0027, \u0027\u003d\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAUX%2F%2B6qU%3D","line":417,"updated":"2014-02-18 08:11:30.000000000","message":"flow_dict.get.\n\ndict.pop modifies dict instance. So the behavior changes from the original code.\nPlease see around L179 of the original code.\n\nIf you want such behavior, it should be another patch.","commit_id":"da85391e44bebbaca42c7bbcb529bf3157c1ed0e"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"553e0e7610027cb5476a65b96300e46d8a371299","unresolved":false,"context_lines":[{"line_number":414,"context_line":""},{"line_number":415,"context_line":"    if action \u003d\u003d \u0027add\u0027:"},{"line_number":416,"context_line":"        flow_expr_arr.append((\u0027hard_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":417,"context_line":"                              str(flow_dict.pop(\u0027hard_timeout\u0027, 0))))"},{"line_number":418,"context_line":"        flow_expr_arr.append((\u0027idle_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":419,"context_line":"                              str(flow_dict.pop(\u0027idle_timeout\u0027, 0))))"},{"line_number":420,"context_line":"        flow_expr_arr.append((\u0027priority\u0027, \u0027\u003d\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAUX%2F%2B4jE%3D","line":417,"in_reply_to":"AAAAUX%2F%2B6qU%3D","updated":"2014-02-18 12:43:53.000000000","message":"This is the point. Please take a look at linked bugs to understand what I\u0027m going for. In one sentence: we build prefix and postfix first, then pack everything what\u0027s left in the middle, and then put it all together - that\u0027s why we need to \u0027pop\u0027 rather than \u0027get\u0027 when building prefix and postfix.","commit_id":"da85391e44bebbaca42c7bbcb529bf3157c1ed0e"},{"author":{"_account_id":105,"name":"Kyle Mestery","email":"mestery@mestery.com","username":"mestery"},"change_message_id":"4766912e3748ddf9e6159f4690d9cb7e9803c92d","unresolved":false,"context_lines":[{"line_number":415,"context_line":"    if action \u003d\u003d \u0027add\u0027:"},{"line_number":416,"context_line":"        flow_expr_arr.append((\u0027hard_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":417,"context_line":"                              str(flow_dict.pop(\u0027hard_timeout\u0027, 0))))"},{"line_number":418,"context_line":"        flow_expr_arr.append((\u0027idle_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":419,"context_line":"                              str(flow_dict.pop(\u0027idle_timeout\u0027, 0))))"},{"line_number":420,"context_line":"        flow_expr_arr.append((\u0027priority\u0027, \u0027\u003d\u0027,"},{"line_number":421,"context_line":"                              str(flow_dict.pop(\u0027priority\u0027, 1))))"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAUX%2F%2FXzY%3D","line":418,"updated":"2014-02-13 16:56:16.000000000","message":"I think a unit test to ensure these are always passed in (since that is what appears to be assumed here) would be good to add.","commit_id":"da85391e44bebbaca42c7bbcb529bf3157c1ed0e"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"553e0e7610027cb5476a65b96300e46d8a371299","unresolved":false,"context_lines":[{"line_number":415,"context_line":"    if action \u003d\u003d \u0027add\u0027:"},{"line_number":416,"context_line":"        flow_expr_arr.append((\u0027hard_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":417,"context_line":"                              str(flow_dict.pop(\u0027hard_timeout\u0027, 0))))"},{"line_number":418,"context_line":"        flow_expr_arr.append((\u0027idle_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":419,"context_line":"                              str(flow_dict.pop(\u0027idle_timeout\u0027, 0))))"},{"line_number":420,"context_line":"        flow_expr_arr.append((\u0027priority\u0027, \u0027\u003d\u0027,"},{"line_number":421,"context_line":"                              str(flow_dict.pop(\u0027priority\u0027, 1))))"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAUX%2F%2B4lg%3D","line":418,"in_reply_to":"AAAAUX%2F%2FXzY%3D","updated":"2014-02-18 12:43:53.000000000","message":"If prefix arguments are not passed in, then we will use default values - 0\u0027s for time-outs, 1 for priority.","commit_id":"da85391e44bebbaca42c7bbcb529bf3157c1ed0e"},{"author":{"_account_id":2592,"name":"Mark McClain","email":"mark@mcclain.xyz","username":"markmcclain"},"change_message_id":"6015d80102f70c0e82445763981e29eeeaf868ee","unresolved":false,"context_lines":[{"line_number":446,"context_line":""},{"line_number":447,"context_line":"def _build_flow_expr_str(flow_dict, action):"},{"line_number":448,"context_line":"    flow_expr_arr \u003d []"},{"line_number":449,"context_line":"    actions \u003d None"},{"line_number":450,"context_line":""},{"line_number":451,"context_line":"    if action \u003d\u003d \u0027add\u0027:"},{"line_number":452,"context_line":"        flow_expr_arr.append((\u0027hard_timeout\u0027, \u0027\u003d\u0027,"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAU3%2F%2F6r8%3D","line":449,"updated":"2014-02-24 04:05:13.000000000","message":"this is not needed","commit_id":"ec26b7a0c194ab8ee0b1e2f779adfb3bac4c96cb"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"5565347bb510edceff80f9feda4c04bb50cbda3c","unresolved":false,"context_lines":[{"line_number":446,"context_line":""},{"line_number":447,"context_line":"def _build_flow_expr_str(flow_dict, action):"},{"line_number":448,"context_line":"    flow_expr_arr \u003d []"},{"line_number":449,"context_line":"    actions \u003d None"},{"line_number":450,"context_line":""},{"line_number":451,"context_line":"    if action \u003d\u003d \u0027add\u0027:"},{"line_number":452,"context_line":"        flow_expr_arr.append((\u0027hard_timeout\u0027, \u0027\u003d\u0027,"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAU3%2F%2Fqc4%3D","line":449,"in_reply_to":"AAAAU3%2F%2F6r8%3D","updated":"2014-02-25 12:01:20.000000000","message":"It\u0027s necessary -  it will be \u0027referenced before assignment\u0027 on line 475 if there is no \u0027actions\u0027 in flow_dict. Maybe you meant something else?","commit_id":"ec26b7a0c194ab8ee0b1e2f779adfb3bac4c96cb"},{"author":{"_account_id":2592,"name":"Mark McClain","email":"mark@mcclain.xyz","username":"markmcclain"},"change_message_id":"6015d80102f70c0e82445763981e29eeeaf868ee","unresolved":false,"context_lines":[{"line_number":459,"context_line":"        msg \u003d _(\"Cannot match priority on flow deletion or modification\")"},{"line_number":460,"context_line":"        raise exceptions.InvalidInput(error_message\u003dmsg)"},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"    if action !\u003d \u0027del\u0027:"},{"line_number":463,"context_line":"        if \"actions\" not in flow_dict:"},{"line_number":464,"context_line":"            msg \u003d _(\"Must specify one or more actions on flow addition\""},{"line_number":465,"context_line":"                    \" or modification\")"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAU3%2F%2F6rw%3D","line":462,"updated":"2014-02-24 04:05:13.000000000","message":"This block can be moved to line 475 and can append to flow_expr_arr directly.","commit_id":"ec26b7a0c194ab8ee0b1e2f779adfb3bac4c96cb"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"5565347bb510edceff80f9feda4c04bb50cbda3c","unresolved":false,"context_lines":[{"line_number":459,"context_line":"        msg \u003d _(\"Cannot match priority on flow deletion or modification\")"},{"line_number":460,"context_line":"        raise exceptions.InvalidInput(error_message\u003dmsg)"},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"    if action !\u003d \u0027del\u0027:"},{"line_number":463,"context_line":"        if \"actions\" not in flow_dict:"},{"line_number":464,"context_line":"            msg \u003d _(\"Must specify one or more actions on flow addition\""},{"line_number":465,"context_line":"                    \" or modification\")"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAU3%2F%2FqWQ%3D","line":462,"in_reply_to":"AAAAU3%2F%2F6rw%3D","updated":"2014-02-25 12:01:20.000000000","message":"I\u0027ll just show you what happened when I did as you proposed:\n\"Actual call: execute([\u0027ovs-ofctl\u0027, \u0027add-flow\u0027, \u0027br-int\u0027, \u0027hard_timeout\u003d1000,idle_timeout\u003d2000,priority\u003d1,actions\u003dnormal,actions\u003dnormal\u0027], process_input\u003dNone, root_helper\u003d\u0027sudo\u0027)\"\nSo \u0027actions\u0027 option appear twice. You didn\u0027t pay enough attention during your review :(","commit_id":"ec26b7a0c194ab8ee0b1e2f779adfb3bac4c96cb"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"c9ced91c3d53982922cb53bd46783f71cacac945","unresolved":false,"context_lines":[{"line_number":459,"context_line":"        msg \u003d _(\"Cannot match priority on flow deletion or modification\")"},{"line_number":460,"context_line":"        raise exceptions.InvalidInput(error_message\u003dmsg)"},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"    if action !\u003d \u0027del\u0027:"},{"line_number":463,"context_line":"        if \"actions\" not in flow_dict:"},{"line_number":464,"context_line":"            msg \u003d _(\"Must specify one or more actions on flow addition\""},{"line_number":465,"context_line":"                    \" or modification\")"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAVX%2F%2Fp8Q%3D","line":462,"in_reply_to":"AAAAU3%2F%2FqWQ%3D","updated":"2014-03-01 20:54:31.000000000","message":"I guess being mean won\u0027t help me to get my patch in :), so I elaborate. So to build correct flow string we have to:\n1. Build prefix and remove prefix params from flow_dict\n2. Build postfix (actions) and remove \u0027actions\u0027 from flow dict\n3. Inside the loop build flow array from everything what\u0027s left in flow_dict.\n4. Append postfix (actions) to the flow array.\nSo if we won\u0027t remove params from flow_dict when building prefix and postfix, at step #3 they will get in flow array for a second time. Same will be if we do step #1 or #2 after step #3.","commit_id":"ec26b7a0c194ab8ee0b1e2f779adfb3bac4c96cb"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"a3f66b5d04853212811d2b329f53ab47e6f64df1","unresolved":false,"context_lines":[{"line_number":435,"context_line":"        return None"},{"line_number":436,"context_line":""},{"line_number":437,"context_line":""},{"line_number":438,"context_line":"def _build_flow_expr_str(flow_dict, action):"},{"line_number":439,"context_line":"    flow_expr_arr \u003d []"},{"line_number":440,"context_line":"    actions \u003d None"},{"line_number":441,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAU3%2F%2FdNI%3D","line":438,"updated":"2014-02-26 14:39:51.000000000","message":"this \"action\" argument seems confusing as it actually contains an openflow flow_mod command, not an action.\nit\u0027s better to name it \"command\".","commit_id":"7187f94588174a5c6feb846209eaa62cc5105341"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"bbd68c5348a1913790f37adab080e37d38f3874e","unresolved":false,"context_lines":[{"line_number":435,"context_line":"        return None"},{"line_number":436,"context_line":""},{"line_number":437,"context_line":""},{"line_number":438,"context_line":"def _build_flow_expr_str(flow_dict, action):"},{"line_number":439,"context_line":"    flow_expr_arr \u003d []"},{"line_number":440,"context_line":"    actions \u003d None"},{"line_number":441,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAVX%2F%2Fq9E%3D","line":438,"in_reply_to":"AAAAU3%2F%2FdNI%3D","updated":"2014-03-01 10:10:17.000000000","message":"operation?","commit_id":"7187f94588174a5c6feb846209eaa62cc5105341"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"b4aa12cf486630b12a296954844ae51defc5951c","unresolved":false,"context_lines":[{"line_number":435,"context_line":"        return None"},{"line_number":436,"context_line":""},{"line_number":437,"context_line":""},{"line_number":438,"context_line":"def _build_flow_expr_str(flow_dict, action):"},{"line_number":439,"context_line":"    flow_expr_arr \u003d []"},{"line_number":440,"context_line":"    actions \u003d None"},{"line_number":441,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAVX%2F%2FqNk%3D","line":438,"in_reply_to":"AAAAVX%2F%2Fq9E%3D","updated":"2014-03-01 18:01:38.000000000","message":"IMO command is better as it corresponds to ofp_flow_mod_command.\nbut i don\u0027t have strong opinion.","commit_id":"7187f94588174a5c6feb846209eaa62cc5105341"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"7e4a1619166d2af29d809f2be347a41bc62056d7","unresolved":false,"context_lines":[{"line_number":435,"context_line":"        return None"},{"line_number":436,"context_line":""},{"line_number":437,"context_line":""},{"line_number":438,"context_line":"def _build_flow_expr_str(flow_dict, action):"},{"line_number":439,"context_line":"    flow_expr_arr \u003d []"},{"line_number":440,"context_line":"    actions \u003d None"},{"line_number":441,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAVX%2F%2Fp88%3D","line":438,"in_reply_to":"AAAAVX%2F%2FqNk%3D","updated":"2014-03-01 20:54:20.000000000","message":"\u0027command\u0027 or \u0027cmd\u0027 sounds good to me.","commit_id":"7187f94588174a5c6feb846209eaa62cc5105341"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"c6876e65027e00376a146e9800f9b45074a6ce17","unresolved":false,"context_lines":[{"line_number":441,"context_line":""},{"line_number":442,"context_line":"    if action \u003d\u003d \u0027add\u0027:"},{"line_number":443,"context_line":"        flow_expr_arr.append((\u0027hard_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":444,"context_line":"                              str(flow_dict.pop(\u0027hard_timeout\u0027, 0))))"},{"line_number":445,"context_line":"        flow_expr_arr.append((\u0027idle_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":446,"context_line":"                              str(flow_dict.pop(\u0027idle_timeout\u0027, 0))))"},{"line_number":447,"context_line":"        flow_expr_arr.append((\u0027priority\u0027, \u0027\u003d\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAU3%2F%2FpuY%3D","line":444,"updated":"2014-02-25 13:19:18.000000000","message":"Why do we need to use a tuple? Elements of a tuple are joined at L.469, so I think we can construct a string here for each entry.\n\n  flow_expr_arr.append(\n      \u0027hard_timeout\u003d%s\u0027 % flow_dict.pop(\u0027hard_timeout\u0027, 0))","commit_id":"7187f94588174a5c6feb846209eaa62cc5105341"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"7e4a1619166d2af29d809f2be347a41bc62056d7","unresolved":false,"context_lines":[{"line_number":441,"context_line":""},{"line_number":442,"context_line":"    if action \u003d\u003d \u0027add\u0027:"},{"line_number":443,"context_line":"        flow_expr_arr.append((\u0027hard_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":444,"context_line":"                              str(flow_dict.pop(\u0027hard_timeout\u0027, 0))))"},{"line_number":445,"context_line":"        flow_expr_arr.append((\u0027idle_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":446,"context_line":"                              str(flow_dict.pop(\u0027idle_timeout\u0027, 0))))"},{"line_number":447,"context_line":"        flow_expr_arr.append((\u0027priority\u0027, \u0027\u003d\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAVX%2F%2Fp9A%3D","line":444,"in_reply_to":"AAAAU3%2F%2FeYg%3D","updated":"2014-03-01 20:54:20.000000000","message":"It does make sense. I just want to mention that in comment for patch set 3 Mark suggestion was \"Why are we using string concatenation here? We should build a list of tuples and then use .join()\", and I did exactly that.","commit_id":"7187f94588174a5c6feb846209eaa62cc5105341"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"4ad09126f3eaf0898163b28ad05fb6e4f19323c9","unresolved":false,"context_lines":[{"line_number":441,"context_line":""},{"line_number":442,"context_line":"    if action \u003d\u003d \u0027add\u0027:"},{"line_number":443,"context_line":"        flow_expr_arr.append((\u0027hard_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":444,"context_line":"                              str(flow_dict.pop(\u0027hard_timeout\u0027, 0))))"},{"line_number":445,"context_line":"        flow_expr_arr.append((\u0027idle_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":446,"context_line":"                              str(flow_dict.pop(\u0027idle_timeout\u0027, 0))))"},{"line_number":447,"context_line":"        flow_expr_arr.append((\u0027priority\u0027, \u0027\u003d\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAU3%2F%2FeYg%3D","line":444,"in_reply_to":"AAAAU3%2F%2FkWo%3D","updated":"2014-02-26 12:27:22.000000000","message":"I checked the discussion in the previous patch sets.\n\nThe first comment from mark is to suggest to use \u0027,\u0027.join() instead of string concat for outer loop (flow_expr_arr).\n\nFor internal join (i.e., \u0027\u0027.join(item)) is not necessarily suggested. It was suggested in patch set 9, but I think it was because the code used \"%s%s%s\" at the end of the method. Compared to this \u0027\u0027.join(item) looks pythonic.\n\nAt a conclusion, I think comments from mark and me are not circulated. If we use a string substitution for each element of flow_expr_ary, we can avoid using \u0027\u003d\u0027 and L.462 will be simpler.\nAnd the last line can be:\n\n  return \u0027,\u0027.join(flow_expr_arr)\n\nIt looks more simple and easy to read.\n\nDoes it make sense?","commit_id":"7187f94588174a5c6feb846209eaa62cc5105341"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"1bcf23890d4e5797c4cfac26cfb93e9c74bc2021","unresolved":false,"context_lines":[{"line_number":441,"context_line":""},{"line_number":442,"context_line":"    if action \u003d\u003d \u0027add\u0027:"},{"line_number":443,"context_line":"        flow_expr_arr.append((\u0027hard_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":444,"context_line":"                              str(flow_dict.pop(\u0027hard_timeout\u0027, 0))))"},{"line_number":445,"context_line":"        flow_expr_arr.append((\u0027idle_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":446,"context_line":"                              str(flow_dict.pop(\u0027idle_timeout\u0027, 0))))"},{"line_number":447,"context_line":"        flow_expr_arr.append((\u0027priority\u0027, \u0027\u003d\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAU3%2F%2FkWo%3D","line":444,"in_reply_to":"AAAAU3%2F%2FpuY%3D","updated":"2014-02-25 21:48:16.000000000","message":"I did start with what you proposing, then Mark told me to use tuple, and I did. And now we came full circle.","commit_id":"7187f94588174a5c6feb846209eaa62cc5105341"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"11c37e09996622edb383c6bd621386a4bff6fa47","unresolved":false,"context_lines":[{"line_number":441,"context_line":""},{"line_number":442,"context_line":"    if action \u003d\u003d \u0027add\u0027:"},{"line_number":443,"context_line":"        flow_expr_arr.append((\u0027hard_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":444,"context_line":"                              str(flow_dict.pop(\u0027hard_timeout\u0027, 0))))"},{"line_number":445,"context_line":"        flow_expr_arr.append((\u0027idle_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":446,"context_line":"                              str(flow_dict.pop(\u0027idle_timeout\u0027, 0))))"},{"line_number":447,"context_line":"        flow_expr_arr.append((\u0027priority\u0027, \u0027\u003d\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAWH%2F%2FsNA%3D","line":444,"in_reply_to":"AAAAU3%2F%2FpuY%3D","updated":"2014-03-14 13:29:12.000000000","message":"I\u0027ve changed flow_expr_arr to be an array of strings. One additional downside of using tuples is that it confuse a lot of people. So once again - simpler is better.","commit_id":"7187f94588174a5c6feb846209eaa62cc5105341"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"c6876e65027e00376a146e9800f9b45074a6ce17","unresolved":false,"context_lines":[{"line_number":446,"context_line":"                              str(flow_dict.pop(\u0027idle_timeout\u0027, 0))))"},{"line_number":447,"context_line":"        flow_expr_arr.append((\u0027priority\u0027, \u0027\u003d\u0027,"},{"line_number":448,"context_line":"                              str(flow_dict.pop(\u0027priority\u0027, 1))))"},{"line_number":449,"context_line":"    elif \u0027priority\u0027 in flow_dict:"},{"line_number":450,"context_line":"        msg \u003d _(\"Cannot match priority on flow deletion or modification\")"},{"line_number":451,"context_line":"        raise exceptions.InvalidInput(error_message\u003dmsg)"},{"line_number":452,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAU3%2F%2Fpxo%3D","line":449,"updated":"2014-02-25 13:19:18.000000000","message":"According to ovs-ofctl man pages, mod-flows and del-flows with --strict option allow to specify priority. Do we need to consider this case?","commit_id":"7187f94588174a5c6feb846209eaa62cc5105341"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"4ad09126f3eaf0898163b28ad05fb6e4f19323c9","unresolved":false,"context_lines":[{"line_number":446,"context_line":"                              str(flow_dict.pop(\u0027idle_timeout\u0027, 0))))"},{"line_number":447,"context_line":"        flow_expr_arr.append((\u0027priority\u0027, \u0027\u003d\u0027,"},{"line_number":448,"context_line":"                              str(flow_dict.pop(\u0027priority\u0027, 1))))"},{"line_number":449,"context_line":"    elif \u0027priority\u0027 in flow_dict:"},{"line_number":450,"context_line":"        msg \u003d _(\"Cannot match priority on flow deletion or modification\")"},{"line_number":451,"context_line":"        raise exceptions.InvalidInput(error_message\u003dmsg)"},{"line_number":452,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAU3%2F%2FeYE%3D","line":449,"in_reply_to":"AAAAU3%2F%2FkVU%3D","updated":"2014-02-26 12:27:22.000000000","message":"Agree. It is just a question.","commit_id":"7187f94588174a5c6feb846209eaa62cc5105341"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"1bcf23890d4e5797c4cfac26cfb93e9c74bc2021","unresolved":false,"context_lines":[{"line_number":446,"context_line":"                              str(flow_dict.pop(\u0027idle_timeout\u0027, 0))))"},{"line_number":447,"context_line":"        flow_expr_arr.append((\u0027priority\u0027, \u0027\u003d\u0027,"},{"line_number":448,"context_line":"                              str(flow_dict.pop(\u0027priority\u0027, 1))))"},{"line_number":449,"context_line":"    elif \u0027priority\u0027 in flow_dict:"},{"line_number":450,"context_line":"        msg \u003d _(\"Cannot match priority on flow deletion or modification\")"},{"line_number":451,"context_line":"        raise exceptions.InvalidInput(error_message\u003dmsg)"},{"line_number":452,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAU3%2F%2FkVU%3D","line":449,"in_reply_to":"AAAAU3%2F%2Fpxo%3D","updated":"2014-02-25 21:48:16.000000000","message":"It is definitely a possibility. I would rather done it with another patch - this patch is already a bit to long in review for such small modifications.","commit_id":"7187f94588174a5c6feb846209eaa62cc5105341"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"b18284caebd053519e762e9442fc71cfec87a2b2","unresolved":false,"context_lines":[{"line_number":506,"context_line":""},{"line_number":507,"context_line":"    if cmd \u003d\u003d \u0027add\u0027:"},{"line_number":508,"context_line":"        flow_expr_arr.append((\u0027hard_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":509,"context_line":"                              str(flow_dict.pop(\u0027hard_timeout\u0027, 0))))"},{"line_number":510,"context_line":"        flow_expr_arr.append((\u0027idle_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":511,"context_line":"                              str(flow_dict.pop(\u0027idle_timeout\u0027, 0))))"},{"line_number":512,"context_line":"        flow_expr_arr.append((\u0027priority\u0027, \u0027\u003d\u0027,"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAVn%2F%2F364%3D","line":509,"updated":"2014-03-03 17:39:34.000000000","message":"(\u0027hard_timeout\u003d\u0027, xxx) should be enough.\n\nwell, these uses of tuples look weird to me.\ni prefer flow_expr_arr be a list of strings.\nbut i don\u0027t have strong opinions.","commit_id":"71e7268bc98a2c15e17209e96764cd2d9d5bb649"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"c8646023d40ceb4bedbb9150a81f8cff7279370c","unresolved":false,"context_lines":[{"line_number":506,"context_line":""},{"line_number":507,"context_line":"    if cmd \u003d\u003d \u0027add\u0027:"},{"line_number":508,"context_line":"        flow_expr_arr.append((\u0027hard_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":509,"context_line":"                              str(flow_dict.pop(\u0027hard_timeout\u0027, 0))))"},{"line_number":510,"context_line":"        flow_expr_arr.append((\u0027idle_timeout\u0027, \u0027\u003d\u0027,"},{"line_number":511,"context_line":"                              str(flow_dict.pop(\u0027idle_timeout\u0027, 0))))"},{"line_number":512,"context_line":"        flow_expr_arr.append((\u0027priority\u0027, \u0027\u003d\u0027,"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAVn%2F%2FZqg%3D","line":509,"in_reply_to":"AAAAVn%2F%2F364%3D","updated":"2014-03-06 10:59:56.000000000","message":"Done","commit_id":"71e7268bc98a2c15e17209e96764cd2d9d5bb649"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"b18284caebd053519e762e9442fc71cfec87a2b2","unresolved":false,"context_lines":[{"line_number":524,"context_line":""},{"line_number":525,"context_line":"    for key, value in flow_dict.iteritems():"},{"line_number":526,"context_line":"        if key \u003d\u003d \u0027proto\u0027:"},{"line_number":527,"context_line":"            flow_expr_arr.append((value, \u0027\u0027, \u0027\u0027))"},{"line_number":528,"context_line":"        else:"},{"line_number":529,"context_line":"            flow_expr_arr.append((key, \u0027\u003d\u0027, str(value)))"},{"line_number":530,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAVn%2F%2F39c%3D","line":527,"updated":"2014-03-03 17:39:34.000000000","message":"(value,) should be enough.","commit_id":"71e7268bc98a2c15e17209e96764cd2d9d5bb649"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"c8646023d40ceb4bedbb9150a81f8cff7279370c","unresolved":false,"context_lines":[{"line_number":524,"context_line":""},{"line_number":525,"context_line":"    for key, value in flow_dict.iteritems():"},{"line_number":526,"context_line":"        if key \u003d\u003d \u0027proto\u0027:"},{"line_number":527,"context_line":"            flow_expr_arr.append((value, \u0027\u0027, \u0027\u0027))"},{"line_number":528,"context_line":"        else:"},{"line_number":529,"context_line":"            flow_expr_arr.append((key, \u0027\u003d\u0027, str(value)))"},{"line_number":530,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAVn%2F%2FZqc%3D","line":527,"in_reply_to":"AAAAVn%2F%2F39c%3D","updated":"2014-03-06 10:59:56.000000000","message":"Done","commit_id":"71e7268bc98a2c15e17209e96764cd2d9d5bb649"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"93dd7f7a78fb5cb2e016c65041fe7a6c8fe59fe7","unresolved":false,"context_lines":[{"line_number":524,"context_line":""},{"line_number":525,"context_line":"    for key, value in flow_dict.iteritems():"},{"line_number":526,"context_line":"        if key \u003d\u003d \u0027proto\u0027:"},{"line_number":527,"context_line":"            flow_expr_arr.append((value))"},{"line_number":528,"context_line":"        else:"},{"line_number":529,"context_line":"            flow_expr_arr.append((key + \u0027\u003d\u0027, str(value)))"},{"line_number":530,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAAVn%2F%2FR84%3D","line":527,"updated":"2014-03-07 03:39:53.000000000","message":"(value,) please.\n\n(value,) is a tuple with a single item.\n(value) is an equivalent of just value.\njoining the latter is not what you intended, right?","commit_id":"2305f35fe30184ba65e96b196bc2ce83a052d33a"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"f46d9369f908f71fb94f1263cec29168b3b8071b","unresolved":false,"context_lines":[{"line_number":524,"context_line":""},{"line_number":525,"context_line":"    for key, value in flow_dict.iteritems():"},{"line_number":526,"context_line":"        if key \u003d\u003d \u0027proto\u0027:"},{"line_number":527,"context_line":"            flow_expr_arr.append((value))"},{"line_number":528,"context_line":"        else:"},{"line_number":529,"context_line":"            flow_expr_arr.append((key + \u0027\u003d\u0027, str(value)))"},{"line_number":530,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAAVn%2F%2B8%2B0%3D","line":527,"in_reply_to":"AAAAVn%2F%2FHFw%3D","updated":"2014-03-11 03:15:01.000000000","message":"i meant it should be\nflow_expr_arr.append((value,))","commit_id":"2305f35fe30184ba65e96b196bc2ce83a052d33a"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"7422e258d3a436525e2f5c3504fbbacedfebc8e6","unresolved":false,"context_lines":[{"line_number":524,"context_line":""},{"line_number":525,"context_line":"    for key, value in flow_dict.iteritems():"},{"line_number":526,"context_line":"        if key \u003d\u003d \u0027proto\u0027:"},{"line_number":527,"context_line":"            flow_expr_arr.append((value))"},{"line_number":528,"context_line":"        else:"},{"line_number":529,"context_line":"            flow_expr_arr.append((key + \u0027\u003d\u0027, str(value)))"},{"line_number":530,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAAVn%2F%2FHFw%3D","line":527,"in_reply_to":"AAAAVn%2F%2FR84%3D","updated":"2014-03-09 13:07:59.000000000","message":"Done","commit_id":"2305f35fe30184ba65e96b196bc2ce83a052d33a"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"7dca970f68b632885b90fd425e370469f9091b75","unresolved":false,"context_lines":[{"line_number":524,"context_line":""},{"line_number":525,"context_line":"    for key, value in flow_dict.iteritems():"},{"line_number":526,"context_line":"        if key \u003d\u003d \u0027proto\u0027:"},{"line_number":527,"context_line":"            flow_expr_arr.append(value)"},{"line_number":528,"context_line":"        else:"},{"line_number":529,"context_line":"            flow_expr_arr.append((key + \u0027\u003d\u0027, str(value)))"},{"line_number":530,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAVn%2F%2FG2c%3D","line":527,"updated":"2014-03-09 15:58:36.000000000","message":"The data tyle of \"item\" at L.534 should be consistent (tuple in this case). Although \u0027\u0027.join(\u0027aaaa\u0027) works, it looks tricky and it is not good from the point of view of code maintenance.\n\nI don\u0027t still understand why we need to use a tuple in this method, but perhaps it is my preference and such thing should not be a blocking issue.....","commit_id":"9f218cdc4e1027058b4d8303bdca892eaebe3dec"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"11ea5d61c3498df4bb32d59e9b3cc03b591e6c1a","unresolved":false,"context_lines":[{"line_number":538,"context_line":"        flow_expr_arr.append(\"hard_timeout\u003d%s\" %"},{"line_number":539,"context_line":"                             flow_dict.pop(\u0027hard_timeout\u0027, \u00270\u0027))"},{"line_number":540,"context_line":"        flow_expr_arr.append(\"idle_timeout\u003d%s\" %"},{"line_number":541,"context_line":"                             flow_dict.pop(\u0027idle_timeout\u0027, \u00270\u0027))"},{"line_number":542,"context_line":"        flow_expr_arr.append(\"priority\u003d%s\" %"},{"line_number":543,"context_line":"                             flow_dict.pop(\u0027priority\u0027, \u00271\u0027))"},{"line_number":544,"context_line":"    elif \u0027priority\u0027 in flow_dict:"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAWH%2F%2FGoY%3D","line":541,"updated":"2014-03-20 08:40:48.000000000","message":"are these necessary?  according to ovs-ofctl.8,\ndefault values for these timeouts are 0.","commit_id":"3c00dd43f613c838f713a7cbf3cedb6767a8c52a"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"937ace3b3aba9ae24fa24a1fca4907b0b93d84ec","unresolved":false,"context_lines":[{"line_number":538,"context_line":"        flow_expr_arr.append(\"hard_timeout\u003d%s\" %"},{"line_number":539,"context_line":"                             flow_dict.pop(\u0027hard_timeout\u0027, \u00270\u0027))"},{"line_number":540,"context_line":"        flow_expr_arr.append(\"idle_timeout\u003d%s\" %"},{"line_number":541,"context_line":"                             flow_dict.pop(\u0027idle_timeout\u0027, \u00270\u0027))"},{"line_number":542,"context_line":"        flow_expr_arr.append(\"priority\u003d%s\" %"},{"line_number":543,"context_line":"                             flow_dict.pop(\u0027priority\u0027, \u00271\u0027))"},{"line_number":544,"context_line":"    elif \u0027priority\u0027 in flow_dict:"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAWH%2F%2FFPo%3D","line":541,"in_reply_to":"AAAAWH%2F%2FF6A%3D","updated":"2014-03-20 12:13:20.000000000","message":"I think we want to specify timeout explicitly when \"add\", so it looks good to control the default value in our code. We have different default value for priority.","commit_id":"3c00dd43f613c838f713a7cbf3cedb6767a8c52a"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"d8655b6a501d7fab488fd8569969a70f293510c3","unresolved":false,"context_lines":[{"line_number":538,"context_line":"        flow_expr_arr.append(\"hard_timeout\u003d%s\" %"},{"line_number":539,"context_line":"                             flow_dict.pop(\u0027hard_timeout\u0027, \u00270\u0027))"},{"line_number":540,"context_line":"        flow_expr_arr.append(\"idle_timeout\u003d%s\" %"},{"line_number":541,"context_line":"                             flow_dict.pop(\u0027idle_timeout\u0027, \u00270\u0027))"},{"line_number":542,"context_line":"        flow_expr_arr.append(\"priority\u003d%s\" %"},{"line_number":543,"context_line":"                             flow_dict.pop(\u0027priority\u0027, \u00271\u0027))"},{"line_number":544,"context_line":"    elif \u0027priority\u0027 in flow_dict:"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAWH%2F%2FF6A%3D","line":541,"in_reply_to":"AAAAWH%2F%2FGoY%3D","updated":"2014-03-20 10:25:48.000000000","message":"Good question, and actually I was thinking about it. I have 2 arguments for keeping this code:\n1. Alternative code won\u0027t be very pretty, I see something like\n\"flow_expr_arr.append(\u0027hard_timeout\u0027 in flow_dict and \"hard_timeout\u003d%s\" % kwargs[\u0027hard_timeout\u0027] or \u0027\u0027)\".\n2. Explicit is better than implicit, and I think alternative solution will raise some debates also.","commit_id":"3c00dd43f613c838f713a7cbf3cedb6767a8c52a"}],"neutron/tests/unit/openvswitch/test_ovs_lib.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"5fd8c65f51ad2efad4223a6aa80a485d5e038500","unresolved":false,"context_lines":[{"line_number":198,"context_line":"                      process_input\u003dNone, root_helper\u003dself.root_helper),"},{"line_number":199,"context_line":"            mock.call([\"ovs-ofctl\", \"add-flow\", self.BR_NAME,"},{"line_number":200,"context_line":"                       \"hard_timeout\u003d0,idle_timeout\u003d0,\""},{"line_number":201,"context_line":"                       \"priority\u003d4,dl_vlan\u003d%s,in_port\u003d%s,\""},{"line_number":202,"context_line":"                       \"actions\u003dstrip_vlan,set_tunnel:%s,normal\""},{"line_number":203,"context_line":"                       % (vid, ofport, lsw_id)],"},{"line_number":204,"context_line":"                      process_input\u003dNone, root_helper\u003dself.root_helper),"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BZNM%3D","line":201,"updated":"2013-11-26 15:32:25.000000000","message":"This match is very fragile. The order of parameters are no longer ensured because you are using flow_dict.iteritems. We need to use more robust way to check the parameter.","commit_id":"c6e9bd6601a867eb8ea7a32b4f6e23d8384167c7"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"ae7555576e9035d1905ee2fb7f9f2a140cedf551","unresolved":false,"context_lines":[{"line_number":18,"context_line":"try:"},{"line_number":19,"context_line":"    from collections import OrderedDict"},{"line_number":20,"context_line":"except ImportError:"},{"line_number":21,"context_line":"    from ordereddict import OrderedDict"},{"line_number":22,"context_line":"import mock"},{"line_number":23,"context_line":"import testtools"},{"line_number":24,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2BPJY%3D","line":21,"updated":"2013-11-28 13:55:17.000000000","message":"Seems a legit way of handling 2.6 compatibility","commit_id":"7945bc683741e7f413e482a96077309644c6c63c"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"6ef6cf7b9870bbb29bc247ec70e2fed93ab4762f","unresolved":false,"context_lines":[{"line_number":313,"context_line":"        self.br.defer_apply_on()"},{"line_number":314,"context_line":"        self.br.add_flow(flow\u003d\u0027add_flow_2\u0027)"},{"line_number":315,"context_line":"        self.br.delete_flows(flow\u003d\u0027delete_flow_1\u0027)"},{"line_number":316,"context_line":"        self.br.defer_apply_off()"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"        flow_expr.assert_has_calls(["},{"line_number":319,"context_line":"            mock.call({\u0027flow\u0027: \u0027add_flow_1\u0027}, action\u003d\u0027add\u0027),"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAT3%2F%2F5gA%3D","line":316,"updated":"2013-12-27 15:16:36.000000000","message":"I know that this is not really part of the bug(s) but it would be really nice to have mod_flow covered in this defer test.","commit_id":"0aa928e16e7f1a713d1b5dbcd23424031108155c"},{"author":{"_account_id":9069,"name":"Aleks Chirko","email":"achirko@mirantis.com","username":"achirko"},"change_message_id":"f0b40bcd24954c36904592f5469807505910960f","unresolved":false,"context_lines":[{"line_number":313,"context_line":"        self.br.defer_apply_on()"},{"line_number":314,"context_line":"        self.br.add_flow(flow\u003d\u0027add_flow_2\u0027)"},{"line_number":315,"context_line":"        self.br.delete_flows(flow\u003d\u0027delete_flow_1\u0027)"},{"line_number":316,"context_line":"        self.br.defer_apply_off()"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"        flow_expr.assert_has_calls(["},{"line_number":319,"context_line":"            mock.call({\u0027flow\u0027: \u0027add_flow_1\u0027}, action\u003d\u0027add\u0027),"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAT3%2F%2F5ag%3D","line":316,"in_reply_to":"AAAAT3%2F%2F5gA%3D","updated":"2013-12-27 15:48:15.000000000","message":"I\u0027ll do that if I\u0027d have to upload another patch set, but if I wouldn\u0027t I really don\u0027t want to go through another Jenkins run - it\u0027s so fragile, it takes a while to have clean run.","commit_id":"0aa928e16e7f1a713d1b5dbcd23424031108155c"}]}