)]}'
{"neutron/agent/ovsdb/api.py":[{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"fe5368fc622e0bc46e4ee73eb9ac8816e6d2cf13","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        If key already exists in a map column, then the current value is not"},{"line_number":219,"context_line":"        replaced (use the set command to replace an existing value)."},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"        :param table:  The OVS table containing the cord to be modified"},{"line_number":222,"context_line":"        :type table:   string"},{"line_number":223,"context_line":"        :param record: The record id (name/uuid) to modified"},{"line_number":224,"context_line":"        :type record:  string"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa6399be_8e095698","line":221,"range":{"start_line":221,"start_character":52,"end_line":221,"end_character":56},"updated":"2016-10-11 21:47:37.000000000","message":"record?","commit_id":"368c11e7c5e0b5efcaebb1954f5eaf884ac24aaa"}],"neutron/agent/ovsdb/native/commands.py":[{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"1c51990afd7d4bb11848a41fa38da05ddbef3a75","unresolved":false,"context_lines":[{"line_number":223,"context_line":"                # already set, do nothing, otherwise set the key to the value"},{"line_number":224,"context_line":"                for k, v in six.iteritems(value):"},{"line_number":225,"context_line":"                    if k in field:"},{"line_number":226,"context_line":"                        continue"},{"line_number":227,"context_line":"                    field[k] \u003d v"},{"line_number":228,"context_line":"            else:"},{"line_number":229,"context_line":"                # We should be appending to a \u0027set\u0027 column."}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_2c1d2766","line":226,"range":{"start_line":226,"start_character":24,"end_line":226,"end_character":32},"updated":"2016-10-14 14:57:55.000000000","message":"I think we should raise here or at least log something. I am sure in future somebody would use this method expecting it to change the value of an existing key, even if in the documentation you stated clearly that the behavior is different.","commit_id":"135a3fdc17336c166cadabbc629539620e245607"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"0e45dbc22b16577eb74920a891176229cda163b4","unresolved":false,"context_lines":[{"line_number":223,"context_line":"                # already set, do nothing, otherwise set the key to the value"},{"line_number":224,"context_line":"                for k, v in six.iteritems(value):"},{"line_number":225,"context_line":"                    if k in field:"},{"line_number":226,"context_line":"                        continue"},{"line_number":227,"context_line":"                    field[k] \u003d v"},{"line_number":228,"context_line":"            else:"},{"line_number":229,"context_line":"                # We should be appending to a \u0027set\u0027 column."}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_3c6c769b","line":226,"range":{"start_line":226,"start_character":24,"end_line":226,"end_character":32},"in_reply_to":"fa6399be_19a26499","updated":"2016-10-14 16:02:28.000000000","message":"Also, on the logging, I don\u0027t think there is a way to do it in the vsctl interface. We just pass values to ovs-vsctl add and it will do what ovs-vsctl does. There won\u0027t be any information with which to log.","commit_id":"135a3fdc17336c166cadabbc629539620e245607"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"ba954628eb21736ff8e3eac8d1d07b1aa44e9d9a","unresolved":false,"context_lines":[{"line_number":223,"context_line":"                # already set, do nothing, otherwise set the key to the value"},{"line_number":224,"context_line":"                for k, v in six.iteritems(value):"},{"line_number":225,"context_line":"                    if k in field:"},{"line_number":226,"context_line":"                        continue"},{"line_number":227,"context_line":"                    field[k] \u003d v"},{"line_number":228,"context_line":"            else:"},{"line_number":229,"context_line":"                # We should be appending to a \u0027set\u0027 column."}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_19a26499","line":226,"range":{"start_line":226,"start_character":24,"end_line":226,"end_character":32},"in_reply_to":"fa6399be_2c1d2766","updated":"2016-10-14 15:52:18.000000000","message":"I\u0027m not a fan of raising for expected behavior. The design has always been to follow the ovs-vsctl commands as closely as possible in behavior to avoid any confusion. This is just how ovs-vsctl\u0027s add() works. Someone looking to set the value would naturally use the set() command.\n\nI guess I\u0027d be okay with a DEBUG-level log here \"just in case\", but I\u0027m very against raising. You shouldn\u0027t have to catch an exception when using add to do specifically what add is meant to do.","commit_id":"135a3fdc17336c166cadabbc629539620e245607"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"0c16299c6d7f7e2b92cd80d95f76817c1f9c6371","unresolved":false,"context_lines":[{"line_number":223,"context_line":"                # already set, do nothing, otherwise set the key to the value"},{"line_number":224,"context_line":"                for k, v in six.iteritems(value):"},{"line_number":225,"context_line":"                    if k in field:"},{"line_number":226,"context_line":"                        continue"},{"line_number":227,"context_line":"                    field[k] \u003d v"},{"line_number":228,"context_line":"            else:"},{"line_number":229,"context_line":"                # We should be appending to a \u0027set\u0027 column."}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_52fe71ae","line":226,"range":{"start_line":226,"start_character":24,"end_line":226,"end_character":32},"in_reply_to":"fa6399be_3c6c769b","updated":"2016-10-14 16:53:03.000000000","message":"good points, I have to agree with you!","commit_id":"135a3fdc17336c166cadabbc629539620e245607"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"d706bf2af5c735dc8339d26e3e2537975a0d46eb","unresolved":false,"context_lines":[{"line_number":222,"context_line":"                # We should be doing an add on a \u0027map\u0027 column. If the key is"},{"line_number":223,"context_line":"                # already set, do nothing, otherwise set the key to the value"},{"line_number":224,"context_line":"                for k, v in six.iteritems(value):"},{"line_number":225,"context_line":"                    if k in field:"},{"line_number":226,"context_line":"                        continue"},{"line_number":227,"context_line":"                    field[k] \u003d v"},{"line_number":228,"context_line":"            else:"},{"line_number":229,"context_line":"                # We should be appending to a \u0027set\u0027 column."},{"line_number":230,"context_line":"                field.append(value)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_c5922e4b","line":227,"range":{"start_line":225,"start_character":0,"end_line":227,"end_character":32},"updated":"2016-10-12 13:49:13.000000000","message":"Nit: I find this more readable:\n\n if k not in field:\n    field[k] \u003d v","commit_id":"135a3fdc17336c166cadabbc629539620e245607"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"82056c2adc5543b10cb0deceac5474c0c89d4772","unresolved":false,"context_lines":[{"line_number":222,"context_line":"                # We should be doing an add on a \u0027map\u0027 column. If the key is"},{"line_number":223,"context_line":"                # already set, do nothing, otherwise set the key to the value"},{"line_number":224,"context_line":"                for k, v in six.iteritems(value):"},{"line_number":225,"context_line":"                    if k in field:"},{"line_number":226,"context_line":"                        continue"},{"line_number":227,"context_line":"                    field[k] \u003d v"},{"line_number":228,"context_line":"            else:"},{"line_number":229,"context_line":"                # We should be appending to a \u0027set\u0027 column."},{"line_number":230,"context_line":"                field.append(value)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_c4ae6997","line":227,"range":{"start_line":225,"start_character":0,"end_line":227,"end_character":32},"in_reply_to":"fa6399be_1a100401","updated":"2016-10-13 19:09:13.000000000","message":"Although, it occurs to me that I could just replace the whole loop by thinking in terms of value being updated with the original field values, like: \n\n value.update(field)\n field \u003d value","commit_id":"135a3fdc17336c166cadabbc629539620e245607"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"8c076181284321101ca325cc7378f8a3a69aead4","unresolved":false,"context_lines":[{"line_number":222,"context_line":"                # We should be doing an add on a \u0027map\u0027 column. If the key is"},{"line_number":223,"context_line":"                # already set, do nothing, otherwise set the key to the value"},{"line_number":224,"context_line":"                for k, v in six.iteritems(value):"},{"line_number":225,"context_line":"                    if k in field:"},{"line_number":226,"context_line":"                        continue"},{"line_number":227,"context_line":"                    field[k] \u003d v"},{"line_number":228,"context_line":"            else:"},{"line_number":229,"context_line":"                # We should be appending to a \u0027set\u0027 column."},{"line_number":230,"context_line":"                field.append(value)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_1a100401","line":227,"range":{"start_line":225,"start_character":0,"end_line":227,"end_character":32},"in_reply_to":"fa6399be_c5922e4b","updated":"2016-10-12 14:06:05.000000000","message":"Old habits from last job\u0027s C coding guidelines. :) Basically, in the style I used, if code would be added to the function later, the majority of the code would be in the section with less indention and therefor be easier to read. Of course, no code is likely to be added here. :p","commit_id":"135a3fdc17336c166cadabbc629539620e245607"}],"neutron/tests/functional/agent/test_ovs_lib.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"d706bf2af5c735dc8339d26e3e2537975a0d46eb","unresolved":false,"context_lines":[{"line_number":367,"context_line":"                             data).execute(check_error\u003dTrue)"},{"line_number":368,"context_line":"        self.assertEqual(data, self.br.db_get_val(\u0027Bridge\u0027, self.br.br_name,"},{"line_number":369,"context_line":"                                                  \u0027external_ids\u0027))"},{"line_number":370,"context_line":"        self.br.ovsdb.db_add(\"Bridge\", self.br.br_name, \"external_ids\","},{"line_number":371,"context_line":"                             {key: \"newdata\"}).execute(check_error\u003dTrue)"},{"line_number":372,"context_line":"        self.assertEqual(data, self.br.db_get_val(\u0027Bridge\u0027, self.br.br_name,"},{"line_number":373,"context_line":"                                                  \u0027external_ids\u0027))"},{"line_number":374,"context_line":""},{"line_number":375,"context_line":"    def test_db_add_map_multiple_one_dict(self):"},{"line_number":376,"context_line":"        data \u003d {\"one\": \"1\", \"two\": \"2\", \"three\": \"3\"}"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_d782e551","line":373,"range":{"start_line":370,"start_character":0,"end_line":373,"end_character":66},"updated":"2016-10-12 13:49:13.000000000","message":"Maybe we should add a comment here that existing keys are not overwritten. That makes me wonder what if I want to update the value?","commit_id":"135a3fdc17336c166cadabbc629539620e245607"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"ba954628eb21736ff8e3eac8d1d07b1aa44e9d9a","unresolved":false,"context_lines":[{"line_number":367,"context_line":"                             data).execute(check_error\u003dTrue)"},{"line_number":368,"context_line":"        self.assertEqual(data, self.br.db_get_val(\u0027Bridge\u0027, self.br.br_name,"},{"line_number":369,"context_line":"                                                  \u0027external_ids\u0027))"},{"line_number":370,"context_line":"        self.br.ovsdb.db_add(\"Bridge\", self.br.br_name, \"external_ids\","},{"line_number":371,"context_line":"                             {key: \"newdata\"}).execute(check_error\u003dTrue)"},{"line_number":372,"context_line":"        self.assertEqual(data, self.br.db_get_val(\u0027Bridge\u0027, self.br.br_name,"},{"line_number":373,"context_line":"                                                  \u0027external_ids\u0027))"},{"line_number":374,"context_line":""},{"line_number":375,"context_line":"    def test_db_add_map_multiple_one_dict(self):"},{"line_number":376,"context_line":"        data \u003d {\"one\": \"1\", \"two\": \"2\", \"three\": \"3\"}"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_b9afb87b","line":373,"range":{"start_line":370,"start_character":0,"end_line":373,"end_character":66},"in_reply_to":"fa6399be_8ca1d3fa","updated":"2016-10-14 15:52:18.000000000","message":"The reviewers in this case pretty much *are* the users. :) I cannot condone raising as a solution. You\u0027d have to use a try/except every time you used the function for it\u0027s intended purpose for a map. That\u0027s just not ok.","commit_id":"135a3fdc17336c166cadabbc629539620e245607"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"e8ecc4c542867cc0d401466c513ffd0e581a4d74","unresolved":false,"context_lines":[{"line_number":367,"context_line":"                             data).execute(check_error\u003dTrue)"},{"line_number":368,"context_line":"        self.assertEqual(data, self.br.db_get_val(\u0027Bridge\u0027, self.br.br_name,"},{"line_number":369,"context_line":"                                                  \u0027external_ids\u0027))"},{"line_number":370,"context_line":"        self.br.ovsdb.db_add(\"Bridge\", self.br.br_name, \"external_ids\","},{"line_number":371,"context_line":"                             {key: \"newdata\"}).execute(check_error\u003dTrue)"},{"line_number":372,"context_line":"        self.assertEqual(data, self.br.db_get_val(\u0027Bridge\u0027, self.br.br_name,"},{"line_number":373,"context_line":"                                                  \u0027external_ids\u0027))"},{"line_number":374,"context_line":""},{"line_number":375,"context_line":"    def test_db_add_map_multiple_one_dict(self):"},{"line_number":376,"context_line":"        data \u003d {\"one\": \"1\", \"two\": \"2\", \"three\": \"3\"}"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_f72fe93b","line":373,"range":{"start_line":370,"start_character":0,"end_line":373,"end_character":66},"in_reply_to":"fa6399be_d782e551","updated":"2016-10-12 13:49:56.000000000","message":"Ok, found my answer here :) https://review.openstack.org/#/c/385087/5/neutron/agent/ovsdb/api.py@219","commit_id":"135a3fdc17336c166cadabbc629539620e245607"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"1c51990afd7d4bb11848a41fa38da05ddbef3a75","unresolved":false,"context_lines":[{"line_number":367,"context_line":"                             data).execute(check_error\u003dTrue)"},{"line_number":368,"context_line":"        self.assertEqual(data, self.br.db_get_val(\u0027Bridge\u0027, self.br.br_name,"},{"line_number":369,"context_line":"                                                  \u0027external_ids\u0027))"},{"line_number":370,"context_line":"        self.br.ovsdb.db_add(\"Bridge\", self.br.br_name, \"external_ids\","},{"line_number":371,"context_line":"                             {key: \"newdata\"}).execute(check_error\u003dTrue)"},{"line_number":372,"context_line":"        self.assertEqual(data, self.br.db_get_val(\u0027Bridge\u0027, self.br.br_name,"},{"line_number":373,"context_line":"                                                  \u0027external_ids\u0027))"},{"line_number":374,"context_line":""},{"line_number":375,"context_line":"    def test_db_add_map_multiple_one_dict(self):"},{"line_number":376,"context_line":"        data \u003d {\"one\": \"1\", \"two\": \"2\", \"three\": \"3\"}"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_8ca1d3fa","line":373,"range":{"start_line":370,"start_character":0,"end_line":373,"end_character":66},"in_reply_to":"fa6399be_f72fe93b","updated":"2016-10-14 14:57:55.000000000","message":"This makes me think that we really need something more visible.  If reviewers have some troubles finding it, imagine users :D. Let\u0027s raise and specify that to change the value, set command should be used.","commit_id":"135a3fdc17336c166cadabbc629539620e245607"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"d706bf2af5c735dc8339d26e3e2537975a0d46eb","unresolved":false,"context_lines":[{"line_number":392,"context_line":"        ovsdb \u003d self.ovs.ovsdb"},{"line_number":393,"context_line":"        brname \u003d utils.get_rand_name(prefix\u003dnet_helpers.BR_PREFIX)"},{"line_number":394,"context_line":"        br \u003d ovs_lib.OVSBridge(brname)  # doesn\u0027t create"},{"line_number":395,"context_line":"        self.addCleanup(br.destroy)"},{"line_number":396,"context_line":"        with ovsdb.transaction(check_error\u003dTrue) as txn:"},{"line_number":397,"context_line":"            br \u003d txn.add(ovsdb.db_create(\u0027Bridge\u0027, name\u003dbrname))"},{"line_number":398,"context_line":"            txn.add(ovsdb.db_add(\u0027Open_vSwitch\u0027, \u0027.\u0027, \u0027bridges\u0027, br))"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_776e7997","line":395,"updated":"2016-10-12 13:49:13.000000000","message":"Nit: Shouldn\u0027t this be on L399? If transaction below fails, we\u0027ll get another failure on destroy in cleanup.","commit_id":"135a3fdc17336c166cadabbc629539620e245607"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"8c076181284321101ca325cc7378f8a3a69aead4","unresolved":false,"context_lines":[{"line_number":392,"context_line":"        ovsdb \u003d self.ovs.ovsdb"},{"line_number":393,"context_line":"        brname \u003d utils.get_rand_name(prefix\u003dnet_helpers.BR_PREFIX)"},{"line_number":394,"context_line":"        br \u003d ovs_lib.OVSBridge(brname)  # doesn\u0027t create"},{"line_number":395,"context_line":"        self.addCleanup(br.destroy)"},{"line_number":396,"context_line":"        with ovsdb.transaction(check_error\u003dTrue) as txn:"},{"line_number":397,"context_line":"            br \u003d txn.add(ovsdb.db_create(\u0027Bridge\u0027, name\u003dbrname))"},{"line_number":398,"context_line":"            txn.add(ovsdb.db_add(\u0027Open_vSwitch\u0027, \u0027.\u0027, \u0027bridges\u0027, br))"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_b7d731ec","line":395,"in_reply_to":"fa6399be_776e7997","updated":"2016-10-12 14:06:05.000000000","message":"destroy calls del_br() which defaults to if_exists\u003dTrue, so it shouldn\u0027t cause an error. My thinking was that If we did it after and there was an exception, we\u0027d never clean up.","commit_id":"135a3fdc17336c166cadabbc629539620e245607"}]}