)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1561,"name":"Russell Bryant","email":"rbryant@redhat.com","username":"russellb"},"unresolved":false,"context_lines":[{"line_number":14,"context_line":"* documents classes, methods \u0026 API logic"},{"line_number":15,"context_line":"* rebased"},{"line_number":16,"context_line":"* rebased again"},{"line_number":17,"context_line":"* rebased and ported"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"fixes bug: #1178369"},{"line_number":20,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"AAAAPn%2F%2Fqxg%3D","line":17,"updated":"2013-06-25 16:22:02.000000000","message":"I would remove these last 3 lines.  Patch rev history really isn\u0027t useful for the commit message.","commit_id":"1c86dff336b96be3e9caa7d62182abd68250e52b"}],"nova/tests/test_vmwareapi_vm_util.py":[{"author":{"_account_id":5652,"name":"David Ripton","email":"dripton@redhat.com","username":"dripton"},"unresolved":false,"context_lines":[{"line_number":16,"context_line":"#    under the License."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from collections import namedtuple"},{"line_number":20,"context_line":"from nova import exception"},{"line_number":21,"context_line":"from nova import test"},{"line_number":22,"context_line":"from nova.virt.vmwareapi import fake"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAPH%2F%2FtQg%3D","line":19,"updated":"2013-05-24 20:36:29.000000000","message":"newline between stdlib imports and nova imports","commit_id":"66df139261121b97816b8013b75175a5ca2cbaac"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"unresolved":false,"context_lines":[{"line_number":14,"context_line":"#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the"},{"line_number":15,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":16,"context_line":"#    under the License."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from collections import namedtuple"},{"line_number":20,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAPH%2F%2Fls8%3D","line":17,"updated":"2013-05-30 06:26:31.000000000","message":"nit: extra line added here.","commit_id":"96c1f479c2af9aa04849e16d3229511889961834"}],"nova/tests/virt/vmwareapi/test_vmwareapi_vm_util.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"unresolved":false,"context_lines":[{"line_number":98,"context_line":"                                \u0027host-123\u0027, \u0027HostSystem\u0027))"},{"line_number":99,"context_line":"                ])]"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"        host_id \u003d\\"},{"line_number":102,"context_line":"            vm_util.get_host_id_from_vm_ref("},{"line_number":103,"context_line":"                fake_session(fake_results), vm_ref)"},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAPn%2F%2FoN0%3D","line":101,"updated":"2013-06-26 13:06:51.000000000","message":"minor nit - is it possible to write this without the \\\n\nfor example\n\nhost_id \u003d vm_util.get_host_id_from_vm_ref(\n        fake_session(fake_results), vm_ref)","commit_id":"8af374bba41fc8068e475551140dbb78bfe57701"},{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"3dfb23c075062bb5f08fc25d4118dd3461043640","unresolved":false,"context_lines":[{"line_number":98,"context_line":"                                \u0027host-123\u0027, \u0027HostSystem\u0027))"},{"line_number":99,"context_line":"                ])]"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"        host_id \u003d\\"},{"line_number":102,"context_line":"            vm_util.get_host_id_from_vm_ref("},{"line_number":103,"context_line":"                fake_session(fake_results), vm_ref)"},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAPn%2F%2FnQ0%3D","line":101,"in_reply_to":"AAAAPn%2F%2FoN0%3D","updated":"2013-06-26 18:17:36.000000000","message":"okay... how did that get in there? I hate \u003d \\ myself...","commit_id":"8af374bba41fc8068e475551140dbb78bfe57701"}],"nova/virt/vmwareapi/driver.py":[{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"cf0f0bf6f1fbe12e5bcfe9cb54ad37c012870043","unresolved":false,"context_lines":[{"line_number":402,"context_line":"                                   post_method, recover_method,"},{"line_number":403,"context_line":"                                   block_migration)"},{"line_number":404,"context_line":""},{"line_number":405,"context_line":"    def get_vnc_console(self, instance):"},{"line_number":406,"context_line":"        \"\"\"Return link to instance\u0027s VNC console using vCenter logic.\"\"\""},{"line_number":407,"context_line":"        return self._vmops.get_vnc_console_vcenter(instance)"},{"line_number":408,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAPH%2F%2FzRU%3D","line":405,"updated":"2013-05-22 18:46:57.000000000","message":"this method is already defined on line 241.","commit_id":"453d7d10aea49c7c5a30772971e51ec531b0b5a0"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"unresolved":false,"context_lines":[{"line_number":402,"context_line":"                                   post_method, recover_method,"},{"line_number":403,"context_line":"                                   block_migration)"},{"line_number":404,"context_line":""},{"line_number":405,"context_line":"    def get_vnc_console(self, instance):"},{"line_number":406,"context_line":"        \"\"\"Return link to instance\u0027s VNC console using vCenter logic.\"\"\""},{"line_number":407,"context_line":"        return self._vmops.get_vnc_console_vcenter(instance)"},{"line_number":408,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAPH%2F%2FtVo%3D","line":405,"updated":"2013-05-26 01:34:32.000000000","message":"There is already a def get_vnc_console() defined on line 241 in this class.","commit_id":"66df139261121b97816b8013b75175a5ca2cbaac"},{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"unresolved":false,"context_lines":[{"line_number":402,"context_line":"                                   post_method, recover_method,"},{"line_number":403,"context_line":"                                   block_migration)"},{"line_number":404,"context_line":""},{"line_number":405,"context_line":"    def get_vnc_console(self, instance):"},{"line_number":406,"context_line":"        \"\"\"Return link to instance\u0027s VNC console using vCenter logic.\"\"\""},{"line_number":407,"context_line":"        return self._vmops.get_vnc_console_vcenter(instance)"},{"line_number":408,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAPH%2F%2FsHQ%3D","line":405,"in_reply_to":"AAAAPH%2F%2FtVo%3D","updated":"2013-05-27 00:00:14.000000000","message":"Aaron, that is actually a different class (ESXDriver) that this class (VCDriver) extends.  We need to add get_vnc_console here b/c the behavior is substantially different when using VCDriver.","commit_id":"66df139261121b97816b8013b75175a5ca2cbaac"}],"nova/virt/vmwareapi/fake.py":[{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"unresolved":false,"context_lines":[{"line_number":101,"context_line":"class ObjectContent(object):"},{"line_number":102,"context_line":"    \"\"\"ObjectContent array hold dynamic properties.\"\"\""},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    def __init__(self, obj_ref\u003dNone, propSet\u003d[], missingSet\u003d[]):"},{"line_number":105,"context_line":"        self.obj \u003d obj_ref"},{"line_number":106,"context_line":"        self.propSet \u003d propSet"},{"line_number":107,"context_line":"        self.missingSet \u003d missingSet"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAPH%2F%2FlpI%3D","line":104,"updated":"2013-05-30 06:26:31.000000000","message":"Is there any reason why we need to have the default prototypes as \u003d[] ? Can\u0027t we just store this internally in this class rather than in the function.","commit_id":"96c1f479c2af9aa04849e16d3229511889961834"},{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"3dfb23c075062bb5f08fc25d4118dd3461043640","unresolved":false,"context_lines":[{"line_number":101,"context_line":"class ObjectContent(object):"},{"line_number":102,"context_line":"    \"\"\"ObjectContent array hold dynamic properties.\"\"\""},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    def __init__(self, obj_ref\u003dNone, propSet\u003d[], missingSet\u003d[]):"},{"line_number":105,"context_line":"        self.obj \u003d obj_ref"},{"line_number":106,"context_line":"        self.propSet \u003d propSet"},{"line_number":107,"context_line":"        self.missingSet \u003d missingSet"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAPH%2F%2Fitc%3D","line":104,"in_reply_to":"AAAAPH%2F%2FlpI%3D","updated":"2013-05-30 21:18:51.000000000","message":"both of these will always be lists. If you don\u0027t \"prime\" default lists to be empty (instead setting them to \"None\") you write the following 50,000 times:\n\nif oc.propSet is not None:\n  for p in oc.propSet:\n  #... blah\n\nas opposed to just doing\n\nfor p in oc.propSet:\n  #... blah\n\nAlso...\n\noc.propSet.append(x)\n\n... becomes ...\n\nif oc.propSet is None:\n  oc.propSet \u003d [x]\nelif:\n  oc.propSet.append(x)\n\n... which takes a simple case blowing it out to a bigger case.\n\nAlso, the VMware API will return a list that is empty not a None when there are no properties in the list.\n\nSo on multiple counts using an empty list here is the right thing to do. How we get the empty list into the object probably doesn\u0027t matter.","commit_id":"96c1f479c2af9aa04849e16d3229511889961834"},{"author":{"_account_id":5652,"name":"David Ripton","email":"dripton@redhat.com","username":"dripton"},"unresolved":false,"context_lines":[{"line_number":102,"context_line":"class ObjectContent(object):"},{"line_number":103,"context_line":"    \"\"\"ObjectContent array hold dynamic properties.\"\"\""},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def __init__(self, obj_ref, propSet\u003dNone, missingSet\u003dNone):"},{"line_number":106,"context_line":"        self.obj \u003d obj_ref"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"        if not isinstance(propSet, collections.Iterable):"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAPH%2F%2Fg8c%3D","line":105,"updated":"2013-05-31 13:55:02.000000000","message":"Two things:\n\n1. pep8.  underscores not camelcase\n2. it\u0027s kind of misleading to put set in the name of a list","commit_id":"8b45e402024e0c8b07fbc2078f54df9b9e9b87ec"},{"author":{"_account_id":1561,"name":"Russell Bryant","email":"rbryant@redhat.com","username":"russellb"},"unresolved":false,"context_lines":[{"line_number":101,"context_line":"        # typically have values like vm-123 or"},{"line_number":102,"context_line":"        # host-232 and not UUID"},{"line_number":103,"context_line":"        object.__setattr__(self, \u0027value\u0027, value)"},{"line_number":104,"context_line":"        object.__setattr__(self, \u0027_type\u0027, _type)"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"class ObjectContent(object):"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAPn%2F%2Fou0%3D","line":104,"updated":"2013-06-26 08:29:44.000000000","message":"Why is this alternate syntax needed instead instead of:\n\n    self.value \u003d value\n    self._type \u003d _type\n\nIt seems effectively identical to the Prop class ...","commit_id":"8af374bba41fc8068e475551140dbb78bfe57701"},{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"3dfb23c075062bb5f08fc25d4118dd3461043640","unresolved":false,"context_lines":[{"line_number":101,"context_line":"        # typically have values like vm-123 or"},{"line_number":102,"context_line":"        # host-232 and not UUID"},{"line_number":103,"context_line":"        object.__setattr__(self, \u0027value\u0027, value)"},{"line_number":104,"context_line":"        object.__setattr__(self, \u0027_type\u0027, _type)"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"class ObjectContent(object):"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAPn%2F%2FnRM%3D","line":104,"in_reply_to":"AAAAPn%2F%2Fou0%3D","updated":"2013-06-26 18:17:36.000000000","message":"I think I was trying to avoid a merge conflict with another patch in flight right now. I don\u0027t have a good reason. I debated making this immutable since it\u0027s *supposed* to be read-only in the API but there\u0027s no immutability enforcement in the SOAP binding.\n\nI\u0027ll just make this change.","commit_id":"8af374bba41fc8068e475551140dbb78bfe57701"}],"nova/virt/vmwareapi/vm_util.py":[{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"cf0f0bf6f1fbe12e5bcfe9cb54ad37c012870043","unresolved":false,"context_lines":[{"line_number":502,"context_line":"    host_ref \u003d None"},{"line_number":503,"context_line":"    if host_id is not None:"},{"line_number":504,"context_line":""},{"line_number":505,"context_line":"        host_refs \u003d\\"},{"line_number":506,"context_line":"            session._call_method("},{"line_number":507,"context_line":"                vim_util, \"get_objects\","},{"line_number":508,"context_line":"                \"HostSystem\", props"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAPH%2F%2FzRQ%3D","line":505,"updated":"2013-05-22 18:46:57.000000000","message":"nit: please use (  ) over \\","commit_id":"453d7d10aea49c7c5a30772971e51ec531b0b5a0"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"cf0f0bf6f1fbe12e5bcfe9cb54ad37c012870043","unresolved":false,"context_lines":[{"line_number":510,"context_line":""},{"line_number":511,"context_line":"        for ref in host_refs:"},{"line_number":512,"context_line":"            if host_id \u003d\u003d ref.obj.value:"},{"line_number":513,"context_line":"                host_ref \u003d ref"},{"line_number":514,"context_line":""},{"line_number":515,"context_line":"    return host_ref"},{"line_number":516,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAPH%2F%2FzRM%3D","line":513,"updated":"2013-05-22 18:46:57.000000000","message":"return ref?","commit_id":"453d7d10aea49c7c5a30772971e51ec531b0b5a0"},{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"3dfb23c075062bb5f08fc25d4118dd3461043640","unresolved":false,"context_lines":[{"line_number":510,"context_line":""},{"line_number":511,"context_line":"        for ref in host_refs:"},{"line_number":512,"context_line":"            if host_id \u003d\u003d ref.obj.value:"},{"line_number":513,"context_line":"                host_ref \u003d ref"},{"line_number":514,"context_line":""},{"line_number":515,"context_line":"    return host_ref"},{"line_number":516,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAPH%2F%2FvIc%3D","line":513,"in_reply_to":"AAAAPH%2F%2FzRM%3D","updated":"2013-05-24 04:46:47.000000000","message":"return from inside a loop? That\u0027s worse than using a mutable return value.","commit_id":"453d7d10aea49c7c5a30772971e51ec531b0b5a0"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"cf0f0bf6f1fbe12e5bcfe9cb54ad37c012870043","unresolved":false,"context_lines":[{"line_number":514,"context_line":""},{"line_number":515,"context_line":"    return host_ref"},{"line_number":516,"context_line":""},{"line_number":517,"context_line":"def get_host_ref_for_vm(session, vm_name, props\u003d[\"name\"]):"},{"line_number":518,"context_line":"    \"\"\"Get the ESXi host running a VM by its name.\"\"\""},{"line_number":519,"context_line":"    vm_ref \u003d get_vm_ref_from_name(session, vm_name)"},{"line_number":520,"context_line":"    properties \u003d \\"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAPH%2F%2FzQs%3D","line":517,"updated":"2013-05-22 18:46:57.000000000","message":"Should not use lists as the default as they have side effects. For example: \n\nIn [2]: def foo(bar\u003d[\u0027name\u0027]):\n   ...:     print bar\n   ...:     bar.append(\u0027blah\u0027)\n   ...:     \n\nIn [3]: foo()\n[\u0027name\u0027]\n\nIn [4]: foo()\n[\u0027name\u0027, \u0027blah\u0027]\n\nIn [5]: foo()\n[\u0027name\u0027, \u0027blah\u0027, \u0027blah\u0027]","commit_id":"453d7d10aea49c7c5a30772971e51ec531b0b5a0"},{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"3dfb23c075062bb5f08fc25d4118dd3461043640","unresolved":false,"context_lines":[{"line_number":514,"context_line":""},{"line_number":515,"context_line":"    return host_ref"},{"line_number":516,"context_line":""},{"line_number":517,"context_line":"def get_host_ref_for_vm(session, vm_name, props\u003d[\"name\"]):"},{"line_number":518,"context_line":"    \"\"\"Get the ESXi host running a VM by its name.\"\"\""},{"line_number":519,"context_line":"    vm_ref \u003d get_vm_ref_from_name(session, vm_name)"},{"line_number":520,"context_line":"    properties \u003d \\"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAPH%2F%2FvIM%3D","line":517,"in_reply_to":"AAAAPH%2F%2FzQs%3D","updated":"2013-05-24 04:46:47.000000000","message":"In this case, we would want that behavior since this is a list for a property collector designed to accrete values.","commit_id":"453d7d10aea49c7c5a30772971e51ec531b0b5a0"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"cf0f0bf6f1fbe12e5bcfe9cb54ad37c012870043","unresolved":false,"context_lines":[{"line_number":525,"context_line":"    for property in properties:"},{"line_number":526,"context_line":"        for prop in property.propSet:"},{"line_number":527,"context_line":"            if prop.name \u003d\u003d \"runtime.host\":"},{"line_number":528,"context_line":"                host_id \u003d prop.val.value"},{"line_number":529,"context_line":""},{"line_number":530,"context_line":"    return get_host_ref_from_id(session, host_id, props)"},{"line_number":531,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAPH%2F%2FzQg%3D","line":528,"updated":"2013-05-22 18:46:57.000000000","message":"break?","commit_id":"453d7d10aea49c7c5a30772971e51ec531b0b5a0"},{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"3dfb23c075062bb5f08fc25d4118dd3461043640","unresolved":false,"context_lines":[{"line_number":525,"context_line":"    for property in properties:"},{"line_number":526,"context_line":"        for prop in property.propSet:"},{"line_number":527,"context_line":"            if prop.name \u003d\u003d \"runtime.host\":"},{"line_number":528,"context_line":"                host_id \u003d prop.val.value"},{"line_number":529,"context_line":""},{"line_number":530,"context_line":"    return get_host_ref_from_id(session, host_id, props)"},{"line_number":531,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAPH%2F%2FvIE%3D","line":528,"in_reply_to":"AAAAPH%2F%2FzQg%3D","updated":"2013-05-24 04:46:47.000000000","message":"good point.","commit_id":"453d7d10aea49c7c5a30772971e51ec531b0b5a0"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"cf0f0bf6f1fbe12e5bcfe9cb54ad37c012870043","unresolved":false,"context_lines":[{"line_number":535,"context_line":"    return get_host_name_from_host_ref(host_ref)"},{"line_number":536,"context_line":""},{"line_number":537,"context_line":"def get_host_name_from_host_ref(host_ref):"},{"line_number":538,"context_line":"    host_name \u003d None"},{"line_number":539,"context_line":"    for prop in host_ref.propSet:"},{"line_number":540,"context_line":"        if prop.name \u003d\u003d \"name\":"},{"line_number":541,"context_line":"            host_name \u003d prop.val"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAPH%2F%2FzQU%3D","line":538,"updated":"2013-05-22 18:46:57.000000000","message":"nit: not needed","commit_id":"453d7d10aea49c7c5a30772971e51ec531b0b5a0"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"cf0f0bf6f1fbe12e5bcfe9cb54ad37c012870043","unresolved":false,"context_lines":[{"line_number":538,"context_line":"    host_name \u003d None"},{"line_number":539,"context_line":"    for prop in host_ref.propSet:"},{"line_number":540,"context_line":"        if prop.name \u003d\u003d \"name\":"},{"line_number":541,"context_line":"            host_name \u003d prop.val"},{"line_number":542,"context_line":"    return host_name"},{"line_number":543,"context_line":""},{"line_number":544,"context_line":"def get_cluster_ref_from_name(session, cluster_name):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAPH%2F%2FzQY%3D","line":541,"updated":"2013-05-22 18:46:57.000000000","message":"return?","commit_id":"453d7d10aea49c7c5a30772971e51ec531b0b5a0"},{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"3dfb23c075062bb5f08fc25d4118dd3461043640","unresolved":false,"context_lines":[{"line_number":538,"context_line":"    host_name \u003d None"},{"line_number":539,"context_line":"    for prop in host_ref.propSet:"},{"line_number":540,"context_line":"        if prop.name \u003d\u003d \"name\":"},{"line_number":541,"context_line":"            host_name \u003d prop.val"},{"line_number":542,"context_line":"    return host_name"},{"line_number":543,"context_line":""},{"line_number":544,"context_line":"def get_cluster_ref_from_name(session, cluster_name):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAPH%2F%2FvHg%3D","line":541,"in_reply_to":"AAAAPH%2F%2FzQY%3D","updated":"2013-05-24 04:46:47.000000000","message":"That violates \"single point of return\"","commit_id":"453d7d10aea49c7c5a30772971e51ec531b0b5a0"},{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"unresolved":false,"context_lines":[{"line_number":504,"context_line":""},{"line_number":505,"context_line":"    host_refs \u003d session._call_method("},{"line_number":506,"context_line":"                    vim_util, \"get_objects\","},{"line_number":507,"context_line":"                     \"HostSystem\", props[:])"},{"line_number":508,"context_line":""},{"line_number":509,"context_line":"    refs \u003d [ref for ref in host_refs if host_id \u003d\u003d ref.obj.value]"},{"line_number":510,"context_line":"    return refs[0] if len(refs) \u003e 0 else None"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAPH%2F%2FsG0%3D","line":507,"updated":"2013-05-27 00:00:14.000000000","message":"is a copy of props needed because the call to session modifies the list?","commit_id":"66df139261121b97816b8013b75175a5ca2cbaac"},{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"unresolved":false,"context_lines":[{"line_number":515,"context_line":""},{"line_number":516,"context_line":"    property_set \u003d session._call_method("},{"line_number":517,"context_line":"            vim_util, \"get_object_properties\","},{"line_number":518,"context_line":"            None, vm_ref, vm_ref._type, [property_name])"},{"line_number":519,"context_line":""},{"line_number":520,"context_line":"    prop \u003d property_from_property_set("},{"line_number":521,"context_line":"        property_name, property_set)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAPH%2F%2FsGM%3D","line":518,"updated":"2013-05-27 00:00:14.000000000","message":"does this mean that VNC will break if a VM is moved due to DRS, vMotion, or HA?  If so, we should make a note of this in docs and see whether there is something we can do to address that (presumably higher up in the chain?)  Other hypervisors in OpenStack likely have the same problem, but it may be a bigger issue for VMware as people actually expect vMotion to work for Vmware :)","commit_id":"66df139261121b97816b8013b75175a5ca2cbaac"},{"author":{"_account_id":5652,"name":"David Ripton","email":"dripton@redhat.com","username":"dripton"},"unresolved":false,"context_lines":[{"line_number":499,"context_line":"    return None"},{"line_number":500,"context_line":""},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"def get_host_ref_from_id(session, host_id, props\u003d[\"name\"]):"},{"line_number":503,"context_line":"    \"\"\"Get a host reference object for a host_id string.\"\"\""},{"line_number":504,"context_line":""},{"line_number":505,"context_line":"    # using value-copy of props list to make surprise"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAPH%2F%2FlrM%3D","line":502,"updated":"2013-05-30 21:24:13.000000000","message":"Using a mutable as a default argument is dangerous in Python, because it gets assigned once when the def runs and then is reused across calls to the function with surprising results.  You want either props\u003d(\"name\") or props\u003dNone and then a later if props is None: props \u003d [\"name\"]\n\n(I don\u0027t you don\u0027t actually mutate props, but it\u0027s a fail-later-when-changed.)","commit_id":"96c1f479c2af9aa04849e16d3229511889961834"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"unresolved":false,"context_lines":[{"line_number":502,"context_line":"def get_host_ref_from_id(session, host_id, props\u003d[\"name\"]):"},{"line_number":503,"context_line":"    \"\"\"Get a host reference object for a host_id string.\"\"\""},{"line_number":504,"context_line":""},{"line_number":505,"context_line":"    # using value-copy of props list to make surprise"},{"line_number":506,"context_line":"    # side-effects to the list held by calling method (above us)"},{"line_number":507,"context_line":"    # impossible (below us)."},{"line_number":508,"context_line":"    # (This is a defensive coding practice creating"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAPH%2F%2Fkzc%3D","line":505,"updated":"2013-05-30 06:26:31.000000000","message":"To be honest i\u0027m still not sure why we don\u0027t want to do: \ndef .. (props \u003d None)\nprops \u003d list(porps, \u0027name\u0027) having a list as a default argument is usually frowned upon.","commit_id":"96c1f479c2af9aa04849e16d3229511889961834"},{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"3dfb23c075062bb5f08fc25d4118dd3461043640","unresolved":false,"context_lines":[{"line_number":502,"context_line":"def get_host_ref_from_id(session, host_id, props\u003d[\"name\"]):"},{"line_number":503,"context_line":"    \"\"\"Get a host reference object for a host_id string.\"\"\""},{"line_number":504,"context_line":""},{"line_number":505,"context_line":"    # using value-copy of props list to make surprise"},{"line_number":506,"context_line":"    # side-effects to the list held by calling method (above us)"},{"line_number":507,"context_line":"    # impossible (below us)."},{"line_number":508,"context_line":"    # (This is a defensive coding practice creating"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAPH%2F%2Fips%3D","line":505,"in_reply_to":"AAAAPH%2F%2FisY%3D","updated":"2013-05-30 21:47:21.000000000","message":"Ugh. I see it now. I just did some reading.\n\nThe default list instantiates with the method declaration and the default list \"pointer\" sits pointing at the memory block allocated during the method\u0027s load-up and not on the activation frame created during the method\u0027s invocation. I see. The method definition itself is a proper object that is initialized once and held as a reference, it\u0027s not a template that is cloned and it\u0027s not re-initialized on subsequent passes.\n\nI would rather have no default here at all then.","commit_id":"96c1f479c2af9aa04849e16d3229511889961834"},{"author":{"_account_id":5652,"name":"David Ripton","email":"dripton@redhat.com","username":"dripton"},"unresolved":false,"context_lines":[{"line_number":502,"context_line":"def get_host_ref_from_id(session, host_id, props\u003d[\"name\"]):"},{"line_number":503,"context_line":"    \"\"\"Get a host reference object for a host_id string.\"\"\""},{"line_number":504,"context_line":""},{"line_number":505,"context_line":"    # using value-copy of props list to make surprise"},{"line_number":506,"context_line":"    # side-effects to the list held by calling method (above us)"},{"line_number":507,"context_line":"    # impossible (below us)."},{"line_number":508,"context_line":"    # (This is a defensive coding practice creating"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAPH%2F%2FisY%3D","line":505,"in_reply_to":"AAAAPH%2F%2Fix8%3D","updated":"2013-05-30 21:24:13.000000000","message":"But if you put an empty list your code is *broken* unless you meant for modifications to that list to persist across calls.  This is not a style point ; it\u0027s a danger point.\n\nWe should add a hacking.py check for mutable default args.","commit_id":"96c1f479c2af9aa04849e16d3229511889961834"},{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"3dfb23c075062bb5f08fc25d4118dd3461043640","unresolved":false,"context_lines":[{"line_number":502,"context_line":"def get_host_ref_from_id(session, host_id, props\u003d[\"name\"]):"},{"line_number":503,"context_line":"    \"\"\"Get a host reference object for a host_id string.\"\"\""},{"line_number":504,"context_line":""},{"line_number":505,"context_line":"    # using value-copy of props list to make surprise"},{"line_number":506,"context_line":"    # side-effects to the list held by calling method (above us)"},{"line_number":507,"context_line":"    # impossible (below us)."},{"line_number":508,"context_line":"    # (This is a defensive coding practice creating"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAPH%2F%2Fix8%3D","line":505,"in_reply_to":"AAAAPH%2F%2Fkzc%3D","updated":"2013-05-30 21:18:51.000000000","message":"because you don\u0027t put \"None\" when you can put empty list... if you put empty list you can go:\n\nfor p in props:\n\nif you don\u0027t put an empty list you go:\n\nif props is not None:\n  for p in props:\n\nI would rather have no default value than put the default for a list as \"None\"","commit_id":"96c1f479c2af9aa04849e16d3229511889961834"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"unresolved":false,"context_lines":[{"line_number":512,"context_line":"                    vim_util, \"get_objects\","},{"line_number":513,"context_line":"                     \"HostSystem\", props[:])"},{"line_number":514,"context_line":""},{"line_number":515,"context_line":"    refs \u003d [ref for ref in host_refs if host_id \u003d\u003d ref.obj.value]"},{"line_number":516,"context_line":"    return refs[0] if len(refs) \u003e 0 else None"},{"line_number":517,"context_line":""},{"line_number":518,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAPH%2F%2Fkz0%3D","line":515,"updated":"2013-05-30 06:26:31.000000000","message":"same comments as below.","commit_id":"96c1f479c2af9aa04849e16d3229511889961834"},{"author":{"_account_id":5652,"name":"David Ripton","email":"dripton@redhat.com","username":"dripton"},"unresolved":false,"context_lines":[{"line_number":578,"context_line":"    # object tree we are interested in in this call."},{"line_number":579,"context_line":"    results \u003d [prop for prop in property_set if"},{"line_number":580,"context_line":"                [p for p in prop.propSet"},{"line_number":581,"context_line":"                        if p.name \u003d\u003d property_name]]"},{"line_number":582,"context_line":""},{"line_number":583,"context_line":"    # here, we fish the actual found value from the result set"},{"line_number":584,"context_line":"    prop \u003d None"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAPH%2F%2Flq8%3D","line":581,"updated":"2013-05-30 21:24:13.000000000","message":"The double-if makes this is a bit too hard to read and understand.  Can you change it to a normal loop?","commit_id":"96c1f479c2af9aa04849e16d3229511889961834"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"unresolved":false,"context_lines":[{"line_number":582,"context_line":""},{"line_number":583,"context_line":"    # here, we fish the actual found value from the result set"},{"line_number":584,"context_line":"    prop \u003d None"},{"line_number":585,"context_line":"    if len(results) \u003e 0:"},{"line_number":586,"context_line":"        result \u003d results[0]"},{"line_number":587,"context_line":"        props \u003d [p for p in result.propSet if p.name \u003d\u003d property_name]"},{"line_number":588,"context_line":"        prop \u003d props[0]"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAPH%2F%2FlWw%3D","line":585,"updated":"2013-05-30 06:26:31.000000000","message":"can we refactor this so it doesn\u0027t have to step though all these list comprehensions. If we used two nested fors we could have return the value right when we found it.","commit_id":"96c1f479c2af9aa04849e16d3229511889961834"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"unresolved":false,"context_lines":[{"line_number":606,"context_line":""},{"line_number":607,"context_line":""},{"line_number":608,"context_line":"def get_host_name_from_host_ref(host_ref):"},{"line_number":609,"context_line":"    props \u003d [p for p in host_ref.propSet if p.name \u003d\u003d \u0027name\u0027]"},{"line_number":610,"context_line":"    return props[0].val if len(props) \u003e 0 else None"},{"line_number":611,"context_line":""},{"line_number":612,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAPH%2F%2FlXY%3D","line":609,"updated":"2013-05-30 06:26:31.000000000","message":"nit: consider\nfor p in host_ref.propSet:\n     if p.name \u003d\u003d \u0027name\u0027:\n            return p.val\n\nThen you don\u0027t need the if statement on the return line.","commit_id":"96c1f479c2af9aa04849e16d3229511889961834"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"unresolved":false,"context_lines":[{"line_number":606,"context_line":""},{"line_number":607,"context_line":""},{"line_number":608,"context_line":"def get_host_name_from_host_ref(host_ref):"},{"line_number":609,"context_line":"    props \u003d [p for p in host_ref.propSet if p.name \u003d\u003d \u0027name\u0027]"},{"line_number":610,"context_line":"    return props[0].val if len(props) \u003e 0 else None"},{"line_number":611,"context_line":""},{"line_number":612,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAPH%2F%2Fino%3D","line":609,"in_reply_to":"AAAAPH%2F%2FiyI%3D","updated":"2013-05-30 21:56:16.000000000","message":"Why? we want to return the first instance of p.name \u003d\u003d\u0027name\u0027.","commit_id":"96c1f479c2af9aa04849e16d3229511889961834"},{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"3dfb23c075062bb5f08fc25d4118dd3461043640","unresolved":false,"context_lines":[{"line_number":606,"context_line":""},{"line_number":607,"context_line":""},{"line_number":608,"context_line":"def get_host_name_from_host_ref(host_ref):"},{"line_number":609,"context_line":"    props \u003d [p for p in host_ref.propSet if p.name \u003d\u003d \u0027name\u0027]"},{"line_number":610,"context_line":"    return props[0].val if len(props) \u003e 0 else None"},{"line_number":611,"context_line":""},{"line_number":612,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAPH%2F%2FiyI%3D","line":609,"in_reply_to":"AAAAPH%2F%2FlXY%3D","updated":"2013-05-30 21:18:51.000000000","message":"I really really really hate returning from inside a nested statement. I don\u0027t think I can state how much I hate that.","commit_id":"96c1f479c2af9aa04849e16d3229511889961834"}],"nova/virt/vmwareapi/vmops.py":[{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"unresolved":false,"context_lines":[{"line_number":1117,"context_line":"        vnc_console \u003d self.get_vnc_console(instance)"},{"line_number":1118,"context_line":"        host_name \u003d vm_util.get_host_name_for_vm("},{"line_number":1119,"context_line":"                        self._session,"},{"line_number":1120,"context_line":"                        instance[\u0027name\u0027])"},{"line_number":1121,"context_line":"        vnc_console[\u0027host\u0027] \u003d host_name"},{"line_number":1122,"context_line":"        return vnc_console"},{"line_number":1123,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAPH%2F%2FsGI%3D","line":1120,"updated":"2013-05-27 00:00:14.000000000","message":"this can return None... seems like we should log something if it does and potentially not return a \"normal\" looking result.","commit_id":"66df139261121b97816b8013b75175a5ca2cbaac"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"unresolved":false,"context_lines":[{"line_number":1100,"context_line":""},{"line_number":1101,"context_line":"        # NOTE: VM can move hosts in some situations."},{"line_number":1102,"context_line":"        LOG.debug("},{"line_number":1103,"context_line":"            \"VM %s is currently on host %s\" % ("},{"line_number":1104,"context_line":"                instance[\u0027name\u0027], host_name))"},{"line_number":1105,"context_line":""},{"line_number":1106,"context_line":"        return vnc_console"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAPn%2F%2FoNk%3D","line":1103,"updated":"2013-06-26 13:06:51.000000000","message":"please use the i18n format\n\nthis should be:\nLOG.debug(_(\"VM %(instance)s is currently on host %(host)s\"),\n        {\u0027instance\u0027: instance[\u0027name\u0027], \u0027host\u0027: host_name})","commit_id":"8af374bba41fc8068e475551140dbb78bfe57701"},{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"3dfb23c075062bb5f08fc25d4118dd3461043640","unresolved":false,"context_lines":[{"line_number":1100,"context_line":""},{"line_number":1101,"context_line":"        # NOTE: VM can move hosts in some situations."},{"line_number":1102,"context_line":"        LOG.debug("},{"line_number":1103,"context_line":"            \"VM %s is currently on host %s\" % ("},{"line_number":1104,"context_line":"                instance[\u0027name\u0027], host_name))"},{"line_number":1105,"context_line":""},{"line_number":1106,"context_line":"        return vnc_console"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAPn%2F%2FnRE%3D","line":1103,"in_reply_to":"AAAAPn%2F%2FoNk%3D","updated":"2013-06-26 18:17:36.000000000","message":"okay","commit_id":"8af374bba41fc8068e475551140dbb78bfe57701"}]}