)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":3012,"name":"Dina Belova","email":"dbelova@mirantis.com","username":"dbelova"},"change_message_id":"40f456f65c0afd2a7d1a53f8269809174aef01c9","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Refactor split_by_op for greater encapsulation"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"split_by_op was returning data against which further error checking was"},{"line_number":10,"context_line":"them required. This changes moves that error handling inside the method"},{"line_number":11,"context_line":"and rationalizes the regular expressions used so they are only doing"},{"line_number":12,"context_line":"the split operation, not constraining what is being split."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3ae8d1ca_5908392d","line":10,"range":{"start_line":10,"start_character":0,"end_line":10,"end_character":4},"updated":"2014-05-29 19:14:50.000000000","message":"them -\u003e then?","commit_id":"8eed60ef32cae2de80a623954a7c4404d5bf7c36"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"1eff7670dff1266b5ae5f9fb7e532afd3a81854f","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Refactor split_by_op for greater encapsulation"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"split_by_op was returning data against which further error checking was"},{"line_number":10,"context_line":"them required. This changes moves that error handling inside the method"},{"line_number":11,"context_line":"and rationalizes the regular expressions used so they are only doing"},{"line_number":12,"context_line":"the split operation, not constraining what is being split."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3ae8d1ca_3ff98d80","line":10,"range":{"start_line":10,"start_character":0,"end_line":10,"end_character":4},"in_reply_to":"3ae8d1ca_5908392d","updated":"2014-05-29 19:58:45.000000000","message":"Done","commit_id":"8eed60ef32cae2de80a623954a7c4404d5bf7c36"}],"ceilometerclient/tests/v2/test_options.py":[{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"56a428f044a3e8743e8d5d43fc343f1e2454c1e0","unresolved":false,"context_lines":[{"line_number":111,"context_line":"        self.assertEqual(ar, [{\u0027field\u0027: \u0027that\u0027,"},{"line_number":112,"context_line":"                               \u0027type\u0027: \u0027float\u0027,"},{"line_number":113,"context_line":"                               \u0027value\u0027: \u0027283.347\u0027,"},{"line_number":114,"context_line":"                               \u0027op\u0027: \u0027le\u0027}])"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    def test_typed_string_whitespace(self):"},{"line_number":117,"context_line":"        ar \u003d options.cli_to_array(\u0027state\u003dstring::insufficient data\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1ae5cdf2_9b122b57","line":114,"updated":"2014-05-29 20:52:31.000000000","message":"I\u0027m not loving the op1 versus op2 for readability.\n\nMaybe parameterize the \u003d versus \u003c\u003d?\n\ne.g.\n\n  def _do_test_typed_float_op(self, op, op_str):\n      ar \u003d options.cli_to_array(\u0027that%sfloat::283.347\u0027 % op)\n      self.assertEqual(ar, [{\u0027field\u0027: \u0027that\u0027,\n                             \u0027type\u0027: \u0027float\u0027,\n                             \u0027value\u0027: \u0027283.347\u0027,\n                             \u0027op\u0027: op_str}])\n\n  def test_typed_float_eq(self):\n      self._do_test_typed_float_op(self, \u0027\u003d\u0027, \u0027eq\u0027)\n\n\n  def test_typed_float_le(self):\n      self._do_test_typed_float_op(self, \u0027\u003c\u003d\u0027, \u0027le\u0027)","commit_id":"ace4d9154aaff8bb6613b388b242774a372cf847"},{"author":{"_account_id":10200,"name":"Christian Martinez","email":"chmartinez35@gmail.com","username":"cmart"},"change_message_id":"f5f2e718168edf2719a33fc865f9a5ff550911ae","unresolved":false,"context_lines":[{"line_number":132,"context_line":"                               \u0027op\u0027: \u0027eq\u0027,"},{"line_number":133,"context_line":"                               \u0027type\u0027: \u0027string\u0027,"},{"line_number":134,"context_line":"                               \u0027value\u0027: \u0027insufficient data\u0027}])"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    def test_invalid_operator(self):"},{"line_number":137,"context_line":"        self.assertRaises(ValueError, options.cli_to_array,"},{"line_number":138,"context_line":"                          \u0027this\u003d2.4;fooo-doof\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1ae5cdf2_3b9af714","line":135,"updated":"2014-05-29 21:00:43.000000000","message":"There is a case I would like to see tested: timestamp expressions like \"timestamp\u003e2014-03-11T16:02:58 or smth like that. I saw that test_query_alarm_history.py has a similar test.","commit_id":"ace4d9154aaff8bb6613b388b242774a372cf847"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"3e4c9e829e5f0f5e80f50cb4c8b28777e74824ff","unresolved":false,"context_lines":[{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def _do_test_typed_float_op(self, op, op_str):"},{"line_number":103,"context_line":"        ar \u003d options.cli_to_array(\u0027that%sfloat::283.347\u0027 % op)"},{"line_number":104,"context_line":"        self.assertEqual(ar, [{\u0027field\u0027: \u0027that\u0027,"},{"line_number":105,"context_line":"                               \u0027type\u0027: \u0027float\u0027,"},{"line_number":106,"context_line":"                               \u0027value\u0027: \u0027283.347\u0027,"},{"line_number":107,"context_line":"                               \u0027op\u0027: op_str}])"}],"source_content_type":"text/x-python","patch_set":11,"id":"1ae5cdf2_176850f4","line":104,"updated":"2014-06-02 16:59:58.000000000","message":"The order of the arguments in the assertEqual() should be expected and observed, like the ones above. I know that this file is not 100% correct from this PoV, but at least the new tests should be consistent and follow the right order.","commit_id":"8939ff397fafe6e528098b145bc4b1318045091e"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"7fd0416aba862df8dfdf607db936ac25fff68d9a","unresolved":false,"context_lines":[{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def _do_test_typed_float_op(self, op, op_str):"},{"line_number":103,"context_line":"        ar \u003d options.cli_to_array(\u0027that%sfloat::283.347\u0027 % op)"},{"line_number":104,"context_line":"        self.assertEqual(ar, [{\u0027field\u0027: \u0027that\u0027,"},{"line_number":105,"context_line":"                               \u0027type\u0027: \u0027float\u0027,"},{"line_number":106,"context_line":"                               \u0027value\u0027: \u0027283.347\u0027,"},{"line_number":107,"context_line":"                               \u0027op\u0027: op_str}])"}],"source_content_type":"text/x-python","patch_set":11,"id":"1ae5cdf2_1a94f535","line":104,"in_reply_to":"1ae5cdf2_176850f4","updated":"2014-06-02 17:16:44.000000000","message":"Yeah, I stumbled on that as well and couldn\u0027t decide which way to go. I\u0027ll fix it to the proper way. Consistency, hobgoblins, etc.","commit_id":"8939ff397fafe6e528098b145bc4b1318045091e"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"3e4c9e829e5f0f5e80f50cb4c8b28777e74824ff","unresolved":false,"context_lines":[{"line_number":107,"context_line":"                               \u0027op\u0027: op_str}])"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    def test_typed_float_eq(self):"},{"line_number":110,"context_line":"        self._do_test_typed_float_op(\u0027\u003d\u0027, \u0027eq\u0027)"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    def test_typed_float_le(self):"},{"line_number":113,"context_line":"        self._do_test_typed_float_op(\u0027\u003c\u003d\u0027, \u0027le\u0027)"}],"source_content_type":"text/x-python","patch_set":11,"id":"1ae5cdf2_376f0cff","line":110,"updated":"2014-06-02 16:59:58.000000000","message":"Only \u0027\u003c\u0027 was not tested below, if I saw right, maybe it could be used here instead of \u0027\u003d\u0027.","commit_id":"8939ff397fafe6e528098b145bc4b1318045091e"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"3e4c9e829e5f0f5e80f50cb4c8b28777e74824ff","unresolved":false,"context_lines":[{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def test_typed_string_whitespace(self):"},{"line_number":116,"context_line":"        ar \u003d options.cli_to_array(\u0027state\u003dstring::insufficient data\u0027)"},{"line_number":117,"context_line":"        self.assertEqual(ar, [{\u0027field\u0027: \u0027state\u0027,"},{"line_number":118,"context_line":"                               \u0027op\u0027: \u0027eq\u0027,"},{"line_number":119,"context_line":"                               \u0027type\u0027: \u0027string\u0027,"},{"line_number":120,"context_line":"                               \u0027value\u0027: \u0027insufficient data\u0027}])"}],"source_content_type":"text/x-python","patch_set":11,"id":"1ae5cdf2_977380e6","line":117,"updated":"2014-06-02 16:59:58.000000000","message":"Same ordering as above.","commit_id":"8939ff397fafe6e528098b145bc4b1318045091e"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"3e4c9e829e5f0f5e80f50cb4c8b28777e74824ff","unresolved":false,"context_lines":[{"line_number":123,"context_line":"        ar \u003d options.cli_to_array("},{"line_number":124,"context_line":"            \u0027that\u003e\u003dfloat::99.9999;state\u003dstring::insufficient data\u0027"},{"line_number":125,"context_line":"        )"},{"line_number":126,"context_line":"        self.assertEqual(ar, [{\u0027field\u0027: \u0027that\u0027,"},{"line_number":127,"context_line":"                               \u0027op\u0027: \u0027ge\u0027,"},{"line_number":128,"context_line":"                               \u0027type\u0027: \u0027float\u0027,"},{"line_number":129,"context_line":"                               \u0027value\u0027: \u002799.9999\u0027},"}],"source_content_type":"text/x-python","patch_set":11,"id":"1ae5cdf2_f74be484","line":126,"updated":"2014-06-02 16:59:58.000000000","message":"Same as above.","commit_id":"8939ff397fafe6e528098b145bc4b1318045091e"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"3e4c9e829e5f0f5e80f50cb4c8b28777e74824ff","unresolved":false,"context_lines":[{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    def test_single_char_field_or_value(self):"},{"line_number":146,"context_line":"        ar \u003d options.cli_to_array(\u0027m\u003c\u003d34;large.thing\u003es;x!\u003dy\u0027)"},{"line_number":147,"context_line":"        self.assertEqual(ar, [{\u0027field\u0027: \u0027m\u0027,"},{"line_number":148,"context_line":"                               \u0027op\u0027: \u0027le\u0027,"},{"line_number":149,"context_line":"                               \u0027value\u0027: \u002734\u0027,"},{"line_number":150,"context_line":"                               \u0027type\u0027: \u0027\u0027},"}],"source_content_type":"text/x-python","patch_set":11,"id":"1ae5cdf2_174f7093","line":147,"updated":"2014-06-02 16:59:58.000000000","message":"Same as above.","commit_id":"8939ff397fafe6e528098b145bc4b1318045091e"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"3e4c9e829e5f0f5e80f50cb4c8b28777e74824ff","unresolved":false,"context_lines":[{"line_number":225,"context_line":"        ar \u003d options.cli_to_array("},{"line_number":226,"context_line":"            \u0027project\u003dcow;timestamp\u003e\u003ddatetime::2014-03-11T16:02:58\u0027"},{"line_number":227,"context_line":"        )"},{"line_number":228,"context_line":"        self.assertEqual(ar, [{\u0027field\u0027: \u0027project\u0027,"},{"line_number":229,"context_line":"                               \u0027op\u0027: \u0027eq\u0027,"},{"line_number":230,"context_line":"                               \u0027type\u0027: \u0027\u0027,"},{"line_number":231,"context_line":"                               \u0027value\u0027: \u0027cow\u0027},"}],"source_content_type":"text/x-python","patch_set":11,"id":"1ae5cdf2_37462c7d","line":228,"updated":"2014-06-02 16:59:58.000000000","message":"Same as above.","commit_id":"8939ff397fafe6e528098b145bc4b1318045091e"}],"ceilometerclient/v2/options.py":[{"author":{"_account_id":3012,"name":"Dina Belova","email":"dbelova@mirantis.com","username":"dbelova"},"change_message_id":"40f456f65c0afd2a7d1a53f8269809174aef01c9","unresolved":false,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        if len(frags) \u003e 1:"},{"line_number":87,"context_line":"            raise ValueError("},{"line_number":88,"context_line":"                \u0027invalid operators in query \"%s\", only one %s in each query\u0027 %"},{"line_number":89,"context_line":"                (string, op_lookup_keys)"},{"line_number":90,"context_line":"            )"},{"line_number":91,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3ae8d1ca_c42700b5","line":88,"updated":"2014-05-29 19:14:50.000000000","message":"I prefer the following format while using multi-string formatting:\n\n\"string 1: %(s1)s, string 2: %(s2)s\" % {\u0027s1\u0027: ..., \u0027s2\u0027: ...}","commit_id":"8eed60ef32cae2de80a623954a7c4404d5bf7c36"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"bf7a24d51cfa0438132fb0a00cdb9390a16f0c72","unresolved":false,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        if len(frags) \u003e 1:"},{"line_number":87,"context_line":"            raise ValueError("},{"line_number":88,"context_line":"                \u0027invalid operators in query \"%s\", only one %s in each query\u0027 %"},{"line_number":89,"context_line":"                (string, op_lookup_keys)"},{"line_number":90,"context_line":"            )"},{"line_number":91,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3ae8d1ca_b12791b5","line":88,"in_reply_to":"3ae8d1ca_9f2b8193","updated":"2014-05-29 20:10:46.000000000","message":"Cool, will fix. Wasn\u0027t sure if the desire was for consistency or for local improvements slowly leading to global \"perfection\". Latter definitely better.","commit_id":"8eed60ef32cae2de80a623954a7c4404d5bf7c36"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"1eff7670dff1266b5ae5f9fb7e532afd3a81854f","unresolved":false,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        if len(frags) \u003e 1:"},{"line_number":87,"context_line":"            raise ValueError("},{"line_number":88,"context_line":"                \u0027invalid operators in query \"%s\", only one %s in each query\u0027 %"},{"line_number":89,"context_line":"                (string, op_lookup_keys)"},{"line_number":90,"context_line":"            )"},{"line_number":91,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3ae8d1ca_ffe805b6","line":88,"in_reply_to":"3ae8d1ca_c42700b5","updated":"2014-05-29 19:58:45.000000000","message":"I wondered about this but couldn\u0027t decide whether it was better to stick with the precedent already set in the earlier code or go with the named parameter.","commit_id":"8eed60ef32cae2de80a623954a7c4404d5bf7c36"},{"author":{"_account_id":3012,"name":"Dina Belova","email":"dbelova@mirantis.com","username":"dbelova"},"change_message_id":"7202b92e2d38656957f94a6bbfdf16642a08f46c","unresolved":false,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        if len(frags) \u003e 1:"},{"line_number":87,"context_line":"            raise ValueError("},{"line_number":88,"context_line":"                \u0027invalid operators in query \"%s\", only one %s in each query\u0027 %"},{"line_number":89,"context_line":"                (string, op_lookup_keys)"},{"line_number":90,"context_line":"            )"},{"line_number":91,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3ae8d1ca_9f2b8193","line":88,"in_reply_to":"3ae8d1ca_ffe805b6","updated":"2014-05-29 20:08:28.000000000","message":"well, that\u0027s matter of style really - but actually it\u0027s better using named params, because in some future this massage might be localised, for instance, and it\u0027s much easier for the translator to understand the meaning of string if params are named well here. Also there are some more reasons here, but using unnamed ones slowly becomes the anti pattern :)","commit_id":"8eed60ef32cae2de80a623954a7c4404d5bf7c36"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"56a428f044a3e8743e8d5d43fc343f1e2454c1e0","unresolved":false,"context_lines":[{"line_number":75,"context_line":"                 \u0027\u003c\u0027: \u0027lt\u0027,"},{"line_number":76,"context_line":"                 \u0027\u003d\u0027: \u0027eq\u0027}"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    op_lookup_keys \u003d \u0027|\u0027.join(sorted(op_lookup.keys(), key\u003dlen, reverse\u003dTrue))"},{"line_number":79,"context_line":"    op_split_re \u003d re.compile(\u0027(.*?)(\u0027 + op_lookup_keys + \u0027)(.*$)\u0027)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def split_by_op(query):"}],"source_content_type":"text/x-python","patch_set":6,"id":"1ae5cdf2_3b1757a5","line":78,"updated":"2014-05-29 20:52:31.000000000","message":"Why the reverse sort on len?\n\nIs there an assumption that two-character operators such as \u0027!\u003d\u0027 are more or less likely to occur than single-character operators like \u0027\u003d\u0027?\n\nUnless there\u0027s a definite performance benefit in the regex matching against ordered case, seems that simply using:\n\n  op_lookup_keys \u003d \u0027|\u0027.join(op_lookup.keys())\n\nis far more readable IMHO.","commit_id":"ace4d9154aaff8bb6613b388b242774a372cf847"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"8a0024922bea82384bb8496e37b6f971c977dc0e","unresolved":false,"context_lines":[{"line_number":75,"context_line":"                 \u0027\u003c\u0027: \u0027lt\u0027,"},{"line_number":76,"context_line":"                 \u0027\u003d\u0027: \u0027eq\u0027}"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    op_lookup_keys \u003d \u0027|\u0027.join(sorted(op_lookup.keys(), key\u003dlen, reverse\u003dTrue))"},{"line_number":79,"context_line":"    op_split_re \u003d re.compile(\u0027(.*?)(\u0027 + op_lookup_keys + \u0027)(.*$)\u0027)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def split_by_op(query):"}],"source_content_type":"text/x-python","patch_set":6,"id":"1ae5cdf2_be36f5e5","line":78,"in_reply_to":"1ae5cdf2_3b1757a5","updated":"2014-05-29 21:24:09.000000000","message":"That\u0027s what I had originally but it does not pass the tests. If the regex is:\n\n    \u0027\u003e|\u003e\u003d\u0027\n\nit will match against \u003e for both \u003e and \u003e\u003d.\n\nNote that this problem didn\u0027t show up until testing in Python 3 where the output of keys() was different from that provided in Python 2.\n\nIt\u0027s awkward but I figured making a minor change (staying in regex land) was better than implementing a tokenizer (which is probably the truly correct thing to do here in the sense that it would be far more maintainable and extensible).","commit_id":"ace4d9154aaff8bb6613b388b242774a372cf847"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"56a428f044a3e8743e8d5d43fc343f1e2454c1e0","unresolved":false,"context_lines":[{"line_number":76,"context_line":"                 \u0027\u003d\u0027: \u0027eq\u0027}"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    op_lookup_keys \u003d \u0027|\u0027.join(sorted(op_lookup.keys(), key\u003dlen, reverse\u003dTrue))"},{"line_number":79,"context_line":"    op_split_re \u003d re.compile(\u0027(.*?)(\u0027 + op_lookup_keys + \u0027)(.*$)\u0027)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def split_by_op(query):"},{"line_number":82,"context_line":"        \"\"\"Split a single query string to field, operator, value."}],"source_content_type":"text/x-python","patch_set":6,"id":"1ae5cdf2_5b99836d","line":79,"updated":"2014-05-29 20:52:31.000000000","message":"Hmmm, seems asymmetric to match end-of-string with \u0027$\u0027, but not the start with \u0027^\u0027.\n\nAlso is it deliberate that the zero-or-more chars are required in the key, but one-or-more chars are required in the value?\n\nDoes it make sense to match both \u0027foo\u003dbar\u0027 and \u0027\u003dbar\u0027?","commit_id":"ace4d9154aaff8bb6613b388b242774a372cf847"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"8a0024922bea82384bb8496e37b6f971c977dc0e","unresolved":false,"context_lines":[{"line_number":76,"context_line":"                 \u0027\u003d\u0027: \u0027eq\u0027}"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    op_lookup_keys \u003d \u0027|\u0027.join(sorted(op_lookup.keys(), key\u003dlen, reverse\u003dTrue))"},{"line_number":79,"context_line":"    op_split_re \u003d re.compile(\u0027(.*?)(\u0027 + op_lookup_keys + \u0027)(.*$)\u0027)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def split_by_op(query):"},{"line_number":82,"context_line":"        \"\"\"Split a single query string to field, operator, value."}],"source_content_type":"text/x-python","patch_set":6,"id":"1ae5cdf2_1e88c921","line":79,"in_reply_to":"1ae5cdf2_5b99836d","updated":"2014-05-29 21:24:09.000000000","message":"It\u0027s generally just good form (performance-wise) to anchor at least one end of your regex if you\u0027re in a position to do so, so in this case it was just habit. It\u0027s not really necessary but it also doesn\u0027t harm things. If the asymmetry bothers then since we\u0027re in a findall it would probably make sense to remove it, not but a ^ on the front.\n\nThe reason for using .* instead of .+ was to emphasize that the method was doing split_by_op not generate_single_query, however I see now that there\u0027s a test case (the one you note) that\u0027s not present that would fail under this code.\n\nMy lame excuse is I trusted the tests too much. Will make more.","commit_id":"ace4d9154aaff8bb6613b388b242774a372cf847"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"56a428f044a3e8743e8d5d43fc343f1e2454c1e0","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        if len(frags) \u003e 1:"},{"line_number":87,"context_line":"            raise ValueError("},{"line_number":88,"context_line":"                \u0027invalid operators in query \"%(query)s\", \u0027"},{"line_number":89,"context_line":"                \u0027only one %(lookup_keys)s in each query\u0027 %"},{"line_number":90,"context_line":"                {\u0027query\u0027: query, \u0027lookup_keys\u0027: op_lookup_keys}"},{"line_number":91,"context_line":"            )"},{"line_number":92,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1ae5cdf2_3be2f7d5","line":89,"updated":"2014-05-29 20:52:31.000000000","message":"This check doesn\u0027t make sense to me, since we now allow arbitrary chars in the key and value (including the operator chars themselves).\n\nSo for example the string:\n\n  \u0027a\u003db\u003c\u003dc!\u003dd\u0027\n\nwill now successfully match, and be parsed out as: \u0027a\u0027, \u0027\u003d\u0027, \u0027b\u003c\u003dc!\u003dd\u0027\n\nso the \u0027... only one %(lookup_keys)s in each query\u0027 seems not to apply?","commit_id":"ace4d9154aaff8bb6613b388b242774a372cf847"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"8a0024922bea82384bb8496e37b6f971c977dc0e","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        if len(frags) \u003e 1:"},{"line_number":87,"context_line":"            raise ValueError("},{"line_number":88,"context_line":"                \u0027invalid operators in query \"%(query)s\", \u0027"},{"line_number":89,"context_line":"                \u0027only one %(lookup_keys)s in each query\u0027 %"},{"line_number":90,"context_line":"                {\u0027query\u0027: query, \u0027lookup_keys\u0027: op_lookup_keys}"},{"line_number":91,"context_line":"            )"},{"line_number":92,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1ae5cdf2_becd1560","line":89,"in_reply_to":"1ae5cdf2_3be2f7d5","updated":"2014-05-29 21:24:09.000000000","message":"You\u0027re right. I reckon that\u0027s a holdover from before I merged in the code from which this patch now depends. Fixes forthcoming.","commit_id":"ace4d9154aaff8bb6613b388b242774a372cf847"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"56a428f044a3e8743e8d5d43fc343f1e2454c1e0","unresolved":false,"context_lines":[{"line_number":90,"context_line":"                {\u0027query\u0027: query, \u0027lookup_keys\u0027: op_lookup_keys}"},{"line_number":91,"context_line":"            )"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"        if len(frags) \u003d\u003d 0:"},{"line_number":94,"context_line":"            raise ValueError(\u0027invalid query %(query)s\u0027 %"},{"line_number":95,"context_line":"                             {\u0027query\u0027: query})"},{"line_number":96,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1ae5cdf2_9b25cb0c","line":93,"updated":"2014-05-29 20:52:31.000000000","message":"More readable as elif?","commit_id":"ace4d9154aaff8bb6613b388b242774a372cf847"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"8a0024922bea82384bb8496e37b6f971c977dc0e","unresolved":false,"context_lines":[{"line_number":90,"context_line":"                {\u0027query\u0027: query, \u0027lookup_keys\u0027: op_lookup_keys}"},{"line_number":91,"context_line":"            )"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"        if len(frags) \u003d\u003d 0:"},{"line_number":94,"context_line":"            raise ValueError(\u0027invalid query %(query)s\u0027 %"},{"line_number":95,"context_line":"                             {\u0027query\u0027: query})"},{"line_number":96,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1ae5cdf2_def10197","line":93,"in_reply_to":"1ae5cdf2_9b25cb0c","updated":"2014-05-29 21:24:09.000000000","message":"The previous chunk is probably gonna go away.","commit_id":"ace4d9154aaff8bb6613b388b242774a372cf847"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"c06246cecdd3f35c3e2d95b2e35ad5c0bb020e11","unresolved":false,"context_lines":[{"line_number":24,"context_line":""},{"line_number":25,"context_line":"OP_LOOKUP_KEYS \u003d \u0027|\u0027.join(OP_LOOKUP.keys())"},{"line_number":26,"context_line":"OP_PREFIX \u003d r\u0027[^!\u003e\u003d\u003c]\u0027"},{"line_number":27,"context_line":"OP_SPLIT_RE \u003d re.compile(r\u0027^(%(prefix)s.+?)(%(keys)s)(%(prefix)s.+)$\u0027 %"},{"line_number":28,"context_line":"                         {\u0027prefix\u0027: OP_PREFIX, \u0027keys\u0027: OP_LOOKUP_KEYS})"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"DATA_TYPE_RE \u003d re.compile(r\u0027^(string|integer|float|datetime|boolean)(::)(.+)$\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1ae5cdf2_55686cf5","line":27,"updated":"2014-05-30 00:05:55.000000000","message":"May be my lack of regex foo, but seems odd to me to include *both* the negated set [^!\u003e\u003d\u003c] *and* the catch-all period in:\n\n ([^!\u003e\u003d\u003c].?)\n\nand:\n\n ([^!\u003e\u003d\u003c].+)\n\nAs I result, for example, I think \u0027a\u003db\u0027 won\u0027t match whereas \u0027a\u003dbb\u0027 will. Open to correction on that.\n\nSimilarly, something like:\n\n  \u0027a\u003cb\u003dc\u0027\n\nwill also match as the first character is not one of the prefixes, which again seems wrong.","commit_id":"93dbcf95fd2a72aae639fbfe69283546d6e51ec3"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"bf7c9d56038bf7b9b8f59cb544eaa2c7572ed7ef","unresolved":false,"context_lines":[{"line_number":24,"context_line":""},{"line_number":25,"context_line":"OP_LOOKUP_KEYS \u003d \u0027|\u0027.join(OP_LOOKUP.keys())"},{"line_number":26,"context_line":"OP_PREFIX \u003d r\u0027[^!\u003e\u003d\u003c]\u0027"},{"line_number":27,"context_line":"OP_SPLIT_RE \u003d re.compile(r\u0027^(%(prefix)s.+?)(%(keys)s)(%(prefix)s.+)$\u0027 %"},{"line_number":28,"context_line":"                         {\u0027prefix\u0027: OP_PREFIX, \u0027keys\u0027: OP_LOOKUP_KEYS})"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"DATA_TYPE_RE \u003d re.compile(r\u0027^(string|integer|float|datetime|boolean)(::)(.+)$\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1ae5cdf2_c12594ac","line":27,"in_reply_to":"1ae5cdf2_3393eefb","updated":"2014-05-30 10:44:47.000000000","message":"Just looking at that regex, my guess is that the issue is not so much an ambiguity, rather a plain oversight in including the period in the original regex, which leaked into the current version.\n\nWhy not simply match on:\n\n  \u0027^([^!\u003e\u003d\u003c]+)(!\u003d|\u003d|\u003c|\u003e|\u003c\u003d|\u003e\u003d)([^!\u003e\u003d\u003c]+)$\u0027\n\ni.e. one-or-more characters of non-operator text, a single operator, one-or-more characters of non-operator text\n\nSo then we would match a\u003db and a\u003dbb, but not \u003db or a\u003cb\u003dc.","commit_id":"93dbcf95fd2a72aae639fbfe69283546d6e51ec3"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"bc71d1ee9ffedc78c0e6a6f1e2188eadf9214b59","unresolved":false,"context_lines":[{"line_number":24,"context_line":""},{"line_number":25,"context_line":"OP_LOOKUP_KEYS \u003d \u0027|\u0027.join(OP_LOOKUP.keys())"},{"line_number":26,"context_line":"OP_PREFIX \u003d r\u0027[^!\u003e\u003d\u003c]\u0027"},{"line_number":27,"context_line":"OP_SPLIT_RE \u003d re.compile(r\u0027^(%(prefix)s.+?)(%(keys)s)(%(prefix)s.+)$\u0027 %"},{"line_number":28,"context_line":"                         {\u0027prefix\u0027: OP_PREFIX, \u0027keys\u0027: OP_LOOKUP_KEYS})"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"DATA_TYPE_RE \u003d re.compile(r\u0027^(string|integer|float|datetime|boolean)(::)(.+)$\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1ae5cdf2_3393eefb","line":27,"in_reply_to":"1ae5cdf2_55686cf5","updated":"2014-05-30 09:48:37.000000000","message":"Your interpretation is correct as far as I can tell. The ambiguity on what should or shouldn\u0027t match is unfortunate and is why I was hoping for some kind of spec on what the defined query language is supposed to be (is it out there somewhere). What\u0027s coded here is designed to fit what is presented in the tests, which have proven to be incomplete.\n\nPerhaps the right thing to do is take a step back and define what is considered valid for keys and values? This patch and the related patch are both predicated on extending what\u0027s valid to include so-called \"special\" characters and whitespace. This flips the logic from saying \"match on this constrained set of characters\" to \"attempt to match based on the boundaries between fields\".\n\nIf we are able to declare that operator characters are _not_ allowed in fields but are allowed in values, or even better are allowed in neither fields nor values, then the regex can be simplified even more and made a bit less ambiguous.\n\nWhat can we declare?","commit_id":"93dbcf95fd2a72aae639fbfe69283546d6e51ec3"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"6a89b3b1f26e0d3fd9874aa8ad810cb08fc14d36","unresolved":false,"context_lines":[{"line_number":24,"context_line":""},{"line_number":25,"context_line":"OP_LOOKUP_KEYS \u003d \u0027|\u0027.join(OP_LOOKUP.keys())"},{"line_number":26,"context_line":"OP_PREFIX \u003d r\u0027[^!\u003e\u003d\u003c]\u0027"},{"line_number":27,"context_line":"OP_SPLIT_RE \u003d re.compile(r\u0027^(%(prefix)s.+?)(%(keys)s)(%(prefix)s.+)$\u0027 %"},{"line_number":28,"context_line":"                         {\u0027prefix\u0027: OP_PREFIX, \u0027keys\u0027: OP_LOOKUP_KEYS})"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"DATA_TYPE_RE \u003d re.compile(r\u0027^(string|integer|float|datetime|boolean)(::)(.+)$\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1ae5cdf2_f3941439","line":27,"in_reply_to":"1ae5cdf2_82db801a","updated":"2014-05-30 17:29:01.000000000","message":"Actually would it work to simply match in explicit *non-greedy* mode?\n\ni.e.\n\n  \u0027(.+?)(\u003d|!\u003d|\u003e|\u003c|\u003c\u003d|\u003e\u003d)(.+?)\u0027","commit_id":"93dbcf95fd2a72aae639fbfe69283546d6e51ec3"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"ad908c682bd9110f93641ce4e6408e06779e5e19","unresolved":false,"context_lines":[{"line_number":24,"context_line":""},{"line_number":25,"context_line":"OP_LOOKUP_KEYS \u003d \u0027|\u0027.join(OP_LOOKUP.keys())"},{"line_number":26,"context_line":"OP_PREFIX \u003d r\u0027[^!\u003e\u003d\u003c]\u0027"},{"line_number":27,"context_line":"OP_SPLIT_RE \u003d re.compile(r\u0027^(%(prefix)s.+?)(%(keys)s)(%(prefix)s.+)$\u0027 %"},{"line_number":28,"context_line":"                         {\u0027prefix\u0027: OP_PREFIX, \u0027keys\u0027: OP_LOOKUP_KEYS})"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"DATA_TYPE_RE \u003d re.compile(r\u0027^(string|integer|float|datetime|boolean)(::)(.+)$\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1ae5cdf2_c1e2f402","line":27,"in_reply_to":"1ae5cdf2_c12594ac","updated":"2014-05-30 10:56:28.000000000","message":"Matching on your suggestion would cause the test added at https://review.openstack.org/#/c/92074/1/ceilometerclient/tests/v2/test_options.py,cm to fail.\n\nSo we have to answer the question of what is allowed in fields and values before we can fully resolve this.","commit_id":"93dbcf95fd2a72aae639fbfe69283546d6e51ec3"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"aa4621f5c9c9ada370237610e4f5273d5fbb6566","unresolved":false,"context_lines":[{"line_number":24,"context_line":""},{"line_number":25,"context_line":"OP_LOOKUP_KEYS \u003d \u0027|\u0027.join(OP_LOOKUP.keys())"},{"line_number":26,"context_line":"OP_PREFIX \u003d r\u0027[^!\u003e\u003d\u003c]\u0027"},{"line_number":27,"context_line":"OP_SPLIT_RE \u003d re.compile(r\u0027^(%(prefix)s.+?)(%(keys)s)(%(prefix)s.+)$\u0027 %"},{"line_number":28,"context_line":"                         {\u0027prefix\u0027: OP_PREFIX, \u0027keys\u0027: OP_LOOKUP_KEYS})"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"DATA_TYPE_RE \u003d re.compile(r\u0027^(string|integer|float|datetime|boolean)(::)(.+)$\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1ae5cdf2_82db801a","line":27,"in_reply_to":"1ae5cdf2_c1e2f402","updated":"2014-05-30 16:36:36.000000000","message":"OK, so say we return to the original bug, the intent was to allow a similar degree of tolerance to that granted by glance \u0026 nova when parsing user-provided metadata and properties, agreed?\n\nIIUC that would require that the following (key, value) variants are supported:\n\n* 1. k, v\n* 2. k!, v\n* 3. k, !v\n* 4. k, v!\n* 5. !k, v\n* 6. !k, !v\n\n(note that in the case of nova user metadata, in order for these data to be stored as resource metadata, there would have to be an implicit \u0027metering.\u0027 prefix on the key, which is then removed by ceilometer)\n\nSo IIUC the current regular expression would not match variants 1, 2, 3, 5 and 6.\n\nIt would match case 2 if the value were \u0027vx\u0027 instead of just \u0027v\u0027, but if the operator happened to be \u0027\u003d\u0027, it would be relying on greedy matching not to misinterpret the trailing \u0027!\u0027 in the key name as part of an !\u003d operator.\n\nSo if I was to venture some rules:\n\n* keys and values may be one character or more in length\n* operator characters may be embedded anywhere in the keys or values\n* for \u0027k\u003c\u003dv\u0027 style ambiguity where the key may be is \u0027k\u003e\u0027 and the operator \u0027\u003d\u0027, or key \u0027k\u0027 and the operator is \u0027\u003c\u003d\u0027, the latter is preferred (i.e. we greedily match the operator over  the key)\n\nI think something real simple like:\n\n  \u0027(.+)(\u003d|!\u003d|\u003e|\u003c|\u003c\u003d|\u003e\u003d)(.+)\u0027\n\nwould fulfill the first two criteria, but not the third.","commit_id":"93dbcf95fd2a72aae639fbfe69283546d6e51ec3"}]}