)]}'
{"ceilometer/compute/nova_notifier.py":[{"author":{"_account_id":5055,"name":"Rain Suo","email":"yugsuo@gmail.com","username":"yugsuo"},"change_message_id":"071d13efcea905aac037ba99a6059bf0fbd2e845","unresolved":false,"context_lines":[{"line_number":107,"context_line":"                setattr(self, \u0027OS-EXT-SRV-ATTR:instance_name\u0027, v)"},{"line_number":108,"context_line":"            elif k \u003d\u003d \u0027metadata\u0027:"},{"line_number":109,"context_line":"                setattr(self, k, utils.metadata_to_dict(v))"},{"line_number":110,"context_line":"            elif k \u003d\u003d \u0027flavor\u0027:"},{"line_number":111,"context_line":"                setattr(self, \u0027vcpus\u0027, v[\u0027vcpus\u0027])"},{"line_number":112,"context_line":"                setattr(self, \u0027memory_mb\u0027, v[\u0027ram\u0027])"},{"line_number":113,"context_line":"                setattr(self, \u0027disk_gb\u0027, v[\u0027disk\u0027])"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAASX%2F%2F4%2B8%3D","line":110,"updated":"2013-08-24 14:54:03.000000000","message":"The Instance class is to translate conductor.API instance_get_by_uuid() result to an instance that looks like ceilometer.nova_client result, not nova.objects.Instance like. And instancet_get_by_uuid result has not an attribute named \u0027flavor\u0027, besides, vcpus, memory_mb, ephemeral_gb and root_gb already exist in the result expect disk_gb, so there is no need set them again.\n\nMaybe this link http://paste.openstack.org/show/43144/ is useful for you","commit_id":"977d38126591354211bb85bec3f054426c6d6fdf"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"ca8fc779908a527d3386902af46a7e6bf003fe89","unresolved":false,"context_lines":[{"line_number":107,"context_line":"                setattr(self, \u0027OS-EXT-SRV-ATTR:instance_name\u0027, v)"},{"line_number":108,"context_line":"            elif k \u003d\u003d \u0027metadata\u0027:"},{"line_number":109,"context_line":"                setattr(self, k, utils.metadata_to_dict(v))"},{"line_number":110,"context_line":"            elif k \u003d\u003d \u0027flavor\u0027:"},{"line_number":111,"context_line":"                setattr(self, \u0027vcpus\u0027, v[\u0027vcpus\u0027])"},{"line_number":112,"context_line":"                setattr(self, \u0027memory_mb\u0027, v[\u0027ram\u0027])"},{"line_number":113,"context_line":"                setattr(self, \u0027disk_gb\u0027, v[\u0027disk\u0027])"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAASX%2F%2F48c%3D","line":110,"in_reply_to":"AAAASX%2F%2F4%2B8%3D","updated":"2013-08-24 15:31:44.000000000","message":"Just like http://paste.openstack.org/show/43144/ line 48 shown, the instance CM get from pollster has \"flavor\" instead of \"vcpus\", \"memory_mb\", etc. In other words, the info CM gotten by notification and pollster are different. That\u0027s why we need to handle it here.","commit_id":"977d38126591354211bb85bec3f054426c6d6fdf"},{"author":{"_account_id":5055,"name":"Rain Suo","email":"yugsuo@gmail.com","username":"yugsuo"},"change_message_id":"98ad68766764201cb1b12cd3c80813f15e785626","unresolved":false,"context_lines":[{"line_number":107,"context_line":"                setattr(self, \u0027OS-EXT-SRV-ATTR:instance_name\u0027, v)"},{"line_number":108,"context_line":"            elif k \u003d\u003d \u0027metadata\u0027:"},{"line_number":109,"context_line":"                setattr(self, k, utils.metadata_to_dict(v))"},{"line_number":110,"context_line":"            elif k \u003d\u003d \u0027flavor\u0027:"},{"line_number":111,"context_line":"                setattr(self, \u0027vcpus\u0027, v[\u0027vcpus\u0027])"},{"line_number":112,"context_line":"                setattr(self, \u0027memory_mb\u0027, v[\u0027ram\u0027])"},{"line_number":113,"context_line":"                setattr(self, \u0027disk_gb\u0027, v[\u0027disk\u0027])"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAASX%2F%2F4z8%3D","line":110,"in_reply_to":"AAAASX%2F%2F48c%3D","updated":"2013-08-24 16:51:51.000000000","message":"Exactly, this Instance class‘s job is to convert the No. 2 instance dict ( line 167)  returned by conductor.API to the No.1 instance dict (line 1) returned by nova_client. And the No.2 instance dict doesn\u0027t contain the flavor attribute, but No.1 instance dict does, so that\u0027s why we have flavor property here: https://github.com/openstack/ceilometer/blob/master/ceilometer/compute/nova_notifier.py#L121 .  Maybe you can take a debug, the code right here will never be executed.","commit_id":"977d38126591354211bb85bec3f054426c6d6fdf"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"5ec214f5cd04c377a511c96d551d572bc9ad128d","unresolved":false,"context_lines":[{"line_number":107,"context_line":"                setattr(self, \u0027OS-EXT-SRV-ATTR:instance_name\u0027, v)"},{"line_number":108,"context_line":"            elif k \u003d\u003d \u0027metadata\u0027:"},{"line_number":109,"context_line":"                setattr(self, k, utils.metadata_to_dict(v))"},{"line_number":110,"context_line":"            elif k \u003d\u003d \u0027flavor\u0027:"},{"line_number":111,"context_line":"                setattr(self, \u0027vcpus\u0027, v[\u0027vcpus\u0027])"},{"line_number":112,"context_line":"                setattr(self, \u0027memory_mb\u0027, v[\u0027ram\u0027])"},{"line_number":113,"context_line":"                setattr(self, \u0027disk_gb\u0027, v[\u0027disk\u0027])"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAASX%2F%2F4Bs%3D","line":110,"in_reply_to":"AAAASX%2F%2F4z8%3D","updated":"2013-08-25 13:20:54.000000000","message":"Thanks for the reminder. After review the code history, it seems like your patch https://review.openstack.org/#/c/40098/ causing my pre patch set UT failure. Since you\u0027re calling util.make_sample_from_instance but passing the incorrect instance, see https://github.com/openstack/ceilometer/blob/master/nova_tests/test_notifier.py#L88.\n\nThe instance object collected by pollster is \u003cnovaclient.v1_1.servers.Server\u003e,  like the #1 in http://paste.openstack.org/show/43144/. my pre patch-set was trying to access the flavor attribute to get more info, so that\u0027s why my pre patchset failed, since the flavor attribute is missing in \u003ccompute.nova_notifier.Instance\u003e.\n\nMay I know is there any specific consideration to call util.make_sample_from_instance to build the sample? Thanks.","commit_id":"977d38126591354211bb85bec3f054426c6d6fdf"},{"author":{"_account_id":5055,"name":"Rain Suo","email":"yugsuo@gmail.com","username":"yugsuo"},"change_message_id":"201dc52dd6068d17aeb509cebb34e5c778b5a898","unresolved":false,"context_lines":[{"line_number":124,"context_line":"            \u0027name\u0027: self.flavor_name,"},{"line_number":125,"context_line":"            \u0027vpus\u0027: self.vcpus,"},{"line_number":126,"context_line":"            \u0027ram\u0027: self.memory_mb,"},{"line_number":127,"context_line":"            \u0027disk\u0027: self.disk_gb,"},{"line_number":128,"context_line":"            \u0027ephemeral\u0027: self.ephemeral_gb"},{"line_number":129,"context_line":"        }"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAASn%2F%2F0%2FU%3D","line":127,"updated":"2013-08-29 03:29:08.000000000","message":"Oh, maybe a little confused. we can get root_gb from instance dict returned by conductor.API, but can\u0027t get disk_gb, so it still will raise AttributeError when deleting instance: \u0027Instance\u0027 object has no attribute \u0027disk_gb\u0027.\n\nMaybe \u0027disk\u0027: self.root_gb+self.ephemeral_gb is better.","commit_id":"fac9f9d3309e5ea91056ebe621870f581a9e8b6c"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"162a3c5cf4ce90acc18f74cfc2da55ee30e31022","unresolved":false,"context_lines":[{"line_number":124,"context_line":"            \u0027name\u0027: self.flavor_name,"},{"line_number":125,"context_line":"            \u0027vpus\u0027: self.vcpus,"},{"line_number":126,"context_line":"            \u0027ram\u0027: self.memory_mb,"},{"line_number":127,"context_line":"            \u0027disk\u0027: self.disk_gb,"},{"line_number":128,"context_line":"            \u0027ephemeral\u0027: self.ephemeral_gb"},{"line_number":129,"context_line":"        }"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAASn%2F%2Fz7U%3D","line":127,"in_reply_to":"AAAASn%2F%2F0%2FU%3D","updated":"2013-08-29 09:48:48.000000000","message":"Thanks a lot, Guangyu. I will submit a new patch soon. Please continue keeping an eye on this, since you\u0027re more familiar with the deleting instance issue. Thanks again for your comments.","commit_id":"fac9f9d3309e5ea91056ebe621870f581a9e8b6c"},{"author":{"_account_id":5055,"name":"Rain Suo","email":"yugsuo@gmail.com","username":"yugsuo"},"change_message_id":"8ba7f7163da7676a08ef1a1f1fc34543fcc86c56","unresolved":false,"context_lines":[{"line_number":122,"context_line":"        return {"},{"line_number":123,"context_line":"            \u0027id\u0027: self.instance_type_id,"},{"line_number":124,"context_line":"            \u0027name\u0027: self.flavor_name,"},{"line_number":125,"context_line":"            \u0027vpus\u0027: self.vcpus,"},{"line_number":126,"context_line":"            \u0027ram\u0027: self.memory_mb,"},{"line_number":127,"context_line":"            \u0027disk\u0027: self.root_gb + self.ephemeral_gb,"},{"line_number":128,"context_line":"            \u0027ephemeral\u0027: self.ephemeral_gb"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAASn%2F%2FzoM%3D","line":125,"updated":"2013-08-29 10:39:08.000000000","message":"should be \u0027vcpus\u0027  :-(","commit_id":"aff668f38a761a8475c36d1b6d1b68c886574120"},{"author":{"_account_id":1669,"name":"Julien Danjou","display_name":"jd","email":"julien@danjou.info","username":"jdanjou"},"change_message_id":"e6abc90251852872facf6a94dbb152d69f18e669","unresolved":false,"context_lines":[{"line_number":124,"context_line":"            \u0027name\u0027: self.flavor_name,"},{"line_number":125,"context_line":"            \u0027vcpus\u0027: self.vcpus,"},{"line_number":126,"context_line":"            \u0027ram\u0027: self.memory_mb,"},{"line_number":127,"context_line":"            \u0027disk\u0027: self.root_gb + self.ephemeral_gb,"},{"line_number":128,"context_line":"            \u0027ephemeral\u0027: self.ephemeral_gb"},{"line_number":129,"context_line":"        }"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAASn%2F%2FutI%3D","line":127,"updated":"2013-08-30 10:09:48.000000000","message":"Why not returning root_gb on its own too?","commit_id":"1605e6ff13aef9281d48ec46fa99481545d60ab0"},{"author":{"_account_id":1669,"name":"Julien Danjou","display_name":"jd","email":"julien@danjou.info","username":"jdanjou"},"change_message_id":"e66577f9e0dde3f2265d7412d8fc536c434980de","unresolved":false,"context_lines":[{"line_number":124,"context_line":"            \u0027name\u0027: self.flavor_name,"},{"line_number":125,"context_line":"            \u0027vcpus\u0027: self.vcpus,"},{"line_number":126,"context_line":"            \u0027ram\u0027: self.memory_mb,"},{"line_number":127,"context_line":"            \u0027disk\u0027: self.root_gb + self.ephemeral_gb,"},{"line_number":128,"context_line":"            \u0027ephemeral\u0027: self.ephemeral_gb"},{"line_number":129,"context_line":"        }"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAASn%2F%2Ftb4%3D","line":127,"in_reply_to":"AAAASn%2F%2Fuos%3D","updated":"2013-08-30 16:04:54.000000000","message":"Fair enough!","commit_id":"1605e6ff13aef9281d48ec46fa99481545d60ab0"},{"author":{"_account_id":5055,"name":"Rain Suo","email":"yugsuo@gmail.com","username":"yugsuo"},"change_message_id":"76166e1741beafcab13135254de4b8f89a930240","unresolved":false,"context_lines":[{"line_number":124,"context_line":"            \u0027name\u0027: self.flavor_name,"},{"line_number":125,"context_line":"            \u0027vcpus\u0027: self.vcpus,"},{"line_number":126,"context_line":"            \u0027ram\u0027: self.memory_mb,"},{"line_number":127,"context_line":"            \u0027disk\u0027: self.root_gb + self.ephemeral_gb,"},{"line_number":128,"context_line":"            \u0027ephemeral\u0027: self.ephemeral_gb"},{"line_number":129,"context_line":"        }"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAASn%2F%2Fuos%3D","line":127,"in_reply_to":"AAAASn%2F%2FutI%3D","updated":"2013-08-30 10:36:27.000000000","message":"Because we can\u0027t get root_gb from nova_client through flavor api, see https://github.com/openstack/ceilometer/blob/master/ceilometer/nova_client.py#L72 ,\nbut conductor.API will can root_gb. The work here is to transform instance returned by conductor.API to look like instance returned by nova_client.","commit_id":"1605e6ff13aef9281d48ec46fa99481545d60ab0"}],"ceilometer/compute/pollsters/util.py":[{"author":{"_account_id":2472,"name":"Doug Hellmann","email":"dhellmann@redhat.com","username":"doug-hellmann"},"change_message_id":"42ef0ec63010c08f295ade2e1cf267aa72c1d8e9","unresolved":false,"context_lines":[{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    for name in INSTANCE_PROPERTIES:"},{"line_number":90,"context_line":"        if hasattr(instance, name):"},{"line_number":91,"context_line":"            metadata[name] \u003d getattr(instance, name)"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    metadata[\u0027vcpus\u0027] \u003d instance.flavor[\u0027vcpus\u0027]"},{"line_number":94,"context_line":"    metadata[\u0027memory_mb\u0027] \u003d instance.flavor[\u0027ram\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAARH%2F%2F4iw%3D","line":91,"updated":"2013-08-15 18:59:30.000000000","message":"This would mean the metadata field is entirely missing. Would it be better to provide a default value (None?) so the field is always present, even if we don\u0027t have a value?","commit_id":"b3389b9db860f8eabd1407c882f58bc8893f55e6"},{"author":{"_account_id":5055,"name":"Rain Suo","email":"yugsuo@gmail.com","username":"yugsuo"},"change_message_id":"4ed38d1f4a35e21c4a04447df869895bada67279","unresolved":false,"context_lines":[{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    for name in INSTANCE_PROPERTIES:"},{"line_number":90,"context_line":"        if hasattr(instance, name):"},{"line_number":91,"context_line":"            metadata[name] \u003d getattr(instance, name)"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    metadata[\u0027vcpus\u0027] \u003d instance.flavor[\u0027vcpus\u0027]"},{"line_number":94,"context_line":"    metadata[\u0027memory_mb\u0027] \u003d instance.flavor[\u0027ram\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAARH%2F%2F1xw%3D","line":91,"in_reply_to":"AAAARH%2F%2F14c%3D","updated":"2013-08-16 13:12:45.000000000","message":"Would this mean if we couldn\u0027t get these info in a single call, then we can ignore them ? By other words,  is it necessary to get these info ? I ask these questions, because I also encounter the same situation in this bug : https://review.openstack.org/#/c/40098/10 . It also fail to get enough info from nova conductor API, such as image_ref_link, flavor_name. So I am a little confused here whether I should make another call to to get these missing fields.\n\nThanks.","commit_id":"b3389b9db860f8eabd1407c882f58bc8893f55e6"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"f43bede1e7600d426ce6d0ca83cc011d2ad4e711","unresolved":false,"context_lines":[{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    for name in INSTANCE_PROPERTIES:"},{"line_number":90,"context_line":"        if hasattr(instance, name):"},{"line_number":91,"context_line":"            metadata[name] \u003d getattr(instance, name)"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    metadata[\u0027vcpus\u0027] \u003d instance.flavor[\u0027vcpus\u0027]"},{"line_number":94,"context_line":"    metadata[\u0027memory_mb\u0027] \u003d instance.flavor[\u0027ram\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAARH%2F%2F14c%3D","line":91,"in_reply_to":"AAAARH%2F%2F18c%3D","updated":"2013-08-16 12:49:07.000000000","message":"Based on current implement, the instance of VM is extracted by nova-list REST API. There is no these info, though they can be gotten by nova conductor API. That means another call to get those info. \n\nWelcome more comments :)","commit_id":"b3389b9db860f8eabd1407c882f58bc8893f55e6"},{"author":{"_account_id":5055,"name":"Rain Suo","email":"yugsuo@gmail.com","username":"yugsuo"},"change_message_id":"be166af43ad4d12119c8f1a0d12b9379164c720f","unresolved":false,"context_lines":[{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    for name in INSTANCE_PROPERTIES:"},{"line_number":90,"context_line":"        if hasattr(instance, name):"},{"line_number":91,"context_line":"            metadata[name] \u003d getattr(instance, name)"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    metadata[\u0027vcpus\u0027] \u003d instance.flavor[\u0027vcpus\u0027]"},{"line_number":94,"context_line":"    metadata[\u0027memory_mb\u0027] \u003d instance.flavor[\u0027ram\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAARH%2F%2F18c%3D","line":91,"in_reply_to":"AAAARH%2F%2F3gI%3D","updated":"2013-08-16 12:26:04.000000000","message":"And why couldn\u0027t we get these information (os_type, reservation_id, etc...) ?","commit_id":"b3389b9db860f8eabd1407c882f58bc8893f55e6"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"31bc753c4b7ce253b420de12f77f84b81a007bf9","unresolved":false,"context_lines":[{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    for name in INSTANCE_PROPERTIES:"},{"line_number":90,"context_line":"        if hasattr(instance, name):"},{"line_number":91,"context_line":"            metadata[name] \u003d getattr(instance, name)"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    metadata[\u0027vcpus\u0027] \u003d instance.flavor[\u0027vcpus\u0027]"},{"line_number":94,"context_line":"    metadata[\u0027memory_mb\u0027] \u003d instance.flavor[\u0027ram\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAARH%2F%2F3gI%3D","line":91,"in_reply_to":"AAAARH%2F%2F4iw%3D","updated":"2013-08-16 00:38:04.000000000","message":"Doug, just like you see, we do provide the default value(empty string) at the previous version. But seems somebody don\u0027t like it. After discussed with jd__, we agreed to remove them if they\u0027re empty to avoid confusing end user. Because, currently, end user maybe wondering if there are something wrong cause the metadata missing.\n\nAnyway, if we still want to keep these metadata though they\u0027re empty, I think we need to document this to let user know more.","commit_id":"b3389b9db860f8eabd1407c882f58bc8893f55e6"},{"author":{"_account_id":4491,"name":"Lianhao Lu","email":"llh_misc@outlook.com","username":"lianhao-lu"},"change_message_id":"eb06571a6b105a2d53d047ec867b05e380348d67","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        if hasattr(instance, name):"},{"line_number":91,"context_line":"            metadata[name] \u003d getattr(instance, name)"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    if len(instance.flavor) \u003e 2:"},{"line_number":94,"context_line":"        metadata[\u0027vcpus\u0027] \u003d instance.flavor[\u0027vcpus\u0027]"},{"line_number":95,"context_line":"        metadata[\u0027memory_mb\u0027] \u003d instance.flavor[\u0027ram\u0027]"},{"line_number":96,"context_line":"        metadata[\u0027disk_gb\u0027] \u003d instance.flavor[\u0027disk\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAASX%2F%2F258%3D","line":93,"updated":"2013-08-26 07:20:57.000000000","message":"why the magic number 2? According to your change to nova-client.py, is it always true?","commit_id":"ad66dec8c4f299d95d6ee725328f7dc3d724df91"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"d4b8606faa89584d79fa39865a34d73d8e8f715b","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        if hasattr(instance, name):"},{"line_number":91,"context_line":"            metadata[name] \u003d getattr(instance, name)"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    if len(instance.flavor) \u003e 2:"},{"line_number":94,"context_line":"        metadata[\u0027vcpus\u0027] \u003d instance.flavor[\u0027vcpus\u0027]"},{"line_number":95,"context_line":"        metadata[\u0027memory_mb\u0027] \u003d instance.flavor[\u0027ram\u0027]"},{"line_number":96,"context_line":"        metadata[\u0027disk_gb\u0027] \u003d instance.flavor[\u0027disk\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAASX%2F%2F2jI%3D","line":93,"in_reply_to":"AAAASX%2F%2F258%3D","updated":"2013-08-26 10:11:06.000000000","message":"Good question. Yes, it\u0027s bad. I\u0027m going to remove it and submit a new patch. Thanks.","commit_id":"ad66dec8c4f299d95d6ee725328f7dc3d724df91"},{"author":{"_account_id":5055,"name":"Rain Suo","email":"yugsuo@gmail.com","username":"yugsuo"},"change_message_id":"3a76d99d71aa881eb85c0c6d810031ac23a1ead6","unresolved":false,"context_lines":[{"line_number":95,"context_line":"    metadata[\u0027disk_gb\u0027] \u003d instance.flavor[\u0027disk\u0027]"},{"line_number":96,"context_line":"    metadata[\u0027ephemeral_gb\u0027] \u003d instance.flavor[\u0027ephemeral\u0027]"},{"line_number":97,"context_line":"    metadata[\u0027root_gb\u0027] \u003d int(metadata[\u0027disk_gb\u0027]) - \\"},{"line_number":98,"context_line":"        int(metadata[\u0027ephemeral_gb\u0027])"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    return _add_reserved_user_metadata(instance, metadata)"},{"line_number":101,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAASn%2F%2F5f8%3D","line":98,"updated":"2013-08-28 08:51:52.000000000","message":"If you really want to do like this, I beg you add other flavor fields at https://github.com/openstack/ceilometer/blob/master/ceilometer/compute/nova_notifier.py#L121","commit_id":"b06645076df71dc331df36e24bd6ffac109ea7ea"},{"author":{"_account_id":5055,"name":"Rain Suo","email":"yugsuo@gmail.com","username":"yugsuo"},"change_message_id":"8fcd04e0d6572cd8b3f1539a2447bb50cfaec436","unresolved":false,"context_lines":[{"line_number":95,"context_line":"    metadata[\u0027disk_gb\u0027] \u003d instance.flavor[\u0027disk\u0027]"},{"line_number":96,"context_line":"    metadata[\u0027ephemeral_gb\u0027] \u003d instance.flavor[\u0027ephemeral\u0027]"},{"line_number":97,"context_line":"    metadata[\u0027root_gb\u0027] \u003d int(metadata[\u0027disk_gb\u0027]) - \\"},{"line_number":98,"context_line":"        int(metadata[\u0027ephemeral_gb\u0027])"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    return _add_reserved_user_metadata(instance, metadata)"},{"line_number":101,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAASn%2F%2F5YM%3D","line":98,"in_reply_to":"AAAASn%2F%2F5co%3D","updated":"2013-08-28 09:32:46.000000000","message":"Yes, nova_notifier will be called when deleting instance, it will also collected samples by pollsters, so we should ensure the flavor\u0027s key-value is existed, or it will raise error when deleting instance in nova-compute progress.","commit_id":"b06645076df71dc331df36e24bd6ffac109ea7ea"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"9b33461ff07af044668b425cffe552210df75745","unresolved":false,"context_lines":[{"line_number":95,"context_line":"    metadata[\u0027disk_gb\u0027] \u003d instance.flavor[\u0027disk\u0027]"},{"line_number":96,"context_line":"    metadata[\u0027ephemeral_gb\u0027] \u003d instance.flavor[\u0027ephemeral\u0027]"},{"line_number":97,"context_line":"    metadata[\u0027root_gb\u0027] \u003d int(metadata[\u0027disk_gb\u0027]) - \\"},{"line_number":98,"context_line":"        int(metadata[\u0027ephemeral_gb\u0027])"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    return _add_reserved_user_metadata(instance, metadata)"},{"line_number":101,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAASn%2F%2F5co%3D","line":98,"in_reply_to":"AAAASn%2F%2F5f8%3D","updated":"2013-08-28 09:07:17.000000000","message":"You know, as for samples collected by pollster, we\u0027re missing almost all the attributes listed in above INSTANCE_PROPERTIES. So I hope we can provide as more info as we can to be consistent with the data collected by notification.\n\nSo I assume the -1 you given is hoping add those attributes to https://github.com/openstack/ceilometer/blob/master/ceilometer/compute/nova_notifier.py#L121, right?","commit_id":"b06645076df71dc331df36e24bd6ffac109ea7ea"}],"nova_tests/test_notifier.py":[{"author":{"_account_id":5055,"name":"Rain Suo","email":"yugsuo@gmail.com","username":"yugsuo"},"change_message_id":"3a76d99d71aa881eb85c0c6d810031ac23a1ead6","unresolved":false,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        def get_samples(self, manager, cache, instance):"},{"line_number":87,"context_line":"            self.instances.append((manager, instance))"},{"line_number":88,"context_line":"            test_data_2 \u003d sample.Sample("},{"line_number":89,"context_line":"                name\u003d\u0027test2\u0027,"},{"line_number":90,"context_line":"                type\u003dsample.TYPE_CUMULATIVE,"},{"line_number":91,"context_line":"                unit\u003d\u0027units-go-here\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAASn%2F%2F5oE%3D","line":88,"updated":"2013-08-28 08:51:52.000000000","message":"The purpose I use util here is to check all instance attributes that be used through the process of make_sample_from_instace() is existed, such as instance.user_id, instance.flavor, instance.flavor[\u0027vcpu\u0027], etc, or there is no need to add another test_data right here. Because compute.nova_notifier.Instance may lose some fields of instance, see this bug https://review.openstack.org/#/c/40098/11.\n\nSo, I prefer you don\u0027t do like this.","commit_id":"b06645076df71dc331df36e24bd6ffac109ea7ea"},{"author":{"_account_id":5055,"name":"Rain Suo","email":"yugsuo@gmail.com","username":"yugsuo"},"change_message_id":"8fcd04e0d6572cd8b3f1539a2447bb50cfaec436","unresolved":false,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        def get_samples(self, manager, cache, instance):"},{"line_number":87,"context_line":"            self.instances.append((manager, instance))"},{"line_number":88,"context_line":"            test_data_2 \u003d sample.Sample("},{"line_number":89,"context_line":"                name\u003d\u0027test2\u0027,"},{"line_number":90,"context_line":"                type\u003dsample.TYPE_CUMULATIVE,"},{"line_number":91,"context_line":"                unit\u003d\u0027units-go-here\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAASn%2F%2F5ZQ%3D","line":88,"in_reply_to":"AAAASn%2F%2F5bU%3D","updated":"2013-08-28 09:32:46.000000000","message":"Yes, I use util instead of Sample directly is to check the incorrect or incomplete instance type, if we lose some attributes, it will raise  AttributeError, which will result the number of the samples doesn\u0027t equal to 2.\n\nMaybe this is not a good way to test this, but we really should ensure the attributes used in util is existed","commit_id":"b06645076df71dc331df36e24bd6ffac109ea7ea"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"9b33461ff07af044668b425cffe552210df75745","unresolved":false,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        def get_samples(self, manager, cache, instance):"},{"line_number":87,"context_line":"            self.instances.append((manager, instance))"},{"line_number":88,"context_line":"            test_data_2 \u003d sample.Sample("},{"line_number":89,"context_line":"                name\u003d\u0027test2\u0027,"},{"line_number":90,"context_line":"                type\u003dsample.TYPE_CUMULATIVE,"},{"line_number":91,"context_line":"                unit\u003d\u0027units-go-here\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAASn%2F%2F5bU%3D","line":88,"in_reply_to":"AAAASn%2F%2F5oE%3D","updated":"2013-08-28 09:07:17.000000000","message":"Yep, I don\u0027t think it\u0027s a good practice to call util.make_sample_from_instace() here. Since we\u0027re passing the incorrect instance type. Sorry for the reverting. Please feel free let me know if you have any concerns. Thanks.","commit_id":"b06645076df71dc331df36e24bd6ffac109ea7ea"},{"author":{"_account_id":5055,"name":"Rain Suo","email":"yugsuo@gmail.com","username":"yugsuo"},"change_message_id":"8ba7f7163da7676a08ef1a1f1fc34543fcc86c56","unresolved":false,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        def get_samples(self, manager, cache, instance):"},{"line_number":87,"context_line":"            self.instances.append((manager, instance))"},{"line_number":88,"context_line":"            test_data_2 \u003d sample.Sample("},{"line_number":89,"context_line":"                name\u003d\u0027test2\u0027,"},{"line_number":90,"context_line":"                type\u003dsample.TYPE_CUMULATIVE,"},{"line_number":91,"context_line":"                unit\u003d\u0027units-go-here\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAASn%2F%2Fzg4%3D","line":88,"updated":"2013-08-29 10:39:08.000000000","message":"Sorry for more words, If you don\u0027t want to use util here, then there is no need to add another test right here, and the comments here  https://github.com/openstack/ceilometer/blob/master/nova_tests/test_notifier.py#L246 is useless.\n\nSo, there may be 3 options: the first one is to remove these useless code, the second one is to add other style tests to ensure instance attributes not missing, and the third one is to leave them as before.","commit_id":"aff668f38a761a8475c36d1b6d1b68c886574120"},{"author":{"_account_id":5055,"name":"Rain Suo","email":"yugsuo@gmail.com","username":"yugsuo"},"change_message_id":"6c00b44aaee3769e84e84aa408191f7c74b6a52a","unresolved":false,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        def get_samples(self, manager, cache, instance):"},{"line_number":87,"context_line":"            self.instances.append((manager, instance))"},{"line_number":88,"context_line":"            test_data_2 \u003d sample.Sample("},{"line_number":89,"context_line":"                name\u003d\u0027test2\u0027,"},{"line_number":90,"context_line":"                type\u003dsample.TYPE_CUMULATIVE,"},{"line_number":91,"context_line":"                unit\u003d\u0027units-go-here\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAASn%2F%2FzeA%3D","line":88,"in_reply_to":"AAAASn%2F%2Fzg4%3D","updated":"2013-08-29 10:46:51.000000000","message":"I prefer the third one. As you modified the util code, but didn\u0027t change the nova_notifier related code, so it raise Error during run_test right here. Now you change the nova_notifier, so there is no need to modify here.","commit_id":"aff668f38a761a8475c36d1b6d1b68c886574120"}]}