)]}'
{"ceilometer/storage/impl_sqlalchemy.py":[{"author":{"_account_id":6537,"name":"gordon chung","email":"gord@live.ca","username":"chungg"},"change_message_id":"99334cf0264e20c355bf33510b42f12dc7ee53bf","unresolved":false,"context_lines":[{"line_number":801,"context_line":"        res \u003d self._make_stats_query(sample_filter,"},{"line_number":802,"context_line":"                                     None,"},{"line_number":803,"context_line":"                                     aggregate).first()"},{"line_number":804,"context_line":"        if not res.count:"},{"line_number":805,"context_line":"            return"},{"line_number":806,"context_line":""},{"line_number":807,"context_line":"        query \u003d self._make_stats_query(sample_filter, groupby, aggregate)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAWH%2F%2Bb94%3D","line":804,"updated":"2014-03-26 19:45:29.000000000","message":"this isn\u0027t a safe condition. with selectable aggregates, count may or may not exist.","commit_id":"0beb129c2b5ed8424d7c1950a31680acb5125ff3"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"bf74374f33fd20a0c33c46e04d7206cc3932dda8","unresolved":false,"context_lines":[{"line_number":801,"context_line":"        res \u003d self._make_stats_query(sample_filter,"},{"line_number":802,"context_line":"                                     None,"},{"line_number":803,"context_line":"                                     aggregate).first()"},{"line_number":804,"context_line":"        if not res.count:"},{"line_number":805,"context_line":"            return"},{"line_number":806,"context_line":""},{"line_number":807,"context_line":"        query \u003d self._make_stats_query(sample_filter, groupby, aggregate)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAWH%2F%2BaDk%3D","line":804,"in_reply_to":"AAAAWH%2F%2Bb94%3D","updated":"2014-03-27 02:08:10.000000000","message":"thanks, I will search around this.","commit_id":"0beb129c2b5ed8424d7c1950a31680acb5125ff3"},{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"1e318f5641fbc11c0b99ebe2d3616b34990895e0","unresolved":false,"context_lines":[{"line_number":810,"context_line":"        # since we\u0027re trying to be SQL agnostic we have to write portable"},{"line_number":811,"context_line":"        # code, so here it is, admire! We\u0027re going to do one request to get"},{"line_number":812,"context_line":"        # stats by period. We would like to use GROUP BY, but there\u0027s no"},{"line_number":813,"context_line":"        # portable way to manipulate timestamp in SQL, so we can\u0027t."},{"line_number":814,"context_line":"        for period_start, period_end in base.iter_period(res.tsmin,"},{"line_number":815,"context_line":"                                                         res.tsmax,"},{"line_number":816,"context_line":"                                                         period):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAWH%2F%2BxcM%3D","line":813,"updated":"2014-03-24 13:49:51.000000000","message":"During testing this, I noticed that res.tsmin and res.tsmax can  become None. Could you please add the test that cover this part of code? As far as I see the test that you have added covers only \"if not res.count\" statement. Please correct me if I\u0027m wrong here","commit_id":"0beb129c2b5ed8424d7c1950a31680acb5125ff3"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"bf74374f33fd20a0c33c46e04d7206cc3932dda8","unresolved":false,"context_lines":[{"line_number":810,"context_line":"        # since we\u0027re trying to be SQL agnostic we have to write portable"},{"line_number":811,"context_line":"        # code, so here it is, admire! We\u0027re going to do one request to get"},{"line_number":812,"context_line":"        # stats by period. We would like to use GROUP BY, but there\u0027s no"},{"line_number":813,"context_line":"        # portable way to manipulate timestamp in SQL, so we can\u0027t."},{"line_number":814,"context_line":"        for period_start, period_end in base.iter_period(res.tsmin,"},{"line_number":815,"context_line":"                                                         res.tsmax,"},{"line_number":816,"context_line":"                                                         period):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAWH%2F%2BZ2U%3D","line":813,"in_reply_to":"AAAAWH%2F%2Bbgc%3D","updated":"2014-03-27 02:08:10.000000000","message":"thank you  for pointing out.  you are right, this change will have a different result against current implementation. \nthe bug 1295463 just want to reduce unnecessary cycle. in line 820 \"if r.count\", if r.count is 0, it will don\u0027t yield statistic.  so this bug is unimportant. it would better to keep compatible with the current returning. \nso, is invalidating this bug  the better way? (and add a \"TODO\"?)\n\nMy best.\nliu sheng","commit_id":"0beb129c2b5ed8424d7c1950a31680acb5125ff3"},{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"5f7d781e46b1dd94dc92cd810c882fbf67e5c9e1","unresolved":false,"context_lines":[{"line_number":810,"context_line":"        # since we\u0027re trying to be SQL agnostic we have to write portable"},{"line_number":811,"context_line":"        # code, so here it is, admire! We\u0027re going to do one request to get"},{"line_number":812,"context_line":"        # stats by period. We would like to use GROUP BY, but there\u0027s no"},{"line_number":813,"context_line":"        # portable way to manipulate timestamp in SQL, so we can\u0027t."},{"line_number":814,"context_line":"        for period_start, period_end in base.iter_period(res.tsmin,"},{"line_number":815,"context_line":"                                                         res.tsmax,"},{"line_number":816,"context_line":"                                                         period):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAWH%2F%2Bbgc%3D","line":813,"in_reply_to":"AAAAWH%2F%2BcGs%3D","updated":"2014-03-26 20:28:36.000000000","message":"Looks like 1295463 is not a bug. Let me explain.\nLet\u0027s consider several samples(DD.MM.YYYY format): 2.01.2014, 3.01.2014. And user runs ceilometer statistics -m instance -p 1day -q \"start\u003d1.01.2014;end\u003d4.01.2014\". So he wants to get 3 sets of statistics. We cannot return only 2 because we have no data on first. In case when user asks \nceilometer statistics -m instance -p 1day -q \"end\u003d4.01.2014\" we will return 2 sets.","commit_id":"0beb129c2b5ed8424d7c1950a31680acb5125ff3"},{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"8e6366acb8c2a63e73e56570edbac15e150dbfa4","unresolved":false,"context_lines":[{"line_number":810,"context_line":"        # since we\u0027re trying to be SQL agnostic we have to write portable"},{"line_number":811,"context_line":"        # code, so here it is, admire! We\u0027re going to do one request to get"},{"line_number":812,"context_line":"        # stats by period. We would like to use GROUP BY, but there\u0027s no"},{"line_number":813,"context_line":"        # portable way to manipulate timestamp in SQL, so we can\u0027t."},{"line_number":814,"context_line":"        for period_start, period_end in base.iter_period(res.tsmin,"},{"line_number":815,"context_line":"                                                         res.tsmax,"},{"line_number":816,"context_line":"                                                         period):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAWH%2F%2BcGs%3D","line":813,"in_reply_to":"AAAAWH%2F%2Bim8%3D","updated":"2014-03-26 19:12:48.000000000","message":"I\u0027m trying to find it out","commit_id":"0beb129c2b5ed8424d7c1950a31680acb5125ff3"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"86fba140610d74b722896908db4cf315413b6c0f","unresolved":false,"context_lines":[{"line_number":810,"context_line":"        # since we\u0027re trying to be SQL agnostic we have to write portable"},{"line_number":811,"context_line":"        # code, so here it is, admire! We\u0027re going to do one request to get"},{"line_number":812,"context_line":"        # stats by period. We would like to use GROUP BY, but there\u0027s no"},{"line_number":813,"context_line":"        # portable way to manipulate timestamp in SQL, so we can\u0027t."},{"line_number":814,"context_line":"        for period_start, period_end in base.iter_period(res.tsmin,"},{"line_number":815,"context_line":"                                                         res.tsmax,"},{"line_number":816,"context_line":"                                                         period):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAWH%2F%2Bim8%3D","line":813,"in_reply_to":"AAAAWH%2F%2BodY%3D","updated":"2014-03-26 02:57:53.000000000","message":"thank you very much, broken the tests, this is what troubled me.\n\n I\u0027m not sure should we change the effected tests?\n\nMy best","commit_id":"0beb129c2b5ed8424d7c1950a31680acb5125ff3"},{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"8fe1c456907ff6e3033b77cce6737c88137b97e2","unresolved":false,"context_lines":[{"line_number":810,"context_line":"        # since we\u0027re trying to be SQL agnostic we have to write portable"},{"line_number":811,"context_line":"        # code, so here it is, admire! We\u0027re going to do one request to get"},{"line_number":812,"context_line":"        # stats by period. We would like to use GROUP BY, but there\u0027s no"},{"line_number":813,"context_line":"        # portable way to manipulate timestamp in SQL, so we can\u0027t."},{"line_number":814,"context_line":"        for period_start, period_end in base.iter_period(res.tsmin,"},{"line_number":815,"context_line":"                                                         res.tsmax,"},{"line_number":816,"context_line":"                                                         period):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAWH%2F%2BodY%3D","line":813,"in_reply_to":"AAAAWH%2F%2BsEM%3D","updated":"2014-03-25 12:57:23.000000000","message":"This change has broken the tests. Actually I think that we may have smth like the following here to fix 1295463:\n\n        max_start \u003d sample_filter.start\n        if max_start is None:\n            max_start \u003d res.tsmin\n        elif res.tsmin is not None:\n            max_start \u003d max(sample_filter.start, res.tsmin)\n\n        min_stop \u003d sample_filter.end\n        if min_stop is None:\n            min_stop \u003d res.tsmax\n        elif res.tsmax is not None:\n            min_stop \u003d min(sample_filter.end, res.tsmax) + datetime.timedelta(minutes\u003d1)\n\n        for period_start, period_end in base.iter_period(max_start,\n                                                         min_stop,\n                                                         period):\n\nWhat do you think? But even with this code tests become broken. But we need to be sure that they are correct. \nI will ask somebody else to look at this.","commit_id":"0beb129c2b5ed8424d7c1950a31680acb5125ff3"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"f9811e2b3968661fc6e37b970730900566837ce7","unresolved":false,"context_lines":[{"line_number":810,"context_line":"        # since we\u0027re trying to be SQL agnostic we have to write portable"},{"line_number":811,"context_line":"        # code, so here it is, admire! We\u0027re going to do one request to get"},{"line_number":812,"context_line":"        # stats by period. We would like to use GROUP BY, but there\u0027s no"},{"line_number":813,"context_line":"        # portable way to manipulate timestamp in SQL, so we can\u0027t."},{"line_number":814,"context_line":"        for period_start, period_end in base.iter_period(res.tsmin,"},{"line_number":815,"context_line":"                                                         res.tsmax,"},{"line_number":816,"context_line":"                                                         period):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAWH%2F%2BsEM%3D","line":813,"in_reply_to":"AAAAWH%2F%2BxcM%3D","updated":"2014-03-25 02:40:32.000000000","message":"thanks for your reviewing:), In my testing, the res.tsmin and res.tsmax is None is equivalent to res.count is 0L. so here I use \"if not res.count\" to judge the  res.tsmax and  res.tsmin.\n\nmy best\n\nliusheng","commit_id":"0beb129c2b5ed8424d7c1950a31680acb5125ff3"}]}