)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"040237f6315d9eb135da1003d31339998f435c34","unresolved":false,"context_lines":[{"line_number":14,"context_line":"list comparison support."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: Ia93fb925b8814210975904a453249f15f3646855"},{"line_number":17,"context_line":"Closes-bug: #1578233"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1a122d0e_34be57ae","line":17,"range":{"start_line":17,"start_character":0,"end_line":17,"end_character":20},"updated":"2016-05-05 10:01:40.000000000","message":"Does this bug serve only to come with workaround? I though we should rather implement the comparison in ovsdb native interface with proper functional tests.\n\nIf we want to fix only functional tests failure I think https://review.openstack.org/#/c/312670/1/neutron/tests/functional/agent/test_firewall.py@149 is a better workaround that doesn\u0027t pollute ovsdb interface production code.","commit_id":"6cd0eaba3c5153db0d7714544883d6532692a423"}],"neutron/agent/ovsdb/native/idlutils.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"408a1a15eda93a91e9cad605c93284e70de59090","unresolved":false,"context_lines":[{"line_number":126,"context_line":"            raise Exception(_(\"Timeout\"))"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"def get_column_value(row, col, vsctl_list\u003dTrue):"},{"line_number":130,"context_line":"    if col \u003d\u003d \u0027_uuid\u0027:"},{"line_number":131,"context_line":"        val \u003d row.uuid"},{"line_number":132,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_76f8334a","line":129,"range":{"start_line":129,"start_character":31,"end_line":129,"end_character":40},"updated":"2016-05-09 11:17:57.000000000","message":"What does this mean? The function is used only for native interface. Should we treat \"[1]\" as \"1\"?","commit_id":"737d92bf87eb23bc60d3c44a3dadcc1fb9f82633"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"5e1ffd0bdedaee80e5bffcafcf91c8b08b6e01e6","unresolved":false,"context_lines":[{"line_number":126,"context_line":"            raise Exception(_(\"Timeout\"))"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"def get_column_value(row, col, vsctl_list\u003dTrue):"},{"line_number":130,"context_line":"    if col \u003d\u003d \u0027_uuid\u0027:"},{"line_number":131,"context_line":"        val \u003d row.uuid"},{"line_number":132,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_e9dc0775","line":129,"range":{"start_line":129,"start_character":31,"end_line":129,"end_character":40},"in_reply_to":"dab17558_18e0bb28","updated":"2016-05-10 11:39:56.000000000","message":"No, I think it\u0027s fine to have this parameter. I am just confused with it. I don\u0027t get in which case it should be used with \"False\" as the function is always called from native.","commit_id":"737d92bf87eb23bc60d3c44a3dadcc1fb9f82633"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"315e5a9e755e10576ae86762e7cc732bc6d679cf","unresolved":false,"context_lines":[{"line_number":126,"context_line":"            raise Exception(_(\"Timeout\"))"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"def get_column_value(row, col, vsctl_list\u003dTrue):"},{"line_number":130,"context_line":"    if col \u003d\u003d \u0027_uuid\u0027:"},{"line_number":131,"context_line":"        val \u003d row.uuid"},{"line_number":132,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_18e0bb28","line":129,"range":{"start_line":129,"start_character":31,"end_line":129,"end_character":40},"in_reply_to":"dab17558_76f8334a","updated":"2016-05-10 11:00:26.000000000","message":"The function is also used in a couple of places in the commands module (https://github.com/openstack/neutron/blob/master/neutron/agent/ovsdb/native/commands.py#L225), twilson has written an explicit comment about its behavior.\nI think I know a way to get around this using ovs library\u0027s methods, but I didn\u0027t want to introduce too much changes in the patch so I though a parameter is a better option.\n\nDo you think I should get rid of the optional parameter and refactor commands module as well?","commit_id":"737d92bf87eb23bc60d3c44a3dadcc1fb9f82633"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"87dae3b0800dde679cf2255de7f9e3e7ccfb47a0","unresolved":false,"context_lines":[{"line_number":126,"context_line":"            raise Exception(_(\"Timeout\"))"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"def get_column_value(row, col, vsctl_list\u003dTrue):"},{"line_number":130,"context_line":"    if col \u003d\u003d \u0027_uuid\u0027:"},{"line_number":131,"context_line":"        val \u003d row.uuid"},{"line_number":132,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_84060284","line":129,"range":{"start_line":129,"start_character":31,"end_line":129,"end_character":40},"in_reply_to":"dab17558_e9dc0775","updated":"2016-05-10 12:49:27.000000000","message":"I introduced the parameter to retrieve precisely the value stored in the db without vsctl\u0027s \"one-element-list to element\" transformation (see line 201, here it\u0027s used with vsctl_list\u003dFalse). Imo that makes comparison a lot clearer as long as types are always met (a list is compared to a list, not single-element-that-is-a-list-actually to a list).","commit_id":"737d92bf87eb23bc60d3c44a3dadcc1fb9f82633"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"408a1a15eda93a91e9cad605c93284e70de59090","unresolved":false,"context_lines":[{"line_number":142,"context_line":"    return val"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"def convert_to_column_type(row, col, value):"},{"line_number":146,"context_line":"    \"\"\"Convert condition value to column type."},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    * lists of string numbers will be coverted to lists of integers"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_f638a33f","line":145,"range":{"start_line":145,"start_character":4,"end_line":145,"end_character":26},"updated":"2016-05-09 11:17:57.000000000","message":"Can you please also add unit test for this function?","commit_id":"737d92bf87eb23bc60d3c44a3dadcc1fb9f82633"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"315e5a9e755e10576ae86762e7cc732bc6d679cf","unresolved":false,"context_lines":[{"line_number":142,"context_line":"    return val"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"def convert_to_column_type(row, col, value):"},{"line_number":146,"context_line":"    \"\"\"Convert condition value to column type."},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    * lists of string numbers will be coverted to lists of integers"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_981aeb7b","line":145,"range":{"start_line":145,"start_character":4,"end_line":145,"end_character":26},"in_reply_to":"dab17558_f638a33f","updated":"2016-05-10 11:00:26.000000000","message":"Ok, will be done.","commit_id":"737d92bf87eb23bc60d3c44a3dadcc1fb9f82633"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"408a1a15eda93a91e9cad605c93284e70de59090","unresolved":false,"context_lines":[{"line_number":175,"context_line":"            val \u003d [val]"},{"line_number":176,"context_line":"        return val"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    if isinstance(value, str):"},{"line_number":179,"context_line":"        try:"},{"line_number":180,"context_line":"            obj \u003d jsonutils.loads(value)"},{"line_number":181,"context_line":"            return do_convert(obj)"},{"line_number":182,"context_line":"            # try converting values to column type\u0027s python_types"},{"line_number":183,"context_line":"        except ValueError:"},{"line_number":184,"context_line":"            # non-valid json, return as is"},{"line_number":185,"context_line":"            return do_convert(value)"},{"line_number":186,"context_line":"    return do_convert(value)"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"def condition_match(row, condition):"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_b62e1bf8","line":186,"range":{"start_line":178,"start_character":0,"end_line":186,"end_character":28},"updated":"2016-05-09 11:17:57.000000000","message":"Nit: IMHO read better:\n\n if isinstance(value, str):\n     try:\n         value \u003d jsonutils.loads(value)\n     except ValueError:\n         pass\n return do_convert(value)","commit_id":"737d92bf87eb23bc60d3c44a3dadcc1fb9f82633"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"315e5a9e755e10576ae86762e7cc732bc6d679cf","unresolved":false,"context_lines":[{"line_number":175,"context_line":"            val \u003d [val]"},{"line_number":176,"context_line":"        return val"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    if isinstance(value, str):"},{"line_number":179,"context_line":"        try:"},{"line_number":180,"context_line":"            obj \u003d jsonutils.loads(value)"},{"line_number":181,"context_line":"            return do_convert(obj)"},{"line_number":182,"context_line":"            # try converting values to column type\u0027s python_types"},{"line_number":183,"context_line":"        except ValueError:"},{"line_number":184,"context_line":"            # non-valid json, return as is"},{"line_number":185,"context_line":"            return do_convert(value)"},{"line_number":186,"context_line":"    return do_convert(value)"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"def condition_match(row, condition):"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_b8152f4a","line":186,"range":{"start_line":178,"start_character":0,"end_line":186,"end_character":28},"in_reply_to":"dab17558_b62e1bf8","updated":"2016-05-10 11:00:26.000000000","message":"Thanks, nice catch!","commit_id":"737d92bf87eb23bc60d3c44a3dadcc1fb9f82633"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"7b50a92a89e466a69ac13902bf072ca3d3c7a680","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        if isinstance(val[0], idl.Row):"},{"line_number":138,"context_line":"            val \u003d [v.uuid for v in val]"},{"line_number":139,"context_line":"        # ovs-vsctl treats lists of 1 as single results"},{"line_number":140,"context_line":"        if len(val) \u003d\u003d 1 and vsctl_list:"},{"line_number":141,"context_line":"            val \u003d val[0]"},{"line_number":142,"context_line":"    return val"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"dab17558_ee6b3b1c","line":140,"range":{"start_line":140,"start_character":29,"end_line":140,"end_character":39},"updated":"2016-05-11 08:57:33.000000000","message":"the meaning of vsctl_list argument is counter-intuitive to me.","commit_id":"721406532a177c5820a65119c5c8c247f098faa1"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"663cd653b01161f2f11534958b6e35b7f4026a33","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        if isinstance(val[0], idl.Row):"},{"line_number":138,"context_line":"            val \u003d [v.uuid for v in val]"},{"line_number":139,"context_line":"        # ovs-vsctl treats lists of 1 as single results"},{"line_number":140,"context_line":"        if len(val) \u003d\u003d 1 and vsctl_list:"},{"line_number":141,"context_line":"            val \u003d val[0]"},{"line_number":142,"context_line":"    return val"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"dab17558_d8f27c05","line":140,"range":{"start_line":140,"start_character":29,"end_line":140,"end_character":39},"in_reply_to":"dab17558_ee6b3b1c","updated":"2016-05-11 09:18:06.000000000","message":"Any suggestions? I could not come up with a better idea than \"no_single_elem_list\", but it looks too long and not too much clearer :(","commit_id":"721406532a177c5820a65119c5c8c247f098faa1"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"f16099d83c9271257df55067e3905feb8700e964","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        if isinstance(val[0], idl.Row):"},{"line_number":138,"context_line":"            val \u003d [v.uuid for v in val]"},{"line_number":139,"context_line":"        # ovs-vsctl treats lists of 1 as single results"},{"line_number":140,"context_line":"        if len(val) \u003d\u003d 1 and not len_1_lists:"},{"line_number":141,"context_line":"            val \u003d val[0]"},{"line_number":142,"context_line":"    return val"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"dab17558_5fb3324d","line":140,"updated":"2016-05-16 17:12:06.000000000","message":"This can be inspected from the schema. Basically we are looking for \u0027optional\u0027 items which have a min: 0, max: 1. Those, ovs-vsctl will return as a single item and not a list.\n\nBasically row._table.columns[col].type.n_min \u003d\u003d 0 and n_max \u003d\u003d 1. And I just noticed that the ovs python library has an \u0027is_optional()\u0027 method on the type that checks exactly that.","commit_id":"5206f66a76c1712a25c7d2b78722d7a495f83fdd"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"6747728ba56f6055c832811f94cf7b765ba696cc","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        if isinstance(val[0], idl.Row):"},{"line_number":138,"context_line":"            val \u003d [v.uuid for v in val]"},{"line_number":139,"context_line":"        # ovs-vsctl treats lists of 1 as single results"},{"line_number":140,"context_line":"        if len(val) \u003d\u003d 1 and not len_1_lists:"},{"line_number":141,"context_line":"            val \u003d val[0]"},{"line_number":142,"context_line":"    return val"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"dab17558_a1de45d7","line":140,"in_reply_to":"dab17558_5fb3324d","updated":"2016-05-17 16:11:03.000000000","message":"Yes, you are right here. Thanks!","commit_id":"5206f66a76c1712a25c7d2b78722d7a495f83fdd"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"81b1b1e62daf44d0055c56c0ee59b786e8fd1aa9","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        if isinstance(val[0], idl.Row):"},{"line_number":138,"context_line":"            val \u003d [v.uuid for v in val]"},{"line_number":139,"context_line":"        # ovs-vsctl treats lists of 1 as single results"},{"line_number":140,"context_line":"        if len(val) \u003d\u003d 1 and not len_1_lists:"},{"line_number":141,"context_line":"            val \u003d val[0]"},{"line_number":142,"context_line":"    return val"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"bab6814e_eb03e30b","line":140,"in_reply_to":"dab17558_a1de45d7","updated":"2016-05-18 10:05:08.000000000","message":"Sorry, on second thought I understood that I didn\u0027t get you right here last night. I do agree with is_optional() and its relation with vsctl 1-len-lists-to-single-element transformation, but \u0027len_1_lists\u0027 parameter was introduced here for a different reason. Its main purpose is to gain some control over transformation of type list to single element and suppress this transformation in condition_match method (L205).\n\nWithout this suppression some comparisons (like match\u003d[42], val\u003d[]) will be a real headache in the terms of type matching: \nget_column_value will return an integer value 42, and list comparison logic will have to deal with an integer and an empty list. \n\nI think we should honor types here and suppress any vsctl transformation at that point. That\u0027s what \u0027len_1_lists\u0027 parameter was actually about.","commit_id":"5206f66a76c1712a25c7d2b78722d7a495f83fdd"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"f16099d83c9271257df55067e3905feb8700e964","unresolved":false,"context_lines":[{"line_number":142,"context_line":"    return val"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"def convert_to_column_type(row, col, value):"},{"line_number":146,"context_line":"    \"\"\"Convert condition value to column type."},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    * lists of string numbers will be coverted to lists of integers"}],"source_content_type":"text/x-python","patch_set":9,"id":"dab17558_ba36ac00","line":145,"updated":"2016-05-16 17:12:06.000000000","message":"I don\u0027t like converting lists of string numbers to lists of integers or doing any type conversions at all.\n\nWe should be using the correct type when we query. When we use the vsctl implementation, we are getting the results as json, so we should always get results that are the correct type. We should always send the correct type too. I went to a lot of trouble to make sure that all of the types we send were correct. :)\n\nIn my opinion, we don\u0027t have to support all conversions that vsctl will possibly do from the command-line (and our performance will probably suck if we try to). Maybe we can just write tests that detect when someone writes code that does a query that would be of the wrong type.","commit_id":"5206f66a76c1712a25c7d2b78722d7a495f83fdd"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"6747728ba56f6055c832811f94cf7b765ba696cc","unresolved":false,"context_lines":[{"line_number":142,"context_line":"    return val"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"def convert_to_column_type(row, col, value):"},{"line_number":146,"context_line":"    \"\"\"Convert condition value to column type."},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    * lists of string numbers will be coverted to lists of integers"}],"source_content_type":"text/x-python","patch_set":9,"id":"dab17558_e1cb2d1c","line":145,"in_reply_to":"dab17558_ba36ac00","updated":"2016-05-17 16:11:03.000000000","message":"I get your point. One remark though - I think we have to keep json string -\u003e python obj convertion (L180-186). No types magic (I disliked that part actually but I thought that identical processing of vsctl and native is a way to go), but json deserialization. No objections here?","commit_id":"5206f66a76c1712a25c7d2b78722d7a495f83fdd"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"e38c57501c507e0280f364979a310522f1f907e7","unresolved":false,"context_lines":[{"line_number":128,"context_line":"def get_column_value(row, col):"},{"line_number":129,"context_line":"    \"\"\"Retrieves column value from the given row."},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    If strict_types parameter is set, then no transformations to received value"},{"line_number":132,"context_line":"    are performed. Otherwise if value is a list of length 1, it will be"},{"line_number":133,"context_line":"    transformed to a single value. The latter is the default behaviour."},{"line_number":134,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":11,"id":"bab6814e_0559008f","line":131,"updated":"2016-05-23 22:24:51.000000000","message":"strict_types parameter has been removed, so comment can be removed as well.","commit_id":"4b825fc6a1cfce0fddb14b3662d0103afa8d3329"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"5a4d150f0c10a06231cdd989d60e551f04fb6aa8","unresolved":false,"context_lines":[{"line_number":155,"context_line":"    :param condition: A 3-tuple containing (column, operation, match)"},{"line_number":156,"context_line":"    \"\"\""},{"line_number":157,"context_line":"    col, op, match \u003d condition"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    val \u003d get_column_value(row, col)"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"    # both match and val are primitive types, so type can be used for type"}],"source_content_type":"text/x-python","patch_set":11,"id":"bab6814e_86be0c63","line":158,"updated":"2016-05-20 09:36:26.000000000","message":"Unrelated change (newline). Will remove when collect some feedback to reduce number of patchsets.","commit_id":"4b825fc6a1cfce0fddb14b3662d0103afa8d3329"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"e38c57501c507e0280f364979a310522f1f907e7","unresolved":false,"context_lines":[{"line_number":172,"context_line":"            # TODO(ivasilevskaya) BTW, should we log it? Raising an exception"},{"line_number":173,"context_line":"            # seems a bit too harsh, but it would be polite to let people know"},{"line_number":174,"context_line":"            # where exactly they are wrong."},{"line_number":175,"context_line":"            return False"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    matched \u003d True"},{"line_number":178,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"bab6814e_e5b95c7b","line":175,"updated":"2016-05-23 22:24:51.000000000","message":"I\u0027d be Ok with raising a ValueError or similar. With that said, if we return False and have good functional tests, the test should fail if someone screws up.","commit_id":"4b825fc6a1cfce0fddb14b3662d0103afa8d3329"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"312a9807b86b681b03f0f6189b41c069fa877144","unresolved":false,"context_lines":[{"line_number":126,"context_line":""},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"def get_column_value(row, col):"},{"line_number":129,"context_line":"    \"\"\"Retrieves column value from the given row."},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    If column\u0027s type is optional, the value will be returned as a single"},{"line_number":132,"context_line":"    element instead of a list of length 1."}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_420a4227","line":129,"range":{"start_line":129,"start_character":7,"end_line":129,"end_character":16},"updated":"2016-05-26 12:36:31.000000000","message":"Nit: Retrieve","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"97c92072b2a12e798ea8647345678f7c4f43da7b","unresolved":false,"context_lines":[{"line_number":126,"context_line":""},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"def get_column_value(row, col):"},{"line_number":129,"context_line":"    \"\"\"Retrieves column value from the given row."},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    If column\u0027s type is optional, the value will be returned as a single"},{"line_number":132,"context_line":"    element instead of a list of length 1."}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_bb397bf4","line":129,"range":{"start_line":129,"start_character":7,"end_line":129,"end_character":16},"in_reply_to":"bab6814e_420a4227","updated":"2016-05-26 22:04:12.000000000","message":"Thanks!","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"312a9807b86b681b03f0f6189b41c069fa877144","unresolved":false,"context_lines":[{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    # both match and val are primitive types, so type can be used for type"},{"line_number":160,"context_line":"    # equality here."},{"line_number":161,"context_line":"    if not (type(match) is type(val)):"},{"line_number":162,"context_line":"        # Types of \u0027val\u0027 and \u0027match\u0027 arguments MUST match in all cases with 1"},{"line_number":163,"context_line":"        # exception: when \u0027match\u0027 is an empty list and column\u0027s type is"},{"line_number":164,"context_line":"        # optional."}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_653b10d2","line":161,"range":{"start_line":161,"start_character":7,"end_line":161,"end_character":37},"updated":"2016-05-26 12:36:31.000000000","message":"Nit: type(match) is not type(val)","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"97c92072b2a12e798ea8647345678f7c4f43da7b","unresolved":false,"context_lines":[{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    # both match and val are primitive types, so type can be used for type"},{"line_number":160,"context_line":"    # equality here."},{"line_number":161,"context_line":"    if not (type(match) is type(val)):"},{"line_number":162,"context_line":"        # Types of \u0027val\u0027 and \u0027match\u0027 arguments MUST match in all cases with 1"},{"line_number":163,"context_line":"        # exception: when \u0027match\u0027 is an empty list and column\u0027s type is"},{"line_number":164,"context_line":"        # optional."}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_db5b9f52","line":161,"range":{"start_line":161,"start_character":7,"end_line":161,"end_character":37},"in_reply_to":"bab6814e_653b10d2","updated":"2016-05-26 22:04:12.000000000","message":"Thanks, will correct this.","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"312a9807b86b681b03f0f6189b41c069fa877144","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        # Types of \u0027val\u0027 and \u0027match\u0027 arguments MUST match in all cases with 1"},{"line_number":163,"context_line":"        # exception: when \u0027match\u0027 is an empty list and column\u0027s type is"},{"line_number":164,"context_line":"        # optional."},{"line_number":165,"context_line":"        if match \u003d\u003d [] and row._table.columns[col].type.is_optional():"},{"line_number":166,"context_line":"            # utilize the single elements comparison logic"},{"line_number":167,"context_line":"            match \u003d None"},{"line_number":168,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_255588c1","line":165,"range":{"start_line":165,"start_character":11,"end_line":165,"end_character":22},"updated":"2016-05-26 12:36:31.000000000","message":"This instantiates a new empty list object, how about\n\n if isinstance(match, list) and match and row._table...\n\nit is more explicit and basically says the same what is written in the comment above, so then I think the comment is not even necessary.","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"a197880a6340e667c61e91592f0a5bd9f856dc59","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        # Types of \u0027val\u0027 and \u0027match\u0027 arguments MUST match in all cases with 1"},{"line_number":163,"context_line":"        # exception: when \u0027match\u0027 is an empty list and column\u0027s type is"},{"line_number":164,"context_line":"        # optional."},{"line_number":165,"context_line":"        if match \u003d\u003d [] and row._table.columns[col].type.is_optional():"},{"line_number":166,"context_line":"            # utilize the single elements comparison logic"},{"line_number":167,"context_line":"            match \u003d None"},{"line_number":168,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"9abb7d3a_d1db20ec","line":165,"range":{"start_line":165,"start_character":11,"end_line":165,"end_character":22},"in_reply_to":"bab6814e_029b0819","updated":"2016-05-27 17:50:19.000000000","message":"Ah, sorry. So I just introduced a slower solution that also contained bug :)","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"9899d2ba6b4266876a07bdb533bec9509ae055b4","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        # Types of \u0027val\u0027 and \u0027match\u0027 arguments MUST match in all cases with 1"},{"line_number":163,"context_line":"        # exception: when \u0027match\u0027 is an empty list and column\u0027s type is"},{"line_number":164,"context_line":"        # optional."},{"line_number":165,"context_line":"        if match \u003d\u003d [] and row._table.columns[col].type.is_optional():"},{"line_number":166,"context_line":"            # utilize the single elements comparison logic"},{"line_number":167,"context_line":"            match \u003d None"},{"line_number":168,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_029b0819","line":165,"range":{"start_line":165,"start_character":11,"end_line":165,"end_character":22},"in_reply_to":"bab6814e_255588c1","updated":"2016-05-26 16:59:16.000000000","message":"So\n\n if isinstance(match, list) and match and row._table...\n\nwould also match nonempty lists. Would need to be:\n\n if isinstance(match, list) and not match and ...\n\nbut even then:\n \u003e\u003e\u003e from timeit import timeit\n \u003e\u003e\u003e timeit(\"match \u003d\u003d []\", setup\u003d\"match \u003d []\")\n 0.052667856216430664\n \u003e\u003e\u003e timeit(\"isinstance(match, list) and not match\", setup\u003d\"match \u003d []\")\n 0.14286494255065918\n\ncreating a list is quite a bit faster.","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"c80483a401ecfb017c92dc9416c1aa49f7358cbf","unresolved":false,"context_lines":[{"line_number":189,"context_line":"    elif isinstance(match, list):"},{"line_number":190,"context_line":"        # According to rfc7047, lists support \u0027\u003d\u0027 and \u0027!\u003d\u0027"},{"line_number":191,"context_line":"        # (both strict and relaxed). Will follow twilson\u0027s dict comparison"},{"line_number":192,"context_line":"        # and implement relaxed version (excludes/includes as per standart)"},{"line_number":193,"context_line":"        if op \u003d\u003d \"\u003d\":"},{"line_number":194,"context_line":"            if any(len(x) \u003d\u003d 0 for x in [val, match]):"},{"line_number":195,"context_line":"                return val \u003d\u003d match"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_15897d26","line":192,"updated":"2016-05-26 17:47:31.000000000","message":"To be honest, I probably shouldn\u0027t have implemented includes/excludes as \u003d/!\u003d. Looking back, not sure what I was thinking there. I wouldn\u0027t be opposed to a patch changing that in the future. I\u0027m not sure if anything outside of neutron would break, though. :/","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"312a9807b86b681b03f0f6189b41c069fa877144","unresolved":false,"context_lines":[{"line_number":191,"context_line":"        # (both strict and relaxed). Will follow twilson\u0027s dict comparison"},{"line_number":192,"context_line":"        # and implement relaxed version (excludes/includes as per standart)"},{"line_number":193,"context_line":"        if op \u003d\u003d \"\u003d\":"},{"line_number":194,"context_line":"            if any(len(x) \u003d\u003d 0 for x in [val, match]):"},{"line_number":195,"context_line":"                return val \u003d\u003d match"},{"line_number":196,"context_line":"            for elem in set(match):"},{"line_number":197,"context_line":"                if elem not in val:"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_25ad8884","line":194,"range":{"start_line":194,"start_character":12,"end_line":194,"end_character":53},"updated":"2016-05-26 12:36:31.000000000","message":"Following should be enough:\n\n if not all([val, match]):","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"c80483a401ecfb017c92dc9416c1aa49f7358cbf","unresolved":false,"context_lines":[{"line_number":191,"context_line":"        # (both strict and relaxed). Will follow twilson\u0027s dict comparison"},{"line_number":192,"context_line":"        # and implement relaxed version (excludes/includes as per standart)"},{"line_number":193,"context_line":"        if op \u003d\u003d \"\u003d\":"},{"line_number":194,"context_line":"            if any(len(x) \u003d\u003d 0 for x in [val, match]):"},{"line_number":195,"context_line":"                return val \u003d\u003d match"},{"line_number":196,"context_line":"            for elem in set(match):"},{"line_number":197,"context_line":"                if elem not in val:"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_8366daea","line":194,"range":{"start_line":194,"start_character":12,"end_line":194,"end_character":53},"in_reply_to":"bab6814e_25ad8884","updated":"2016-05-26 17:47:31.000000000","message":"This also happens to be faster. Yay boolean algebra.\n\n \u003e\u003e\u003e timeit(\"any(len(x) \u003d\u003d 0 for x in [[1], []])\")\n 1.202779769897461\n \u003e\u003e\u003e timeit(\"not all([[1], []])\")\n 0.372236967086792","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"97c92072b2a12e798ea8647345678f7c4f43da7b","unresolved":false,"context_lines":[{"line_number":191,"context_line":"        # (both strict and relaxed). Will follow twilson\u0027s dict comparison"},{"line_number":192,"context_line":"        # and implement relaxed version (excludes/includes as per standart)"},{"line_number":193,"context_line":"        if op \u003d\u003d \"\u003d\":"},{"line_number":194,"context_line":"            if any(len(x) \u003d\u003d 0 for x in [val, match]):"},{"line_number":195,"context_line":"                return val \u003d\u003d match"},{"line_number":196,"context_line":"            for elem in set(match):"},{"line_number":197,"context_line":"                if elem not in val:"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_1b8d27d3","line":194,"range":{"start_line":194,"start_character":12,"end_line":194,"end_character":53},"in_reply_to":"bab6814e_8366daea","updated":"2016-05-26 22:04:12.000000000","message":"nearly 4x faster. Thanks a lot!","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"312a9807b86b681b03f0f6189b41c069fa877144","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        if op \u003d\u003d \"\u003d\":"},{"line_number":194,"context_line":"            if any(len(x) \u003d\u003d 0 for x in [val, match]):"},{"line_number":195,"context_line":"                return val \u003d\u003d match"},{"line_number":196,"context_line":"            for elem in set(match):"},{"line_number":197,"context_line":"                if elem not in val:"},{"line_number":198,"context_line":"                    matched \u003d False"},{"line_number":199,"context_line":"                    break"},{"line_number":200,"context_line":"        elif op \u003d\u003d \u0027!\u003d\u0027:"},{"line_number":201,"context_line":"            if any(len(x) \u003d\u003d 0 for x in [val, match]):"},{"line_number":202,"context_line":"                return val !\u003d match"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_a57e98ac","line":199,"range":{"start_line":196,"start_character":0,"end_line":199,"end_character":25},"updated":"2016-05-26 12:36:31.000000000","message":"Nit: I think the following will be faster\n\n match_set \u003d set(match)\n matched \u003d match_set \u003d\u003d match_set.intersection(set(val))","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"59307ed6431e3ca6df354f94167d477b41f34732","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        if op \u003d\u003d \"\u003d\":"},{"line_number":194,"context_line":"            if any(len(x) \u003d\u003d 0 for x in [val, match]):"},{"line_number":195,"context_line":"                return val \u003d\u003d match"},{"line_number":196,"context_line":"            for elem in set(match):"},{"line_number":197,"context_line":"                if elem not in val:"},{"line_number":198,"context_line":"                    matched \u003d False"},{"line_number":199,"context_line":"                    break"},{"line_number":200,"context_line":"        elif op \u003d\u003d \u0027!\u003d\u0027:"},{"line_number":201,"context_line":"            if any(len(x) \u003d\u003d 0 for x in [val, match]):"},{"line_number":202,"context_line":"                return val !\u003d match"}],"source_content_type":"text/x-python","patch_set":12,"id":"9abb7d3a_60c7fc12","line":199,"range":{"start_line":196,"start_character":0,"end_line":199,"end_character":25},"in_reply_to":"9abb7d3a_b16adc18","updated":"2016-05-27 20:15:28.000000000","message":"Huh. I\u0027d tried by throwing a single range(1000) in before originally posting and it didn\u0027t change much. I guess I wasn\u0027t thorough enough! I probably added the big range to the \u0027val\u0027 side and not \u0027match\u0027 since includes/excludes will almost always have match be a small list. I\u0027m happy with whatever Inessa chooses.","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"a197880a6340e667c61e91592f0a5bd9f856dc59","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        if op \u003d\u003d \"\u003d\":"},{"line_number":194,"context_line":"            if any(len(x) \u003d\u003d 0 for x in [val, match]):"},{"line_number":195,"context_line":"                return val \u003d\u003d match"},{"line_number":196,"context_line":"            for elem in set(match):"},{"line_number":197,"context_line":"                if elem not in val:"},{"line_number":198,"context_line":"                    matched \u003d False"},{"line_number":199,"context_line":"                    break"},{"line_number":200,"context_line":"        elif op \u003d\u003d \u0027!\u003d\u0027:"},{"line_number":201,"context_line":"            if any(len(x) \u003d\u003d 0 for x in [val, match]):"},{"line_number":202,"context_line":"                return val !\u003d match"}],"source_content_type":"text/x-python","patch_set":12,"id":"9abb7d3a_b16adc18","line":199,"range":{"start_line":196,"start_character":0,"end_line":199,"end_character":25},"in_reply_to":"bab6814e_290eec8b","updated":"2016-05-27 17:50:19.000000000","message":"Aha, I based the comment on the fact that sets have constant complexity while iteration has linear. This complexity is better measurable with bigger data: http://paste.openstack.org/show/505937/\n\na 5.70757603645\nb 0.828513145447\n\nNot sure though how it can influence the overall performance or my comments are just premature optimization.","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"c80483a401ecfb017c92dc9416c1aa49f7358cbf","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        if op \u003d\u003d \"\u003d\":"},{"line_number":194,"context_line":"            if any(len(x) \u003d\u003d 0 for x in [val, match]):"},{"line_number":195,"context_line":"                return val \u003d\u003d match"},{"line_number":196,"context_line":"            for elem in set(match):"},{"line_number":197,"context_line":"                if elem not in val:"},{"line_number":198,"context_line":"                    matched \u003d False"},{"line_number":199,"context_line":"                    break"},{"line_number":200,"context_line":"        elif op \u003d\u003d \u0027!\u003d\u0027:"},{"line_number":201,"context_line":"            if any(len(x) \u003d\u003d 0 for x in [val, match]):"},{"line_number":202,"context_line":"                return val !\u003d match"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_f54f998f","line":199,"range":{"start_line":196,"start_character":0,"end_line":199,"end_character":25},"in_reply_to":"bab6814e_a57e98ac","updated":"2016-05-26 17:47:31.000000000","message":"I wrote a little test: http://paste.openstack.org/show/505713/\n\nIt turned out that the original was faster.\n# orig: 5.79876589775\n# set: 7.50743508339","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"97c92072b2a12e798ea8647345678f7c4f43da7b","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        if op \u003d\u003d \"\u003d\":"},{"line_number":194,"context_line":"            if any(len(x) \u003d\u003d 0 for x in [val, match]):"},{"line_number":195,"context_line":"                return val \u003d\u003d match"},{"line_number":196,"context_line":"            for elem in set(match):"},{"line_number":197,"context_line":"                if elem not in val:"},{"line_number":198,"context_line":"                    matched \u003d False"},{"line_number":199,"context_line":"                    break"},{"line_number":200,"context_line":"        elif op \u003d\u003d \u0027!\u003d\u0027:"},{"line_number":201,"context_line":"            if any(len(x) \u003d\u003d 0 for x in [val, match]):"},{"line_number":202,"context_line":"                return val !\u003d match"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_290eec8b","line":199,"range":{"start_line":196,"start_character":0,"end_line":199,"end_character":25},"in_reply_to":"bab6814e_f54f998f","updated":"2016-05-26 22:04:12.000000000","message":"Wanted to use sets first, but they are slower :( Though imho code is clearer with sets.","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"312a9807b86b681b03f0f6189b41c069fa877144","unresolved":false,"context_lines":[{"line_number":198,"context_line":"                    matched \u003d False"},{"line_number":199,"context_line":"                    break"},{"line_number":200,"context_line":"        elif op \u003d\u003d \u0027!\u003d\u0027:"},{"line_number":201,"context_line":"            if any(len(x) \u003d\u003d 0 for x in [val, match]):"},{"line_number":202,"context_line":"                return val !\u003d match"},{"line_number":203,"context_line":"            for elem in set(match):"},{"line_number":204,"context_line":"                if elem in val:"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_25fbc87c","line":201,"range":{"start_line":201,"start_character":15,"end_line":201,"end_character":53},"updated":"2016-05-26 12:36:31.000000000","message":"ditto","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"}],"neutron/tests/functional/agent/test_ovs_lib.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"408a1a15eda93a91e9cad605c93284e70de59090","unresolved":false,"context_lines":[{"line_number":358,"context_line":"        br.destroy()"},{"line_number":359,"context_line":"        self.assertFalse(self.ovs.bridge_exists(name))"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"    def test_db_find_column_type_list(self):"},{"line_number":362,"context_line":"        # make sure that db_find search queries give the same output for"},{"line_number":363,"context_line":"        # both native and vsctl implementations"},{"line_number":364,"context_line":"        tags \u003d self.ovs.ovsdb.db_list(\u0027Port\u0027, columns\u003d[\u0027tag\u0027]).execute()"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_768f93cc","line":361,"range":{"start_line":361,"start_character":8,"end_line":361,"end_character":37},"updated":"2016-05-09 11:17:57.000000000","message":"Should we make sure there actually exist values in ovsdb that we\u0027re working with in this test?","commit_id":"737d92bf87eb23bc60d3c44a3dadcc1fb9f82633"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"87dae3b0800dde679cf2255de7f9e3e7ccfb47a0","unresolved":false,"context_lines":[{"line_number":358,"context_line":"        br.destroy()"},{"line_number":359,"context_line":"        self.assertFalse(self.ovs.bridge_exists(name))"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"    def test_db_find_column_type_list(self):"},{"line_number":362,"context_line":"        # make sure that db_find search queries give the same output for"},{"line_number":363,"context_line":"        # both native and vsctl implementations"},{"line_number":364,"context_line":"        tags \u003d self.ovs.ovsdb.db_list(\u0027Port\u0027, columns\u003d[\u0027tag\u0027]).execute()"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_4e19d478","line":361,"range":{"start_line":361,"start_character":8,"end_line":361,"end_character":37},"in_reply_to":"dab17558_2951efd8","updated":"2016-05-10 12:49:27.000000000","message":"Ok","commit_id":"737d92bf87eb23bc60d3c44a3dadcc1fb9f82633"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"315e5a9e755e10576ae86762e7cc732bc6d679cf","unresolved":false,"context_lines":[{"line_number":358,"context_line":"        br.destroy()"},{"line_number":359,"context_line":"        self.assertFalse(self.ovs.bridge_exists(name))"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"    def test_db_find_column_type_list(self):"},{"line_number":362,"context_line":"        # make sure that db_find search queries give the same output for"},{"line_number":363,"context_line":"        # both native and vsctl implementations"},{"line_number":364,"context_line":"        tags \u003d self.ovs.ovsdb.db_list(\u0027Port\u0027, columns\u003d[\u0027tag\u0027]).execute()"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_a344e01a","line":361,"range":{"start_line":361,"start_character":8,"end_line":361,"end_character":37},"in_reply_to":"dab17558_768f93cc","updated":"2016-05-10 11:00:26.000000000","message":"Yes, we should. At first I wanted to come up with a more general solution - populate the database with some definite values to be sure the test will be stable and predictable, but then turned it down as overcomplicated.\n\nI\u0027ll add some asserts.","commit_id":"737d92bf87eb23bc60d3c44a3dadcc1fb9f82633"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"5e1ffd0bdedaee80e5bffcafcf91c8b08b6e01e6","unresolved":false,"context_lines":[{"line_number":358,"context_line":"        br.destroy()"},{"line_number":359,"context_line":"        self.assertFalse(self.ovs.bridge_exists(name))"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"    def test_db_find_column_type_list(self):"},{"line_number":362,"context_line":"        # make sure that db_find search queries give the same output for"},{"line_number":363,"context_line":"        # both native and vsctl implementations"},{"line_number":364,"context_line":"        tags \u003d self.ovs.ovsdb.db_list(\u0027Port\u0027, columns\u003d[\u0027tag\u0027]).execute()"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_2951efd8","line":361,"range":{"start_line":361,"start_character":8,"end_line":361,"end_character":37},"in_reply_to":"dab17558_a344e01a","updated":"2016-05-10 11:39:56.000000000","message":"I think you can just create couple of resources that would have tag column populated with data according what is tested.","commit_id":"737d92bf87eb23bc60d3c44a3dadcc1fb9f82633"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"7b50a92a89e466a69ac13902bf072ca3d3c7a680","unresolved":false,"context_lines":[{"line_number":382,"context_line":"        # make sure non-null values exists, no sense in a test otherwise"},{"line_number":383,"context_line":"        # will raise StopIteraion if all tags are not defined"},{"line_number":384,"context_line":"        tags_42 \u003d [{\u0027tag\u0027: 42}]"},{"line_number":385,"context_line":"        len_0_list \u003d self.ovs.ovsdb.db_find("},{"line_number":386,"context_line":"            \u0027Port\u0027, (\u0027tag\u0027, \u0027!\u003d\u0027, \u0027[]\u0027), columns\u003d[\u0027tag\u0027]).execute()"},{"line_number":387,"context_line":"        self.assertEqual(tags_present, len_0_list)"},{"line_number":388,"context_line":"        len_0_str_list \u003d self.ovs.ovsdb.db_find("}],"source_content_type":"text/x-python","patch_set":6,"id":"dab17558_621d9f65","line":385,"range":{"start_line":385,"start_character":8,"end_line":385,"end_character":18},"updated":"2016-05-11 08:57:33.000000000","message":"otherwise variable names here are confusing.\nmaybe len_0_list and len_0_str_list are swapped?","commit_id":"721406532a177c5820a65119c5c8c247f098faa1"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"663cd653b01161f2f11534958b6e35b7f4026a33","unresolved":false,"context_lines":[{"line_number":382,"context_line":"        # make sure non-null values exists, no sense in a test otherwise"},{"line_number":383,"context_line":"        # will raise StopIteraion if all tags are not defined"},{"line_number":384,"context_line":"        tags_42 \u003d [{\u0027tag\u0027: 42}]"},{"line_number":385,"context_line":"        len_0_list \u003d self.ovs.ovsdb.db_find("},{"line_number":386,"context_line":"            \u0027Port\u0027, (\u0027tag\u0027, \u0027!\u003d\u0027, \u0027[]\u0027), columns\u003d[\u0027tag\u0027]).execute()"},{"line_number":387,"context_line":"        self.assertEqual(tags_present, len_0_list)"},{"line_number":388,"context_line":"        len_0_str_list \u003d self.ovs.ovsdb.db_find("}],"source_content_type":"text/x-python","patch_set":6,"id":"dab17558_9829148d","line":385,"range":{"start_line":385,"start_character":8,"end_line":385,"end_character":18},"in_reply_to":"dab17558_621d9f65","updated":"2016-05-11 09:18:06.000000000","message":"you are right, they are swapped. Thanks!","commit_id":"721406532a177c5820a65119c5c8c247f098faa1"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"312a9807b86b681b03f0f6189b41c069fa877144","unresolved":false,"context_lines":[{"line_number":379,"context_line":"        # and \u0027Interface\u0027 columns will be populated, so they can be queried."},{"line_number":380,"context_line":"        # Precise values may vary though, so will add a port with a definite"},{"line_number":381,"context_line":"        # tag."},{"line_number":382,"context_line":"        bridge_name \u003d base.get_rand_name(prefix\u003dnet_helpers.BR_PREFIX)"},{"line_number":383,"context_line":"        self.addCleanup(self.ovs.delete_bridge, bridge_name)"},{"line_number":384,"context_line":"        br \u003d self.ovs.add_bridge(bridge_name)"},{"line_number":385,"context_line":"        port_name \u003d base.get_rand_name(prefix\u003dnet_helpers.PORT_PREFIX)"},{"line_number":386,"context_line":"        br.add_port(port_name)"},{"line_number":387,"context_line":"        self.ovs.set_db_attribute(\u0027Port\u0027, port_name, \u0027tag\u0027, 42)"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_30dae44b","line":384,"range":{"start_line":382,"start_character":0,"end_line":384,"end_character":45},"updated":"2016-05-26 12:36:31.000000000","message":"Why not using the bridge from BaseOVSLinuxTestCase like you describe above?","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"875e2c3df314d1685a5508112020891dea6ca426","unresolved":false,"context_lines":[{"line_number":379,"context_line":"        # and \u0027Interface\u0027 columns will be populated, so they can be queried."},{"line_number":380,"context_line":"        # Precise values may vary though, so will add a port with a definite"},{"line_number":381,"context_line":"        # tag."},{"line_number":382,"context_line":"        bridge_name \u003d base.get_rand_name(prefix\u003dnet_helpers.BR_PREFIX)"},{"line_number":383,"context_line":"        self.addCleanup(self.ovs.delete_bridge, bridge_name)"},{"line_number":384,"context_line":"        br \u003d self.ovs.add_bridge(bridge_name)"},{"line_number":385,"context_line":"        port_name \u003d base.get_rand_name(prefix\u003dnet_helpers.PORT_PREFIX)"},{"line_number":386,"context_line":"        br.add_port(port_name)"},{"line_number":387,"context_line":"        self.ovs.set_db_attribute(\u0027Port\u0027, port_name, \u0027tag\u0027, 42)"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_871a587b","line":384,"range":{"start_line":382,"start_character":0,"end_line":384,"end_character":45},"in_reply_to":"bab6814e_30dae44b","updated":"2016-05-27 16:08:53.000000000","message":"Oooh, my bad here. Seems it remained from the time I thought we had to make identical vsctl/native output with type convertions: I was under the impression there\u0027ll be a lot of stuff tested and had a separate test class for that derived from OVSBridgeTestBase. Then the plans changed and I adapted the comment not giving much thought to it, sorry.\n\nAs long as it took me some time to figure out what it was all about, we\u0027ll all be better without it.","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"312a9807b86b681b03f0f6189b41c069fa877144","unresolved":false,"context_lines":[{"line_number":390,"context_line":"        # It should be, but let\u0027s be a little paranoid here as otherwise"},{"line_number":391,"context_line":"        # the test has no sense"},{"line_number":392,"context_line":"        tags_present \u003d [t for t in tags if t[\u0027tag\u0027] !\u003d []]"},{"line_number":393,"context_line":"        self.assertFalse(0, len(tags_present))"},{"line_number":394,"context_line":"        tags_42 \u003d [t for t in tags_present if t[\u0027tag\u0027] \u003d\u003d 42]"},{"line_number":395,"context_line":"        len_0_list \u003d self.ovs.ovsdb.db_find("},{"line_number":396,"context_line":"            \u0027Port\u0027, (\u0027tag\u0027, \u0027!\u003d\u0027, []), columns\u003d[\u0027tag\u0027]).execute()"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_50ccc888","line":393,"range":{"start_line":393,"start_character":8,"end_line":393,"end_character":46},"updated":"2016-05-26 12:36:31.000000000","message":"self.assertTrue(tags_present)","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"875e2c3df314d1685a5508112020891dea6ca426","unresolved":false,"context_lines":[{"line_number":390,"context_line":"        # It should be, but let\u0027s be a little paranoid here as otherwise"},{"line_number":391,"context_line":"        # the test has no sense"},{"line_number":392,"context_line":"        tags_present \u003d [t for t in tags if t[\u0027tag\u0027] !\u003d []]"},{"line_number":393,"context_line":"        self.assertFalse(0, len(tags_present))"},{"line_number":394,"context_line":"        tags_42 \u003d [t for t in tags_present if t[\u0027tag\u0027] \u003d\u003d 42]"},{"line_number":395,"context_line":"        len_0_list \u003d self.ovs.ovsdb.db_find("},{"line_number":396,"context_line":"            \u0027Port\u0027, (\u0027tag\u0027, \u0027!\u003d\u0027, []), columns\u003d[\u0027tag\u0027]).execute()"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_32176846","line":393,"range":{"start_line":393,"start_character":8,"end_line":393,"end_character":46},"in_reply_to":"bab6814e_50ccc888","updated":"2016-05-27 16:08:53.000000000","message":"Done","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"312a9807b86b681b03f0f6189b41c069fa877144","unresolved":false,"context_lines":[{"line_number":392,"context_line":"        tags_present \u003d [t for t in tags if t[\u0027tag\u0027] !\u003d []]"},{"line_number":393,"context_line":"        self.assertFalse(0, len(tags_present))"},{"line_number":394,"context_line":"        tags_42 \u003d [t for t in tags_present if t[\u0027tag\u0027] \u003d\u003d 42]"},{"line_number":395,"context_line":"        len_0_list \u003d self.ovs.ovsdb.db_find("},{"line_number":396,"context_line":"            \u0027Port\u0027, (\u0027tag\u0027, \u0027!\u003d\u0027, []), columns\u003d[\u0027tag\u0027]).execute()"},{"line_number":397,"context_line":"        self.assertEqual(tags_present, len_0_list)"},{"line_number":398,"context_line":"        single_value \u003d self.ovs.ovsdb.db_find("},{"line_number":399,"context_line":"            \u0027Port\u0027, (\u0027tag\u0027, \u0027\u003d\u0027, 42), columns\u003d[\u0027tag\u0027]).execute()"},{"line_number":400,"context_line":"        self.assertEqual(tags_42, single_value)"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_707d8c1b","line":397,"range":{"start_line":395,"start_character":0,"end_line":397,"end_character":50},"updated":"2016-05-26 12:36:31.000000000","message":"Nit: Just in sake of readablity, I would move these three lines to the bottom.","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"875e2c3df314d1685a5508112020891dea6ca426","unresolved":false,"context_lines":[{"line_number":392,"context_line":"        tags_present \u003d [t for t in tags if t[\u0027tag\u0027] !\u003d []]"},{"line_number":393,"context_line":"        self.assertFalse(0, len(tags_present))"},{"line_number":394,"context_line":"        tags_42 \u003d [t for t in tags_present if t[\u0027tag\u0027] \u003d\u003d 42]"},{"line_number":395,"context_line":"        len_0_list \u003d self.ovs.ovsdb.db_find("},{"line_number":396,"context_line":"            \u0027Port\u0027, (\u0027tag\u0027, \u0027!\u003d\u0027, []), columns\u003d[\u0027tag\u0027]).execute()"},{"line_number":397,"context_line":"        self.assertEqual(tags_present, len_0_list)"},{"line_number":398,"context_line":"        single_value \u003d self.ovs.ovsdb.db_find("},{"line_number":399,"context_line":"            \u0027Port\u0027, (\u0027tag\u0027, \u0027\u003d\u0027, 42), columns\u003d[\u0027tag\u0027]).execute()"},{"line_number":400,"context_line":"        self.assertEqual(tags_42, single_value)"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_27fd6cb1","line":397,"range":{"start_line":395,"start_character":0,"end_line":397,"end_character":50},"in_reply_to":"bab6814e_707d8c1b","updated":"2016-05-27 16:08:53.000000000","message":"Done","commit_id":"78a70532db922f290c657bcd42485ab8c5154270"}]}