)]}'
{"ceilometer/compute/pollsters/disk.py":[{"author":{"_account_id":6537,"name":"gordon chung","email":"gord@live.ca","username":"chungg"},"change_message_id":"f4ab49231dde55e19a71ffe28b1b9f11b2ceca20","unresolved":false,"context_lines":[{"line_number":328,"context_line":""},{"line_number":329,"context_line":"    def _get_sample(self, instance, disk_rates_info):"},{"line_number":330,"context_line":"        for disk, value in disk_rates_info.per_disk_rate["},{"line_number":331,"context_line":"                \u0027read_bytes_rate\u0027].items():"},{"line_number":332,"context_line":"            return util.make_sample_from_instance("},{"line_number":333,"context_line":"                instance,"},{"line_number":334,"context_line":"                name\u003d\u0027disk.%s.read.bytes.rate\u0027 % disk,"}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_18c0da49","line":331,"updated":"2014-08-05 14:20:13.000000000","message":"should probably change these to all use six.iteritems","commit_id":"cc1676d89a023012a519afe858d65b2f7c911223"},{"author":{"_account_id":6537,"name":"gordon chung","email":"gord@live.ca","username":"chungg"},"change_message_id":"f4ab49231dde55e19a71ffe28b1b9f11b2ceca20","unresolved":false,"context_lines":[{"line_number":331,"context_line":"                \u0027read_bytes_rate\u0027].items():"},{"line_number":332,"context_line":"            return util.make_sample_from_instance("},{"line_number":333,"context_line":"                instance,"},{"line_number":334,"context_line":"                name\u003d\u0027disk.%s.read.bytes.rate\u0027 % disk,"},{"line_number":335,"context_line":"                type\u003dsample.TYPE_GAUGE,"},{"line_number":336,"context_line":"                unit\u003d\u0027B/s\u0027,"},{"line_number":337,"context_line":"                volume\u003dvalue,"}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_1897fa46","line":334,"updated":"2014-08-05 14:20:13.000000000","message":"i\u0027m wondering if it\u0027s better to name this: disk.read.bytes.rate:%s ... might be a bit easier to query","commit_id":"cc1676d89a023012a519afe858d65b2f7c911223"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"c5119692199f2e01cacb1654b15743d9ba2fa056","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                \u0027read_bytes\u0027]):"},{"line_number":177,"context_line":"            return util.make_sample_from_instance("},{"line_number":178,"context_line":"                instance,"},{"line_number":179,"context_line":"                name\u003d\u0027disk.read.bytes:%s\u0027 % disk,"},{"line_number":180,"context_line":"                type\u003dsample.TYPE_CUMULATIVE,"},{"line_number":181,"context_line":"                unit\u003d\u0027B\u0027,"},{"line_number":182,"context_line":"                volume\u003dvalue,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_a82bbc93","line":179,"updated":"2014-08-07 21:22:50.000000000","message":"Using a meter names such as \u0027disk.read.bytes:vda1\u0027 or \u0027disk.read.bytes:vda2\u0027 etc. doesn\u0027t make a great deal of sense to me, since those names are essentially arbitrary strings.\n\nContrast with the closest equivalent in terms of naming, the instance:\u003cflavor\u003e meter. All instances of that type m1.medium share common sizing characteristics with all other instances of the same flavor. So the instance:\u003cflavor\u003e meter name has some semantic value.\n\nWhereas disks with names vda1 could have very different sizing characteristics. Which suggests the \u0027disk.read.bytes:vda1\u0027 meter name has little semantic value, other than to be different from \u0027disk.read.bytes:vda2\u0027 etc.\n\nSo it doesn\u0027t make sense to me to lump the data associated with all disks of the same name under the same meter.\n\nHow about instead using a more generic meter name that just emphasizes the individual disk nature of the values being recorded, without focusing on the disk name, and instead punt the disk name into the metadata? e.g.\n\n  \u0027disk.device.read.bytes\u0027","commit_id":"4619712a1988256e26cd0ba57f67b83b3070f17d"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"28d65c00d3000dd474f6d9057989206811d07466","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                \u0027read_bytes\u0027]):"},{"line_number":177,"context_line":"            return util.make_sample_from_instance("},{"line_number":178,"context_line":"                instance,"},{"line_number":179,"context_line":"                name\u003d\u0027disk.read.bytes:%s\u0027 % disk,"},{"line_number":180,"context_line":"                type\u003dsample.TYPE_CUMULATIVE,"},{"line_number":181,"context_line":"                unit\u003d\u0027B\u0027,"},{"line_number":182,"context_line":"                volume\u003dvalue,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_b1bb24bd","line":179,"in_reply_to":"baada198_1106901c","updated":"2014-08-07 22:15:00.000000000","message":"IIRC the push-back was on the point of putting the *numeric* data in the metadata of an overarching disk.* sample, not on putting the device name in the metadata of a device-specific sample.","commit_id":"4619712a1988256e26cd0ba57f67b83b3070f17d"},{"author":{"_account_id":6537,"name":"gordon chung","email":"gord@live.ca","username":"chungg"},"change_message_id":"a9115d5938e5c2c389aad4d359b6cf82248c71af","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                \u0027read_bytes\u0027]):"},{"line_number":177,"context_line":"            return util.make_sample_from_instance("},{"line_number":178,"context_line":"                instance,"},{"line_number":179,"context_line":"                name\u003d\u0027disk.read.bytes:%s\u0027 % disk,"},{"line_number":180,"context_line":"                type\u003dsample.TYPE_CUMULATIVE,"},{"line_number":181,"context_line":"                unit\u003d\u0027B\u0027,"},{"line_number":182,"context_line":"                volume\u003dvalue,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_458f4d2e","line":179,"in_reply_to":"baada198_37d1c4cc","updated":"2014-08-08 13:44:55.000000000","message":"i actually preferred the above approach but if i remember correctly Nadya was against any modification of metadata.. so might want her opinion here... or don\u0027t and sneak it in ;)","commit_id":"4619712a1988256e26cd0ba57f67b83b3070f17d"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"709908162a4eaf08fdc2221f20a37b96b2c0ebdd","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                \u0027read_bytes\u0027]):"},{"line_number":177,"context_line":"            return util.make_sample_from_instance("},{"line_number":178,"context_line":"                instance,"},{"line_number":179,"context_line":"                name\u003d\u0027disk.read.bytes:%s\u0027 % disk,"},{"line_number":180,"context_line":"                type\u003dsample.TYPE_CUMULATIVE,"},{"line_number":181,"context_line":"                unit\u003d\u0027B\u0027,"},{"line_number":182,"context_line":"                volume\u003dvalue,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_596c949d","line":179,"in_reply_to":"baada198_458f4d2e","updated":"2014-08-08 15:02:46.000000000","message":"It took me quite a while to learn the difference between the name of a meter and what that meant in regard to a particular sample. Eoghan\u0027s comment \"devices that happen to have the same name by pure coincidence\" is the crux: A sample gains nothing from being grouped with other samples with \"vda1\" in the name of the meter if those samples are of different disks (which most of the time they will be, if you have a sufficiently complex installation).\n\nSo yeah, my vote is \u0027disk.device.read.bytes\u0027 or something similarly general, and put vda1 in the metadata.\n\nWhat you must insure is possible is that you can construct (easily) a query that says \"give me the read bytes for such and such disk being used on such and such\".","commit_id":"4619712a1988256e26cd0ba57f67b83b3070f17d"},{"author":{"_account_id":6924,"name":"Pradeep Kilambi","email":"pkilambi@redhat.com","username":"pkilambi"},"change_message_id":"d49f02cdf365409636bf9b81c57d2cf1d04040be","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                \u0027read_bytes\u0027]):"},{"line_number":177,"context_line":"            return util.make_sample_from_instance("},{"line_number":178,"context_line":"                instance,"},{"line_number":179,"context_line":"                name\u003d\u0027disk.read.bytes:%s\u0027 % disk,"},{"line_number":180,"context_line":"                type\u003dsample.TYPE_CUMULATIVE,"},{"line_number":181,"context_line":"                unit\u003d\u0027B\u0027,"},{"line_number":182,"context_line":"                volume\u003dvalue,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_b95db07e","line":179,"in_reply_to":"baada198_596c949d","updated":"2014-08-08 15:12:41.000000000","message":"Agreed! Eoghan\u0027s suggestion of having a generic meter name and having the device name in metadata makes total sense. I have this implemented in the latest updates to this patch. Once @Nadya gives her blessing, this should be good to go.","commit_id":"4619712a1988256e26cd0ba57f67b83b3070f17d"},{"author":{"_account_id":6924,"name":"Pradeep Kilambi","email":"pkilambi@redhat.com","username":"pkilambi"},"change_message_id":"0506479424795c625299686ca01a91732c81f935","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                \u0027read_bytes\u0027]):"},{"line_number":177,"context_line":"            return util.make_sample_from_instance("},{"line_number":178,"context_line":"                instance,"},{"line_number":179,"context_line":"                name\u003d\u0027disk.read.bytes:%s\u0027 % disk,"},{"line_number":180,"context_line":"                type\u003dsample.TYPE_CUMULATIVE,"},{"line_number":181,"context_line":"                unit\u003d\u0027B\u0027,"},{"line_number":182,"context_line":"                volume\u003dvalue,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_37d1c4cc","line":179,"in_reply_to":"baada198_770a5c1f","updated":"2014-08-07 23:18:36.000000000","message":"understood. Makes sense to me. @gordon you cool with this approach as well ?\n\nI\u0027ll get an update in hopefully tonight","commit_id":"4619712a1988256e26cd0ba57f67b83b3070f17d"},{"author":{"_account_id":6924,"name":"Pradeep Kilambi","email":"pkilambi@redhat.com","username":"pkilambi"},"change_message_id":"d72b07da6efd12c603d8d9bee847d0dab930e60e","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                \u0027read_bytes\u0027]):"},{"line_number":177,"context_line":"            return util.make_sample_from_instance("},{"line_number":178,"context_line":"                instance,"},{"line_number":179,"context_line":"                name\u003d\u0027disk.read.bytes:%s\u0027 % disk,"},{"line_number":180,"context_line":"                type\u003dsample.TYPE_CUMULATIVE,"},{"line_number":181,"context_line":"                unit\u003d\u0027B\u0027,"},{"line_number":182,"context_line":"                volume\u003dvalue,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_1106901c","line":179,"in_reply_to":"baada198_a82bbc93","updated":"2014-08-07 21:58:39.000000000","message":"hmm so in my first implementation for this[1], we got push back for including the disk info as part of the metadata. Has that perspective changed? \n\n[1] https://review.openstack.org/#/c/86408/","commit_id":"4619712a1988256e26cd0ba57f67b83b3070f17d"},{"author":{"_account_id":6924,"name":"Pradeep Kilambi","email":"pkilambi@redhat.com","username":"pkilambi"},"change_message_id":"34d5d385509671dbbf51baeb6dd73291c43d38fe","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                \u0027read_bytes\u0027]):"},{"line_number":177,"context_line":"            return util.make_sample_from_instance("},{"line_number":178,"context_line":"                instance,"},{"line_number":179,"context_line":"                name\u003d\u0027disk.read.bytes:%s\u0027 % disk,"},{"line_number":180,"context_line":"                type\u003dsample.TYPE_CUMULATIVE,"},{"line_number":181,"context_line":"                unit\u003d\u0027B\u0027,"},{"line_number":182,"context_line":"                volume\u003dvalue,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_d1ea5876","line":179,"in_reply_to":"baada198_b1bb24bd","updated":"2014-08-07 22:20:24.000000000","message":"So to clarify, you\u0027re suggesting to rename the meter to something more generic and in the additional_metadata param pass in something like\n\nadditional_metadata \u003d {\u0027device\u0027 : disk }","commit_id":"4619712a1988256e26cd0ba57f67b83b3070f17d"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"4962b2f53b8ddc177f44406b6e05638584b727ed","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                \u0027read_bytes\u0027]):"},{"line_number":177,"context_line":"            return util.make_sample_from_instance("},{"line_number":178,"context_line":"                instance,"},{"line_number":179,"context_line":"                name\u003d\u0027disk.read.bytes:%s\u0027 % disk,"},{"line_number":180,"context_line":"                type\u003dsample.TYPE_CUMULATIVE,"},{"line_number":181,"context_line":"                unit\u003d\u0027B\u0027,"},{"line_number":182,"context_line":"                volume\u003dvalue,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_770a5c1f","line":179,"in_reply_to":"baada198_d1ea5876","updated":"2014-08-07 22:56:19.000000000","message":"Exactly.\n\nInstead of separate meters called  \u0027disk.read.bytes:vda1\u0027 and \u0027disk.read.bytes:vda2\u0027 for devices that happen to have the same name by pure coincidence, we\u0027d have a single device-granularity meter called say \u0027disk.device.read.bytes\u0027 with the actual device name recorded as metadata on the individual samples.\n\nMakes sense?","commit_id":"4619712a1988256e26cd0ba57f67b83b3070f17d"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"c5119692199f2e01cacb1654b15743d9ba2fa056","unresolved":false,"context_lines":[{"line_number":255,"context_line":"            per_disk_r_requests_rate \u003d {}"},{"line_number":256,"context_line":"            per_disk_w_bytes_rate \u003d {}"},{"line_number":257,"context_line":"            per_disk_w_requests_rate \u003d {}"},{"line_number":258,"context_line":"            disk_rates \u003d inspector.inspect_disk_rates("},{"line_number":259,"context_line":"                instance, self._inspection_duration)"},{"line_number":260,"context_line":"            for disk, info in disk_rates:"},{"line_number":261,"context_line":"                r_bytes_rate +\u003d info.read_bytes_rate"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_83dee13a","line":258,"updated":"2014-08-07 21:22:50.000000000","message":"This is only supported by the vSphere inspector, right?","commit_id":"4619712a1988256e26cd0ba57f67b83b3070f17d"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"96ad1c897660f503844eb3822fccfcc068c3025d","unresolved":false,"context_lines":[{"line_number":65,"context_line":"            r_requests \u003d 0"},{"line_number":66,"context_line":"            w_bytes \u003d 0"},{"line_number":67,"context_line":"            w_requests \u003d 0"},{"line_number":68,"context_line":"            per_disk_read_bytes \u003d {}"},{"line_number":69,"context_line":"            per_disk_read_requests \u003d {}"},{"line_number":70,"context_line":"            per_disk_write_bytes \u003d {}"},{"line_number":71,"context_line":"            per_disk_write_requests \u003d {}"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_b71a227a","line":68,"updated":"2014-08-08 11:23:28.000000000","message":"s/per_disk/per_device/ throughout.","commit_id":"6068e90846f6d14727424bd984157a0619fc0666"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"96ad1c897660f503844eb3822fccfcc068c3025d","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                per_disk_read_bytes[disk.device] \u003d info.read_bytes"},{"line_number":83,"context_line":"                per_disk_read_requests[disk.device] \u003d info.read_requests"},{"line_number":84,"context_line":"                per_disk_write_bytes[disk.device] \u003d info.write_bytes"},{"line_number":85,"context_line":"                per_disk_write_requests[disk.device] \u003d info.write_requests"},{"line_number":86,"context_line":"            per_disk_requests \u003d {"},{"line_number":87,"context_line":"                \u0027read_bytes\u0027: per_disk_read_bytes,"},{"line_number":88,"context_line":"                \u0027read_requests\u0027: per_disk_read_requests,"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_c228b687","line":85,"updated":"2014-08-08 11:23:28.000000000","message":"I\u0027m a bit concerned here about the duplication of data in the single-device disk case. Does it make sense to replicate exactly the same data in say the \u0027disk.device.read.requests\u0027 and \u0027disk.read.requests\u0027 meters? Is the break-down between device only needed for multi-device disks?","commit_id":"6068e90846f6d14727424bd984157a0619fc0666"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"89c27f3b774a09a389cfb18ed95c1549f0bacc38","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                per_disk_read_bytes[disk.device] \u003d info.read_bytes"},{"line_number":83,"context_line":"                per_disk_read_requests[disk.device] \u003d info.read_requests"},{"line_number":84,"context_line":"                per_disk_write_bytes[disk.device] \u003d info.write_bytes"},{"line_number":85,"context_line":"                per_disk_write_requests[disk.device] \u003d info.write_requests"},{"line_number":86,"context_line":"            per_disk_requests \u003d {"},{"line_number":87,"context_line":"                \u0027read_bytes\u0027: per_disk_read_bytes,"},{"line_number":88,"context_line":"                \u0027read_requests\u0027: per_disk_read_requests,"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_43cbc572","line":85,"in_reply_to":"baada198_086c8c06","updated":"2014-08-08 14:24:31.000000000","message":"In the special case where there is only a *single* disk device (as opposed to multiple devices) why would the numbers be different?","commit_id":"6068e90846f6d14727424bd984157a0619fc0666"},{"author":{"_account_id":6924,"name":"Pradeep Kilambi","email":"pkilambi@redhat.com","username":"pkilambi"},"change_message_id":"468b10b7735b786589f7c29913955e8581ecf477","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                per_disk_read_bytes[disk.device] \u003d info.read_bytes"},{"line_number":83,"context_line":"                per_disk_read_requests[disk.device] \u003d info.read_requests"},{"line_number":84,"context_line":"                per_disk_write_bytes[disk.device] \u003d info.write_bytes"},{"line_number":85,"context_line":"                per_disk_write_requests[disk.device] \u003d info.write_requests"},{"line_number":86,"context_line":"            per_disk_requests \u003d {"},{"line_number":87,"context_line":"                \u0027read_bytes\u0027: per_disk_read_bytes,"},{"line_number":88,"context_line":"                \u0027read_requests\u0027: per_disk_read_requests,"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_dea32aaf","line":85,"in_reply_to":"baada198_43cbc572","updated":"2014-08-08 14:43:22.000000000","message":"yes if there is just one device then the numbers would be same and both meters will have the same value. I see your point. But to me one is cumulative and other is per device. if i have only one device my cumulative \u003d\u003d per device and i think thats ok. keep the meters consistent for what they mean? \n\nExtra logic to differentiate and skipping the per disk might break some automated scripts that are specifically looking for a per metric data as well as cumulative.\n\nFor example, I have some automated scripts that poll these metrics for billing. I don\u0027t want to deal with special cases where a sample is missing because it so happened that cumulative \u003d\u003d per device?","commit_id":"6068e90846f6d14727424bd984157a0619fc0666"},{"author":{"_account_id":6924,"name":"Pradeep Kilambi","email":"pkilambi@redhat.com","username":"pkilambi"},"change_message_id":"31a9e2922b361307818defdb56d562fe2d82acbf","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                per_disk_read_bytes[disk.device] \u003d info.read_bytes"},{"line_number":83,"context_line":"                per_disk_read_requests[disk.device] \u003d info.read_requests"},{"line_number":84,"context_line":"                per_disk_write_bytes[disk.device] \u003d info.write_bytes"},{"line_number":85,"context_line":"                per_disk_write_requests[disk.device] \u003d info.write_requests"},{"line_number":86,"context_line":"            per_disk_requests \u003d {"},{"line_number":87,"context_line":"                \u0027read_bytes\u0027: per_disk_read_bytes,"},{"line_number":88,"context_line":"                \u0027read_requests\u0027: per_disk_read_requests,"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_086c8c06","line":85,"in_reply_to":"baada198_c228b687","updated":"2014-08-08 14:18:41.000000000","message":"I don\u0027t think its duplication. the info.* is per device vs in the other case its cumulative, so these are two different numbers. i prefer having a separate hash for per device data as it make it cleaner to iterate over in per device pollsters.\n\nagree with rest of your comments, will take care of then in the new patch.","commit_id":"6068e90846f6d14727424bd984157a0619fc0666"},{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"e6e80a211c93cb059c6862c75316395d27e8dd49","unresolved":false,"context_lines":[{"line_number":151,"context_line":"                name\u003d\u0027disk.device.read.requests\u0027,"},{"line_number":152,"context_line":"                type\u003dsample.TYPE_CUMULATIVE,"},{"line_number":153,"context_line":"                unit\u003d\u0027request\u0027,"},{"line_number":154,"context_line":"                volume\u003dvalue,"},{"line_number":155,"context_line":"                additional_metadata\u003d{\u0027device\u0027: disk},"},{"line_number":156,"context_line":"            )"},{"line_number":157,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"baada198_e48935c8","line":154,"updated":"2014-08-08 20:35:55.000000000","message":"AFAIU additional_metadata updates resource_metadata. So if resource_metadata contains \u0027device\u0027 it will be rewritten. One of backends (HBase) relies on resource_metadata and there is an assumption that the field \u0027resource_metadata\u0027 contains the latest and correct metadata about resource. So I\u0027m trying to say that using the following we cannot see all devices associated with instance. I guess it\u0027s only HBase problem. It may be solved, of course (using \u0027if\u0027...). But maybe we should rethink this approach and not update but append metadata sometimes? E.g. when value of metadata_key is list or map.","commit_id":"a6c32a8276aa196c962613085da54f5aae279643"},{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"d3cd86a1adb081238abec56d73c7b453aa34a6d3","unresolved":false,"context_lines":[{"line_number":169,"context_line":"            volume\u003dc_data.r_requests,"},{"line_number":170,"context_line":"        )"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"class PerDeviceReadRequestsPollster(_PerDeviceBase):"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"baada198_c4793831","line":172,"updated":"2014-08-12 13:03:51.000000000","message":"Let me try to describe my thoughts once more.\nWe want to collect statistics for each instance\u0027s disk. So there are several  *main* things we want to know: 1. For what volume info is collected 2. Where is this volume (instance) and finally 3. value\nIn this implementation the first item is put to *resource*-metadata... But it\u0027s actually *sample*-metadata. Unfortunately there is no such parameter in Ceilo. Maybe we should introduce this?\n\nAs for me, if storage driver gets such a sample (with resource_metadata \u003d {\u0027device\u0027 : disk}) it should update corresponding resource. I expect to see all devices that belongs to resource in db: {\u0027device\u0027: [\u0027vda1\u0027, \u0027vda2\u0027]}. But actually I will get only the latest device that was checked. \n\nWe have already had such a situation in Ceilo with network interfaces. In that case resource_id is rid \u003d \"%s-%s-%s\" % (instance_name, instance.id, vnic_data.name). \nMaybe it\u0027s better to be predictable and for same situations use same approaches?\n\nAFAIK, no one driver except HBase uses the fact that \u0027 metadata from newest samples is considered as actual for resource\u0027. So this patch will not break almost nothing :) But it\u0027s incorrect for the model that is chosen in Ceilo.\n\nCores, I don\u0027t want to block this once more. If you consider that the patch is ok, go ahead :) The bug that was found previously is fixed.","commit_id":"7ff577e44167a960f41504e005717b3867e7da11"},{"author":{"_account_id":6537,"name":"gordon chung","email":"gord@live.ca","username":"chungg"},"change_message_id":"ec9ec94eb10c41481310888421f2a7ffe498e598","unresolved":false,"context_lines":[{"line_number":169,"context_line":"            volume\u003dc_data.r_requests,"},{"line_number":170,"context_line":"        )"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"class PerDeviceReadRequestsPollster(_PerDeviceBase):"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"baada198_85a1c13c","line":172,"in_reply_to":"baada198_5d1dcbbe","updated":"2014-08-13 14:00:25.000000000","message":"@Nadya, we do get the whole list of devices. originally, right or wrong, we aggregated these devices and did not capture the fact that the meter was rolled up data.","commit_id":"7ff577e44167a960f41504e005717b3867e7da11"},{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"7f0d545c070c4547966143a183da5715c6a9b831","unresolved":false,"context_lines":[{"line_number":169,"context_line":"            volume\u003dc_data.r_requests,"},{"line_number":170,"context_line":"        )"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"class PerDeviceReadRequestsPollster(_PerDeviceBase):"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"baada198_5d1dcbbe","line":172,"in_reply_to":"baada198_7e3e308b","updated":"2014-08-13 09:42:30.000000000","message":"@Eoghan, yes, I think that adding the device/volume name as a suffix in the resource-id will work.\nRegarding your second question. No, I\u0027m not sure that nova send us the whole list of devices. But in this change sample from one device will rewrite \u0027device\u0027 metadata from other device in instance\u0027s metadata. It\u0027s incorrect.\nSumming up, if we add device_name into resource_id we may keep metadata unchanged.","commit_id":"7ff577e44167a960f41504e005717b3867e7da11"},{"author":{"_account_id":6924,"name":"Pradeep Kilambi","email":"pkilambi@redhat.com","username":"pkilambi"},"change_message_id":"c9e4c89a60c5c8826f7ae69bfd207f97375a320f","unresolved":false,"context_lines":[{"line_number":169,"context_line":"            volume\u003dc_data.r_requests,"},{"line_number":170,"context_line":"        )"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"class PerDeviceReadRequestsPollster(_PerDeviceBase):"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"baada198_b548acb5","line":172,"in_reply_to":"baada198_85a1c13c","updated":"2014-08-13 16:04:10.000000000","message":"So do we have a consensus on this? I prefer not to Abandon this effort once again. If we can come to an agreement on what it would take to merge this I can look into it. From what i read above: We want to include device name as part of the instance.id for disk.device.* metrics?","commit_id":"7ff577e44167a960f41504e005717b3867e7da11"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"67451295ec93d2d7b3adff9e7e639340990c09b2","unresolved":false,"context_lines":[{"line_number":169,"context_line":"            volume\u003dc_data.r_requests,"},{"line_number":170,"context_line":"        )"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"class PerDeviceReadRequestsPollster(_PerDeviceBase):"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"baada198_7e3e308b","line":172,"in_reply_to":"baada198_c4793831","updated":"2014-08-12 15:56:51.000000000","message":"@_nadya_: Can you confirm whether I fully understand your concerns about resource-id?\n\nCurrently we define the resource-id for the disk.* meters to be the UUID of the associated instance. Are you suggesting that since the 1:1 mapping between instances and disk.* samples is now broken in the disk.device.* case, then this borrowing of the instance resource-id for unique is no longer valid?\n\nThat seems like a fair point. Would adding the device/volume name as a suffix in the resource-id work do you think? e.g.\n\n  meter                  | resource-id\n  --------------------------------------------\n  disk.read.bytes        | \u003cinstance_uuid\u003e \n  disk.device.read.bytes | \u003cinstance_uuid\u003e-vda1\n  disk.device.read.bytes | \u003cinstance_uuid\u003e-vda2\n\nAlso, are you expecting that in the case over the exisiting (non-device-specific) disk.* meters, e.g. disk.read.requests as opposed to disk.device.read.requests, that *all* the device/volume names for the associated devices are included in the metadata?\n\nThat is a reasonable point, and perhaps one that we should have thought about originally. We could go ahead now and have:\n\n  meter                  | metadata\n  --------------------------------------------\n  disk.read.bytes        | {device: [\u0027vda1\u0027, \u0027vda2\u0027]}. \n  disk.device.read.bytes | {device: [\u0027vda1\u0027]}\n  disk.device.read.bytes | {device: [\u0027vda2\u0027]}\n\nthough of course it would only impact on disk.{read|write}.{bytes|request} samples going forward.","commit_id":"7ff577e44167a960f41504e005717b3867e7da11"},{"author":{"_account_id":6924,"name":"Pradeep Kilambi","email":"pkilambi@redhat.com","username":"pkilambi"},"change_message_id":"15cb495600310bb01400de1b40cf18a8e2e9909d","unresolved":false,"context_lines":[{"line_number":169,"context_line":"            volume\u003dc_data.r_requests,"},{"line_number":170,"context_line":"        )"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"class PerDeviceReadRequestsPollster(_PerDeviceBase):"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"baada198_bb329af0","line":172,"in_reply_to":"baada198_c4793831","updated":"2014-08-12 15:27:55.000000000","message":"Thanks Nadya. Is this something you\u0027re ok if we investigate as a follow up patch. Since this behavior is an existing condition in HBase case and a limitation with current model and does not break anything, seems like it makes sense to investigate this problem from broader standpoint separately then handle it as part of this patch.","commit_id":"7ff577e44167a960f41504e005717b3867e7da11"},{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"a0e538ad835505d83af78c9b0f621172fc7e2aca","unresolved":false,"context_lines":[{"line_number":187,"context_line":"                type\u003dsample.TYPE_CUMULATIVE,"},{"line_number":188,"context_line":"                unit\u003d\u0027request\u0027,"},{"line_number":189,"context_line":"                volume\u003dvalue,"},{"line_number":190,"context_line":"                resource_id\u003d\"%s-%s\" % (instance.id, disk),"},{"line_number":191,"context_line":"                additional_metadata\u003d{\u0027device\u0027: disk},"},{"line_number":192,"context_line":"            ))"},{"line_number":193,"context_line":"        return samples"}],"source_content_type":"text/x-python","patch_set":7,"id":"7ac7a964_75b629de","line":190,"updated":"2014-08-14 12:32:19.000000000","message":"The last plea from me, please remove additional_metadata\u003d{\u0027device\u0027: disk} from the patch. You already has this info in resource_id. All my concerns were exactly about metadata","commit_id":"4d01023a75f5f5b07bcd3be71d584da3d8fa92f2"},{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"a0e538ad835505d83af78c9b0f621172fc7e2aca","unresolved":false,"context_lines":[{"line_number":221,"context_line":"                type\u003dsample.TYPE_CUMULATIVE,"},{"line_number":222,"context_line":"                unit\u003d\u0027B\u0027,"},{"line_number":223,"context_line":"                volume\u003dvalue,"},{"line_number":224,"context_line":"                resource_id\u003d\"%s-%s\" % (instance.id, disk),"},{"line_number":225,"context_line":"                additional_metadata\u003d{\u0027device\u0027: disk},"},{"line_number":226,"context_line":"            ))"},{"line_number":227,"context_line":"        return samples"}],"source_content_type":"text/x-python","patch_set":7,"id":"7ac7a964_d5b53dce","line":224,"updated":"2014-08-14 12:32:19.000000000","message":"ditto","commit_id":"4d01023a75f5f5b07bcd3be71d584da3d8fa92f2"},{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"a0e538ad835505d83af78c9b0f621172fc7e2aca","unresolved":false,"context_lines":[{"line_number":255,"context_line":"                type\u003dsample.TYPE_CUMULATIVE,"},{"line_number":256,"context_line":"                unit\u003d\u0027request\u0027,"},{"line_number":257,"context_line":"                volume\u003dvalue,"},{"line_number":258,"context_line":"                resource_id\u003d\"%s-%s\" % (instance.id, disk),"},{"line_number":259,"context_line":"                additional_metadata\u003d{\u0027device\u0027: disk},"},{"line_number":260,"context_line":"            ))"},{"line_number":261,"context_line":"        return samples"}],"source_content_type":"text/x-python","patch_set":7,"id":"7ac7a964_55c12d2d","line":258,"updated":"2014-08-14 12:32:19.000000000","message":"ditto","commit_id":"4d01023a75f5f5b07bcd3be71d584da3d8fa92f2"},{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"a0e538ad835505d83af78c9b0f621172fc7e2aca","unresolved":false,"context_lines":[{"line_number":289,"context_line":"                type\u003dsample.TYPE_CUMULATIVE,"},{"line_number":290,"context_line":"                unit\u003d\u0027B\u0027,"},{"line_number":291,"context_line":"                volume\u003dvalue,"},{"line_number":292,"context_line":"                resource_id\u003d\"%s-%s\" % (instance.id, disk),"},{"line_number":293,"context_line":"                additional_metadata\u003d{\u0027device\u0027: disk},"},{"line_number":294,"context_line":"            ))"},{"line_number":295,"context_line":"        return samples"}],"source_content_type":"text/x-python","patch_set":7,"id":"7ac7a964_55ef6db0","line":292,"updated":"2014-08-14 12:32:19.000000000","message":"ditto","commit_id":"4d01023a75f5f5b07bcd3be71d584da3d8fa92f2"},{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"a0e538ad835505d83af78c9b0f621172fc7e2aca","unresolved":false,"context_lines":[{"line_number":424,"context_line":"                type\u003dsample.TYPE_GAUGE,"},{"line_number":425,"context_line":"                unit\u003d\u0027B/s\u0027,"},{"line_number":426,"context_line":"                volume\u003dvalue,"},{"line_number":427,"context_line":"                resource_id\u003d\"%s-%s\" % (instance.id, disk),"},{"line_number":428,"context_line":"                additional_metadata\u003d{\u0027device\u0027: disk},"},{"line_number":429,"context_line":"            ))"},{"line_number":430,"context_line":"        return samples"}],"source_content_type":"text/x-python","patch_set":7,"id":"7ac7a964_55468dc0","line":427,"updated":"2014-08-14 12:32:19.000000000","message":"ditto","commit_id":"4d01023a75f5f5b07bcd3be71d584da3d8fa92f2"},{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"a0e538ad835505d83af78c9b0f621172fc7e2aca","unresolved":false,"context_lines":[{"line_number":457,"context_line":"                type\u003dsample.TYPE_GAUGE,"},{"line_number":458,"context_line":"                unit\u003d\u0027requests/s\u0027,"},{"line_number":459,"context_line":"                volume\u003dvalue,"},{"line_number":460,"context_line":"                resource_id\u003d\"%s-%s\" % (instance.id, disk),"},{"line_number":461,"context_line":"                additional_metadata\u003d{\u0027device\u0027: disk},"},{"line_number":462,"context_line":"            ))"},{"line_number":463,"context_line":"        return samples"}],"source_content_type":"text/x-python","patch_set":7,"id":"7ac7a964_7512a9b5","line":460,"updated":"2014-08-14 12:32:19.000000000","message":"ditto","commit_id":"4d01023a75f5f5b07bcd3be71d584da3d8fa92f2"},{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"a0e538ad835505d83af78c9b0f621172fc7e2aca","unresolved":false,"context_lines":[{"line_number":490,"context_line":"                type\u003dsample.TYPE_GAUGE,"},{"line_number":491,"context_line":"                unit\u003d\u0027B/s\u0027,"},{"line_number":492,"context_line":"                volume\u003dvalue,"},{"line_number":493,"context_line":"                resource_id\u003d\"%s-%s\" % (instance.id, disk),"},{"line_number":494,"context_line":"                additional_metadata\u003d{\u0027device\u0027: disk},"},{"line_number":495,"context_line":"            ))"},{"line_number":496,"context_line":"        return samples"}],"source_content_type":"text/x-python","patch_set":7,"id":"7ac7a964_f525d90a","line":493,"updated":"2014-08-14 12:32:19.000000000","message":"ditto","commit_id":"4d01023a75f5f5b07bcd3be71d584da3d8fa92f2"},{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"a0e538ad835505d83af78c9b0f621172fc7e2aca","unresolved":false,"context_lines":[{"line_number":523,"context_line":"                type\u003dsample.TYPE_GAUGE,"},{"line_number":524,"context_line":"                unit\u003d\u0027requests/s\u0027,"},{"line_number":525,"context_line":"                volume\u003dvalue,"},{"line_number":526,"context_line":"                resource_id\u003d\"%s-%s\" % (instance.id, disk),"},{"line_number":527,"context_line":"                additional_metadata\u003d{\u0027device\u0027: disk},"},{"line_number":528,"context_line":"            ))"},{"line_number":529,"context_line":"        return samples"}],"source_content_type":"text/x-python","patch_set":7,"id":"7ac7a964_352031f7","line":526,"updated":"2014-08-14 12:32:19.000000000","message":"ditto","commit_id":"4d01023a75f5f5b07bcd3be71d584da3d8fa92f2"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"1b3421d5d6118de19320df594436f670a02afe3d","unresolved":false,"context_lines":[{"line_number":144,"context_line":"                    instance_name,"},{"line_number":145,"context_line":"                )"},{"line_number":146,"context_line":"                for s in self._get_sample(instance, c_data):"},{"line_number":147,"context_line":"                    yield s"},{"line_number":148,"context_line":"            except virt_inspector.InstanceNotFoundException as err:"},{"line_number":149,"context_line":"                # Instance was deleted while getting samples. Ignore it."},{"line_number":150,"context_line":"                LOG.debug(_(\u0027Exception while getting samples %s\u0027), err)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7ac7a964_8e62cfd6","line":147,"updated":"2014-08-14 14:27:32.000000000","message":"See comment on line 192 ... why not return here, after yielding the individual samples from within _get_sample()?","commit_id":"50aefcabf185bf69218365dfd500d5e2286def1a"},{"author":{"_account_id":6537,"name":"gordon chung","email":"gord@live.ca","username":"chungg"},"change_message_id":"21c9a717faa2aa9caf3ecc60c9cfeb40c6dbbc19","unresolved":false,"context_lines":[{"line_number":144,"context_line":"                    instance_name,"},{"line_number":145,"context_line":"                )"},{"line_number":146,"context_line":"                for s in self._get_sample(instance, c_data):"},{"line_number":147,"context_line":"                    yield s"},{"line_number":148,"context_line":"            except virt_inspector.InstanceNotFoundException as err:"},{"line_number":149,"context_line":"                # Instance was deleted while getting samples. Ignore it."},{"line_number":150,"context_line":"                LOG.debug(_(\u0027Exception while getting samples %s\u0027), err)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7ac7a964_8bf9d3ca","line":147,"in_reply_to":"7ac7a964_8e62cfd6","updated":"2014-08-14 20:04:06.000000000","message":"iiuc, i think we need to do it this way... if we return here we\u0027ll skip over items in resources. if we yield in both, we\u0027ll end up yielding a generator.","commit_id":"50aefcabf185bf69218365dfd500d5e2286def1a"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"1b3421d5d6118de19320df594436f670a02afe3d","unresolved":false,"context_lines":[{"line_number":189,"context_line":"                volume\u003dvalue,"},{"line_number":190,"context_line":"                resource_id\u003d\"%s-%s\" % (instance.id, disk),"},{"line_number":191,"context_line":"            ))"},{"line_number":192,"context_line":"        return samples"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"class ReadBytesPollster(_Base):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7ac7a964_ce4da774","line":192,"updated":"2014-08-14 14:27:32.000000000","message":"Why not yield the individual samples here, rather than returning a list?","commit_id":"50aefcabf185bf69218365dfd500d5e2286def1a"},{"author":{"_account_id":6924,"name":"Pradeep Kilambi","email":"pkilambi@redhat.com","username":"pkilambi"},"change_message_id":"78bd4f732696be827a7df2d85ff765f9bb966ddd","unresolved":false,"context_lines":[{"line_number":189,"context_line":"                volume\u003dvalue,"},{"line_number":190,"context_line":"                resource_id\u003d\"%s-%s\" % (instance.id, disk),"},{"line_number":191,"context_line":"            ))"},{"line_number":192,"context_line":"        return samples"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"class ReadBytesPollster(_Base):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7ac7a964_2eba9bde","line":192,"in_reply_to":"7ac7a964_ce4da774","updated":"2014-08-14 14:32:59.000000000","message":"I just stuck with the convention in this module, which seems to be to return from _get_samples and yield in the main call. I also discussed this with gordc and seems like the pattern is to use yield if there is a lot more data and in this case usually the # of disks is a small number.","commit_id":"50aefcabf185bf69218365dfd500d5e2286def1a"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"1b3421d5d6118de19320df594436f670a02afe3d","unresolved":false,"context_lines":[{"line_number":222,"context_line":"                volume\u003dvalue,"},{"line_number":223,"context_line":"                resource_id\u003d\"%s-%s\" % (instance.id, disk),"},{"line_number":224,"context_line":"            ))"},{"line_number":225,"context_line":"        return samples"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"class WriteRequestsPollster(_Base):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7ac7a964_ce9be7c7","line":225,"updated":"2014-08-14 14:27:32.000000000","message":"Similarly.","commit_id":"50aefcabf185bf69218365dfd500d5e2286def1a"},{"author":{"_account_id":6537,"name":"gordon chung","email":"gord@live.ca","username":"chungg"},"change_message_id":"5b357ce3667e8d98baf3aa9aec2d1f97ac809e30","unresolved":false,"context_lines":[{"line_number":143,"context_line":"                    instance,"},{"line_number":144,"context_line":"                    instance_name,"},{"line_number":145,"context_line":"                )"},{"line_number":146,"context_line":"                return self._get_sample(instance, c_data)"},{"line_number":147,"context_line":"            except virt_inspector.InstanceNotFoundException as err:"},{"line_number":148,"context_line":"                # Instance was deleted while getting samples. Ignore it."},{"line_number":149,"context_line":"                LOG.debug(_(\u0027Exception while getting samples %s\u0027), err)"}],"source_content_type":"text/x-python","patch_set":9,"id":"7ac7a964_28052511","line":146,"updated":"2014-08-14 19:22:52.000000000","message":"this will skip all other instances... i feel like there\u0027s a gap in test","commit_id":"03078f3f95ed9049e49f1d05e87745dd5511a710"},{"author":{"_account_id":6924,"name":"Pradeep Kilambi","email":"pkilambi@redhat.com","username":"pkilambi"},"change_message_id":"93438f5c00c49ef84e31cee62b0ca0390ab0eada","unresolved":false,"context_lines":[{"line_number":143,"context_line":"                    instance,"},{"line_number":144,"context_line":"                    instance_name,"},{"line_number":145,"context_line":"                )"},{"line_number":146,"context_line":"                return self._get_sample(instance, c_data)"},{"line_number":147,"context_line":"            except virt_inspector.InstanceNotFoundException as err:"},{"line_number":148,"context_line":"                # Instance was deleted while getting samples. Ignore it."},{"line_number":149,"context_line":"                LOG.debug(_(\u0027Exception while getting samples %s\u0027), err)"}],"source_content_type":"text/x-python","patch_set":9,"id":"7ac7a964_288e052f","line":146,"in_reply_to":"7ac7a964_28052511","updated":"2014-08-14 19:36:54.000000000","message":"well yea , this is exactly why I had a list of samples returned and yielded each here. Unless there is a better suggestion I\u0027m gonna revert back to what i had in my previous patch? Yield each disk here and have the per disk return as a list.","commit_id":"03078f3f95ed9049e49f1d05e87745dd5511a710"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"0e746a92aa6dc5e21c868ae5a07a6708826a9e9c","unresolved":false,"context_lines":[{"line_number":143,"context_line":"                    instance,"},{"line_number":144,"context_line":"                    instance_name,"},{"line_number":145,"context_line":"                )"},{"line_number":146,"context_line":"                return self._get_sample(instance, c_data)"},{"line_number":147,"context_line":"            except virt_inspector.InstanceNotFoundException as err:"},{"line_number":148,"context_line":"                # Instance was deleted while getting samples. Ignore it."},{"line_number":149,"context_line":"                LOG.debug(_(\u0027Exception while getting samples %s\u0027), err)"}],"source_content_type":"text/x-python","patch_set":9,"id":"7ac7a964_72ce2385","line":146,"in_reply_to":"7ac7a964_288e052f","updated":"2014-08-14 23:43:25.000000000","message":"I\u0027ll put my hand up here as the proposer of transposing the yield \u0026 return.\n\nI was thinking more in terms of returning a get_sample*s* applied to all the instances, and then yielding the individual samples within get_samples().\n\n(As opposed to the skipping-the-remainder approach taken)\n\nBut if that suggestion was counter-productive, please feel free to revert, while taking on board gordc\u0027s observation about the tests not having caught the skipped instances.","commit_id":"03078f3f95ed9049e49f1d05e87745dd5511a710"},{"author":{"_account_id":6924,"name":"Pradeep Kilambi","email":"pkilambi@redhat.com","username":"pkilambi"},"change_message_id":"7ae7d358e196eeece4dedba66e117a376060a29e","unresolved":false,"context_lines":[{"line_number":143,"context_line":"                    instance,"},{"line_number":144,"context_line":"                    instance_name,"},{"line_number":145,"context_line":"                )"},{"line_number":146,"context_line":"                return self._get_sample(instance, c_data)"},{"line_number":147,"context_line":"            except virt_inspector.InstanceNotFoundException as err:"},{"line_number":148,"context_line":"                # Instance was deleted while getting samples. Ignore it."},{"line_number":149,"context_line":"                LOG.debug(_(\u0027Exception while getting samples %s\u0027), err)"}],"source_content_type":"text/x-python","patch_set":9,"id":"7ac7a964_be7ec4ac","line":146,"in_reply_to":"7ac7a964_72ce2385","updated":"2014-08-15 02:31:44.000000000","message":"Yea considering we have couple of generators to deal with on yields it wasn\u0027t clean to handle the way you suggested. Reverted back to my previous implementation in the latest patch. \n\nRegarding the tests, since there was only one instance, it passed fine. Spoke to gordc, since this behavior existed prior to this patch in the tests, I\u0027ll handle it as a separate patch. \n\nIf all looks good to you and gordc, please review the latest patch and merge.","commit_id":"03078f3f95ed9049e49f1d05e87745dd5511a710"},{"author":{"_account_id":6537,"name":"gordon chung","email":"gord@live.ca","username":"chungg"},"change_message_id":"5b357ce3667e8d98baf3aa9aec2d1f97ac809e30","unresolved":false,"context_lines":[{"line_number":367,"context_line":"                    cache,"},{"line_number":368,"context_line":"                    instance,"},{"line_number":369,"context_line":"                )"},{"line_number":370,"context_line":"                return self._get_sample(instance, disk_rates_info)"},{"line_number":371,"context_line":"            except virt_inspector.InstanceNotFoundException as err:"},{"line_number":372,"context_line":"                # Instance was deleted while getting samples. Ignore it."},{"line_number":373,"context_line":"                LOG.debug(_(\u0027Exception while getting samples %s\u0027), err)"}],"source_content_type":"text/x-python","patch_set":9,"id":"7ac7a964_480a6122","line":370,"updated":"2014-08-14 19:22:52.000000000","message":"same","commit_id":"03078f3f95ed9049e49f1d05e87745dd5511a710"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"619bd0e30d735e6825e52bf012a3587f017a973f","unresolved":false,"context_lines":[{"line_number":144,"context_line":"                    instance_name,"},{"line_number":145,"context_line":"                )"},{"line_number":146,"context_line":"                for s in self._get_sample(instance, c_data):"},{"line_number":147,"context_line":"                    yield s"},{"line_number":148,"context_line":"            except virt_inspector.InstanceNotFoundException as err:"},{"line_number":149,"context_line":"                # Instance was deleted while getting samples. Ignore it."},{"line_number":150,"context_line":"                LOG.debug(_(\u0027Exception while getting samples %s\u0027), err)"}],"source_content_type":"text/x-python","patch_set":10,"id":"7ac7a964_a586a563","line":147,"updated":"2014-08-15 09:17:51.000000000","message":"OK, even if you\u0027re going to do it this way (yield-return, instead of return-yield) can you at least rename the _get_sample() method so that it\u0027s clear its returning *multiple* samples and does so consistently?\n\ne.g.\n\n      for s in self._get_samples(instance, c_data):\n          yield s\n  ...\n  class ReadRequestsPollster(_Base):\n      @staticmethod\n      def _get_samples(instance, c_data):\n          return [util.make_sample_from_instance(...)]\n  ...\n  class PerDeviceReadRequestsPollster(_PerDeviceBase):\n      @staticmethod\n      def _get_samples(instance, c_data):\n          samples \u003d []\n          for disk, value in six.iteritems(...)\n              samples.append(util.make_sample_from_instance(...)\n          return samples\n\nJust to clear on what I mean by return-yield, it would be more like:\n\n      return self._get_samples(instance, c_data)\n  ...\n  class ReadRequestsPollster(_Base):\n      @staticmethod\n      def _get_samples(instance, c_data):\n          yield util.make_sample_from_instance(...)\n  ...\n  class PerDeviceReadRequestsPollster(_PerDeviceBase):\n      @staticmethod\n      def _get_samples(instance, c_data):\n          for disk, value in six.iteritems(...)\n              yield util.make_sample_from_instance(...)","commit_id":"83614d14fd36dbacd5eb88bcbf47afc8e95fb096"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"5d13e346faeb8655a6032ee8b686dcca1f4f0596","unresolved":false,"context_lines":[{"line_number":144,"context_line":"                    instance_name,"},{"line_number":145,"context_line":"                )"},{"line_number":146,"context_line":"                for s in self._get_sample(instance, c_data):"},{"line_number":147,"context_line":"                    yield s"},{"line_number":148,"context_line":"            except virt_inspector.InstanceNotFoundException as err:"},{"line_number":149,"context_line":"                # Instance was deleted while getting samples. Ignore it."},{"line_number":150,"context_line":"                LOG.debug(_(\u0027Exception while getting samples %s\u0027), err)"}],"source_content_type":"text/x-python","patch_set":10,"id":"7ac7a964_692ff8f6","line":147,"in_reply_to":"7ac7a964_52d4ae46","updated":"2014-08-15 18:13:15.000000000","message":"\u003e but that wont work as its within the for loop and return will always exit on first occurrence\n\nWhich for loop do you mean?","commit_id":"83614d14fd36dbacd5eb88bcbf47afc8e95fb096"},{"author":{"_account_id":6924,"name":"Pradeep Kilambi","email":"pkilambi@redhat.com","username":"pkilambi"},"change_message_id":"31207ba7056c6ddcee131bab857cf7f42009482a","unresolved":false,"context_lines":[{"line_number":144,"context_line":"                    instance_name,"},{"line_number":145,"context_line":"                )"},{"line_number":146,"context_line":"                for s in self._get_sample(instance, c_data):"},{"line_number":147,"context_line":"                    yield s"},{"line_number":148,"context_line":"            except virt_inspector.InstanceNotFoundException as err:"},{"line_number":149,"context_line":"                # Instance was deleted while getting samples. Ignore it."},{"line_number":150,"context_line":"                LOG.debug(_(\u0027Exception while getting samples %s\u0027), err)"}],"source_content_type":"text/x-python","patch_set":10,"id":"7ac7a964_52d4ae46","line":147,"in_reply_to":"7ac7a964_a586a563","updated":"2014-08-15 14:51:11.000000000","message":"Done. Regarding your return/yield clarification, I understood what you suggested, but that wont work as its within the for loop and return will always exit on first occurrence and you can yield twice as that will lead to generator(generator) . hence reverted to my original implementation.\n\nAnyways, renamed and made the _get_samples return consistently across all pollsters, please review.","commit_id":"83614d14fd36dbacd5eb88bcbf47afc8e95fb096"}],"ceilometer/tests/compute/pollsters/test_diskio.py":[{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"96ad1c897660f503844eb3822fccfcc068c3025d","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @mock.patch(\u0027ceilometer.pipeline.setup_pipeline\u0027, mock.MagicMock())"},{"line_number":46,"context_line":"    def _check_get_samples(self, factory, name,"},{"line_number":47,"context_line":"                           expected_volume, expected_disk\u003dNone):"},{"line_number":48,"context_line":"        pollster \u003d factory()"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"        mgr \u003d manager.AgentManager()"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_574706a4","line":47,"updated":"2014-08-08 11:23:28.000000000","message":"Clearer as:\n\n  s/expected_disk/expected_device/","commit_id":"6068e90846f6d14727424bd984157a0619fc0666"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"96ad1c897660f503844eb3822fccfcc068c3025d","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        self.assertEqual(\u0027cumulative\u0027, match[0].type)"},{"line_number":62,"context_line":"        if expected_disk:"},{"line_number":63,"context_line":"            self.assertEqual(expected_disk,"},{"line_number":64,"context_line":"                             match[0].resource_metadata[\u0027device\u0027])"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def test_disk_read_requests(self):"},{"line_number":67,"context_line":"        self._check_get_samples(disk.ReadRequestsPollster,"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_97eb5e7a","line":64,"updated":"2014-08-08 11:23:28.000000000","message":"Could lines 62-63 be more simply expressed as:\n\n  self.assertEqual(expected_disk, match[0].resource_metadata.get(\u0027device\u0027))","commit_id":"6068e90846f6d14727424bd984157a0619fc0666"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"96ad1c897660f503844eb3822fccfcc068c3025d","unresolved":false,"context_lines":[{"line_number":112,"context_line":""},{"line_number":113,"context_line":"    @mock.patch(\u0027ceilometer.pipeline.setup_pipeline\u0027, mock.MagicMock())"},{"line_number":114,"context_line":"    def _check_get_samples(self, factory, sample_name,"},{"line_number":115,"context_line":"                           expected_volume, expected_disk\u003dNone):"},{"line_number":116,"context_line":"        pollster \u003d factory()"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        mgr \u003d manager.AgentManager()"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_f7e65aae","line":115,"updated":"2014-08-08 11:23:28.000000000","message":"Similarly.","commit_id":"6068e90846f6d14727424bd984157a0619fc0666"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"96ad1c897660f503844eb3822fccfcc068c3025d","unresolved":false,"context_lines":[{"line_number":131,"context_line":"        self.assertEqual(\u0027gauge\u0027, match[0].type)"},{"line_number":132,"context_line":"        if expected_disk:"},{"line_number":133,"context_line":"            self.assertEqual(expected_disk,"},{"line_number":134,"context_line":"                             match[0].resource_metadata[\u0027device\u0027])"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    def test_disk_read_bytes_rate(self):"},{"line_number":137,"context_line":"        self._check_get_samples(disk.ReadBytesRatePollster,"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_97927efa","line":134,"updated":"2014-08-08 11:23:28.000000000","message":"Similarly.","commit_id":"6068e90846f6d14727424bd984157a0619fc0666"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"5cf95d2f513dd8e8c06dd8f17a171973fb1d913b","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        self.assertEqual(\u0027cumulative\u0027, match[0].type)"},{"line_number":62,"context_line":"        if expected_device:"},{"line_number":63,"context_line":"            self.assertEqual(expected_device,"},{"line_number":64,"context_line":"                             match[0].resource_metadata.get(\u0027device\u0027))"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def test_disk_read_requests(self):"},{"line_number":67,"context_line":"        self._check_get_samples(disk.ReadRequestsPollster,"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_f4d64d25","line":64,"updated":"2014-08-08 15:25:12.000000000","message":"Missed comment from an earlier draft ...\n\nCould lines 62-63 be more simply expressed as:\n  self.assertEqual(expected_disk, match[0].resource_metadata.get(\u0027device\u0027))","commit_id":"59f37b6c9a7be0399e89b8c1c961c175c4d61a18"},{"author":{"_account_id":6924,"name":"Pradeep Kilambi","email":"pkilambi@redhat.com","username":"pkilambi"},"change_message_id":"68693528120ed361d690d85c6bb4f191f6837907","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        self.assertEqual(\u0027cumulative\u0027, match[0].type)"},{"line_number":62,"context_line":"        if expected_device:"},{"line_number":63,"context_line":"            self.assertEqual(expected_device,"},{"line_number":64,"context_line":"                             match[0].resource_metadata.get(\u0027device\u0027))"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def test_disk_read_requests(self):"},{"line_number":67,"context_line":"        self._check_get_samples(disk.ReadRequestsPollster,"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_3a56ef05","line":64,"in_reply_to":"baada198_f4d64d25","updated":"2014-08-08 15:36:18.000000000","message":"hmm I think I already addressed this? or am i missing something?","commit_id":"59f37b6c9a7be0399e89b8c1c961c175c4d61a18"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"5cf95d2f513dd8e8c06dd8f17a171973fb1d913b","unresolved":false,"context_lines":[{"line_number":131,"context_line":"        self.assertEqual(\u0027gauge\u0027, match[0].type)"},{"line_number":132,"context_line":"        if expected_device:"},{"line_number":133,"context_line":"            self.assertEqual(expected_device,"},{"line_number":134,"context_line":"                             match[0].resource_metadata.get(\u0027device\u0027))"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    def test_disk_read_bytes_rate(self):"},{"line_number":137,"context_line":"        self._check_get_samples(disk.ReadBytesRatePollster,"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_b4b775f7","line":134,"updated":"2014-08-08 15:25:12.000000000","message":"Similarly.","commit_id":"59f37b6c9a7be0399e89b8c1c961c175c4d61a18"},{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"e6e80a211c93cb059c6862c75316395d27e8dd49","unresolved":false,"context_lines":[{"line_number":77,"context_line":"    def test_disk_write_bytes(self):"},{"line_number":78,"context_line":"        self._check_get_samples(disk.WriteBytesPollster,"},{"line_number":79,"context_line":"                                \u0027disk.write.bytes\u0027, 8L)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def test_per_disk_read_requests(self):"},{"line_number":82,"context_line":"        self._check_get_samples(disk.PerDeviceReadRequestsPollster,"},{"line_number":83,"context_line":"                                \u0027disk.device.read.requests\u0027, 2L, \u0027vda1\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"baada198_24924d4b","line":80,"updated":"2014-08-08 20:35:55.000000000","message":"Is it possible to check \u0027vda2\u0027? I ran the test:\n    def test_per_disk_read_requests_2(self):\n        self._check_get_samples(disk.PerDeviceReadRequestsPollster,\n                                \u0027disk.device.read.requests\u0027, 3L, \u0027vda2\u0027)\n\nbut it fails... I suspect I don\u0027t understand something, e.g. line 59.  We should have two samples with \u0027disk.device.read.requests\u0027, shouldn\u0027t we?","commit_id":"a6c32a8276aa196c962613085da54f5aae279643"},{"author":{"_account_id":6924,"name":"Pradeep Kilambi","email":"pkilambi@redhat.com","username":"pkilambi"},"change_message_id":"e75727f429dff44d55cf8e1a6f530d23c523d871","unresolved":false,"context_lines":[{"line_number":77,"context_line":"    def test_disk_write_bytes(self):"},{"line_number":78,"context_line":"        self._check_get_samples(disk.WriteBytesPollster,"},{"line_number":79,"context_line":"                                \u0027disk.write.bytes\u0027, 8L)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def test_per_disk_read_requests(self):"},{"line_number":82,"context_line":"        self._check_get_samples(disk.PerDeviceReadRequestsPollster,"},{"line_number":83,"context_line":"                                \u0027disk.device.read.requests\u0027, 2L, \u0027vda1\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"baada198_b5892572","line":80,"in_reply_to":"baada198_24924d4b","updated":"2014-08-08 21:38:53.000000000","message":"hmm I think this is perhaps because I\u0027m doing a return inside the for loop in\ndisk.PerDeviceReadRequestsPollster instead i should be doing a yield? and thats why its always returning the first disk looks like. \n\nLooks like you found a bug. Thank you, i\u0027ll investigate.","commit_id":"a6c32a8276aa196c962613085da54f5aae279643"}],"doc/source/measurements.rst":[{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"cc2977da98b1b29c6fc125279a195f9219befc11","unresolved":false,"context_lines":[{"line_number":83,"context_line":"disk.read.bytes.rate             g      B/s        inst ID   p         1, 2, 3     Average rate of reads in B per second"},{"line_number":84,"context_line":"disk.write.bytes                 c      B          inst ID   p         1, 2        Volume of writes in B"},{"line_number":85,"context_line":"disk.write.bytes.rate            g      B/s        inst ID   p         1, 2, 3     Average volume of writes in B per second"},{"line_number":86,"context_line":"disk.device.read.requests        c      request    inst ID   p         1, 2        Number of read requests per device"},{"line_number":87,"context_line":"disk.device.read.requests.rate   g      request/s  inst ID   p         1, 2, 3     Average rate of read requests per second per device"},{"line_number":88,"context_line":"disk.device.write.requests       c      request    inst ID   p         1, 2        Number of write requests per device"},{"line_number":89,"context_line":"disk.device.write.requests.rate  g      request/s  inst ID   p         1, 2, 3     Average rate of write requests per second per device"}],"source_content_type":"text/x-rst","patch_set":8,"id":"7ac7a964_0e36bf52","line":86,"updated":"2014-08-14 14:39:19.000000000","message":"For *.device.* I think it\u0027s better to change \"inst ID\" to \u0027disk ID\u0027","commit_id":"50aefcabf185bf69218365dfd500d5e2286def1a"}]}