)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"01f16466812e433693677065c047744d5727bb33","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Sylvain Bauza \u003csbauza@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-12-11 16:23:27 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Modify Scheduler RPC API select_dests()"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Since we now have a RequestSpec object, we can directly provide it thru"},{"line_number":10,"context_line":"the RPC API select_destinations(). Since the conductor also uses the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"5a710552_ae3633e5","line":7,"updated":"2015-12-15 17:00:28.000000000","message":"Maybe you\u0027re just trying to get in under the number or characters for the first line of a commit message, but this line misled me: I was expecting to find a new method on the scheduler named `select_dests` which accepts a RequestSpect and is proxied to by select_destinations(). But that\u0027s not what the code does, so it might make sense to not abbreviate.","commit_id":"64b487b151f7dc7fb2c3b6eac3678531b98a6806"}],"nova/scheduler/client/query.py":[{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"1dd3c62b1bd2aa8574e0bc24ac6b33cf2f5a4a80","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        \"\"\""},{"line_number":33,"context_line":"        # TODO(sbauza): Provide directly the RequestSpec object as an arg"},{"line_number":34,"context_line":"        # once the RPC API is modified for that."},{"line_number":35,"context_line":"        request_spec \u003d spec_obj.to_legacy_request_spec_dict()"},{"line_number":36,"context_line":"        filter_properties \u003d spec_obj.to_legacy_filter_properties_dict()"},{"line_number":37,"context_line":"        return self.scheduler_rpcapi.select_destinations(context, request_spec,"},{"line_number":38,"context_line":"                                                         filter_properties)"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def update_aggregates(self, context, aggregates):"},{"line_number":41,"context_line":"        \"\"\"Updates HostManager internal aggregates information."}],"source_content_type":"text/x-python","patch_set":39,"id":"5a710552_daf8ef4b","side":"PARENT","line":38,"range":{"start_line":35,"start_character":8,"end_line":38,"end_character":75},"updated":"2015-12-16 11:59:13.000000000","message":"let\u0027s just please squash these 2 patches....","commit_id":"f32377bcdf672b0ba2aa69985171fc1ff4246b0f"}],"nova/scheduler/manager.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"06389e40bcf809f388869c7ff80a5d8bded9cc00","unresolved":false,"context_lines":[{"line_number":65,"context_line":"                                               *args, **kwargs)"},{"line_number":66,"context_line":"        self.additional_endpoints.append(_SchedulerManagerV5Proxy(self))"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    @periodic_task.periodic_task"},{"line_number":69,"context_line":"    def _expire_reservations(self, context):"},{"line_number":70,"context_line":"        QUOTAS.expire(context)"},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":36,"id":"9a8ffd7b_009a346e","line":68,"updated":"2015-11-27 14:29:31.000000000","message":"should these periodic tasks not move to the new class?","commit_id":"20daa897c5b0947d4c28cc6651d23dbde188752a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"06389e40bcf809f388869c7ff80a5d8bded9cc00","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        The result should be a list of dicts with \u0027host\u0027, \u0027nodename\u0027 and"},{"line_number":83,"context_line":"        \u0027limits\u0027 as keys."},{"line_number":84,"context_line":"        \"\"\""},{"line_number":85,"context_line":"        spec_obj \u003d objects.RequestSpec.from_primitives(context,"},{"line_number":86,"context_line":"                                                       request_spec,"},{"line_number":87,"context_line":"                                                       filter_properties)"},{"line_number":88,"context_line":"        dests \u003d self.driver.select_destinations(context, spec_obj)"}],"source_content_type":"text/x-python","patch_set":36,"id":"9a8ffd7b_c0bc2cbd","line":85,"updated":"2015-11-27 14:29:31.000000000","message":"I think it would be easier to make filter_properties optional, and inspect the type of request_spec to see if its an object or not.\n\nThen leave the major version bump till the end of the release?\n\nBut I am open to doing this now, just feel we need to complete the task.","commit_id":"20daa897c5b0947d4c28cc6651d23dbde188752a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"06389e40bcf809f388869c7ff80a5d8bded9cc00","unresolved":false,"context_lines":[{"line_number":96,"context_line":"                          or :class:`nova.objects.AggregateList`"},{"line_number":97,"context_line":"        \"\"\""},{"line_number":98,"context_line":"        # NOTE(sbauza): We\u0027re dropping the user context now as we don\u0027t need it"},{"line_number":99,"context_line":"        self.driver.host_manager.update_aggregates(aggregates)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def delete_aggregate(self, ctxt, aggregate):"},{"line_number":102,"context_line":"        \"\"\"Deletes HostManager internal information about a specific aggregate."}],"source_content_type":"text/x-python","patch_set":36,"id":"9a8ffd7b_406cbc34","line":99,"updated":"2015-11-27 14:29:31.000000000","message":"I think I prefer calling through to the v5 manager, rather than the cut and paste here, even though its really quite a small one.","commit_id":"20daa897c5b0947d4c28cc6651d23dbde188752a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"06389e40bcf809f388869c7ff80a5d8bded9cc00","unresolved":false,"context_lines":[{"line_number":129,"context_line":"                                                    instance_uuids)"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"class _SchedulerManagerV5Proxy(object):"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"    target \u003d messaging.Target(version\u003d\u00275.0\u0027)"},{"line_number":135,"context_line":""}],"source_content_type":"text/x-python","patch_set":36,"id":"9a8ffd7b_0053143a","line":132,"updated":"2015-11-27 14:29:31.000000000","message":"wondering about the pattern we use here:\nhttps://github.com/openstack/nova/blob/stable/liberty/nova/conductor/manager.py#L924\n\nIt does assume its about to be dropped, where this way around, we probably want the proxy to be the 4.x version, that just calls the other manager? just so its easier to create 5.1","commit_id":"20daa897c5b0947d4c28cc6651d23dbde188752a"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"c19f60536afa5943f6e3c86ea6ed8b99cfa97cd7","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        dests \u003d self.driver.select_destinations(context, spec_obj)"},{"line_number":89,"context_line":"        return jsonutils.to_primitive(dests)"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    def update_aggregates(self, ctxt, aggregates):"},{"line_number":92,"context_line":"        \"\"\"Updates HostManager internal aggregates information."},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        :param aggregates: Aggregate(s) to update"},{"line_number":95,"context_line":"        :type aggregates: :class:`nova.objects.Aggregate`"},{"line_number":96,"context_line":"                          or :class:`nova.objects.AggregateList`"},{"line_number":97,"context_line":"        \"\"\""},{"line_number":98,"context_line":"        # NOTE(sbauza): We\u0027re dropping the user context now as we don\u0027t need it"},{"line_number":99,"context_line":"        self.driver.host_manager.update_aggregates(aggregates)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def delete_aggregate(self, ctxt, aggregate):"},{"line_number":102,"context_line":"        \"\"\"Deletes HostManager internal information about a specific aggregate."},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        :param aggregate: Aggregate to delete"},{"line_number":105,"context_line":"        :type aggregate: :class:`nova.objects.Aggregate`"},{"line_number":106,"context_line":"        \"\"\""},{"line_number":107,"context_line":"        # NOTE(sbauza): We\u0027re dropping the user context now as we don\u0027t need it"},{"line_number":108,"context_line":"        self.driver.host_manager.delete_aggregate(aggregate)"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    def update_instance_info(self, context, host_name, instance_info):"},{"line_number":111,"context_line":"        \"\"\"Receives information about changes to a host\u0027s instances, and"},{"line_number":112,"context_line":"        updates the driver\u0027s HostManager with that information."},{"line_number":113,"context_line":"        \"\"\""},{"line_number":114,"context_line":"        self.driver.host_manager.update_instance_info(context, host_name,"},{"line_number":115,"context_line":"                                                      instance_info)"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"    def delete_instance_info(self, context, host_name, instance_uuid):"},{"line_number":118,"context_line":"        \"\"\"Receives information about the deletion of one of a host\u0027s"},{"line_number":119,"context_line":"        instances, and updates the driver\u0027s HostManager with that information."},{"line_number":120,"context_line":"        \"\"\""},{"line_number":121,"context_line":"        self.driver.host_manager.delete_instance_info(context, host_name,"},{"line_number":122,"context_line":"                                                      instance_uuid)"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def sync_instance_info(self, context, host_name, instance_uuids):"},{"line_number":125,"context_line":"        \"\"\"Receives a sync request from a host, and passes it on to the"},{"line_number":126,"context_line":"        driver\u0027s HostManager."},{"line_number":127,"context_line":"        \"\"\""},{"line_number":128,"context_line":"        self.driver.host_manager.sync_instance_info(context, host_name,"},{"line_number":129,"context_line":"                                                    instance_uuids)"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"class _SchedulerManagerV5Proxy(object):"}],"source_content_type":"text/x-python","patch_set":37,"id":"7a740942_06449518","line":129,"range":{"start_line":91,"start_character":4,"end_line":129,"end_character":67},"updated":"2015-12-08 10:43:20.000000000","message":"We didn\u0027t need to copy any of these we could just extend the class really.","commit_id":"bc46404c69186a675235c5cff5ca531ac49fc7fe"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"5408fb6e9264019ab5f4bd7503c7beaa1c11d4cf","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        self.driver.run_periodic_tasks(context)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    @messaging.expected_exceptions(exception.NoValidHost)"},{"line_number":63,"context_line":"    def select_destinations(self, ctxt, spec_obj):"},{"line_number":64,"context_line":"        \"\"\"Returns destinations(s) best suited for this RequestSpec."},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        The result should be a list of dicts with \u0027host\u0027, \u0027nodename\u0027 and"}],"source_content_type":"text/x-python","patch_set":38,"id":"7a740942_55a0e541","line":63,"updated":"2015-12-14 13:58:19.000000000","message":"This seems broken to me.\n\nOld requests will fail. We should make the old two args optional, and the spec_obj added and optional, I think.","commit_id":"64b487b151f7dc7fb2c3b6eac3678531b98a6806"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"88678f9225809f69fd6cff8705cb42f31c58c78d","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        self.driver.run_periodic_tasks(context)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    @messaging.expected_exceptions(exception.NoValidHost)"},{"line_number":63,"context_line":"    def select_destinations(self, ctxt, spec_obj):"},{"line_number":64,"context_line":"        \"\"\"Returns destinations(s) best suited for this RequestSpec."},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        The result should be a list of dicts with \u0027host\u0027, \u0027nodename\u0027 and"}],"source_content_type":"text/x-python","patch_set":38,"id":"5a710552_7f0c8d94","line":63,"in_reply_to":"7a740942_4e84cf5c","updated":"2015-12-15 12:21:03.000000000","message":"IMHO we should really squash this and previous changes together, and simply add the spec object as a kwarg with a note to redo it in V5\n\nI got scolded several times by dan smith for changing the client side in a separate commit. In this case it does not really matter since we just change the data format, but in case we add functionality, we expose it before honouring it (for people that deploy commit by commit).","commit_id":"64b487b151f7dc7fb2c3b6eac3678531b98a6806"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4183036825cbbd984a37d1625212c4fb80446927","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        self.driver.run_periodic_tasks(context)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    @messaging.expected_exceptions(exception.NoValidHost)"},{"line_number":63,"context_line":"    def select_destinations(self, ctxt, spec_obj):"},{"line_number":64,"context_line":"        \"\"\"Returns destinations(s) best suited for this RequestSpec."},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        The result should be a list of dicts with \u0027host\u0027, \u0027nodename\u0027 and"}],"source_content_type":"text/x-python","patch_set":38,"id":"7a740942_4e84cf5c","line":63,"in_reply_to":"7a740942_55a0e541","updated":"2015-12-14 15:16:48.000000000","message":"Right, I made a mistake because I was lazy when reworking on the previous V5 proxy. Will respin","commit_id":"64b487b151f7dc7fb2c3b6eac3678531b98a6806"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"1dd3c62b1bd2aa8574e0bc24ac6b33cf2f5a4a80","unresolved":false,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    @messaging.expected_exceptions(exception.NoValidHost)"},{"line_number":64,"context_line":"    def select_destinations(self, ctxt,"},{"line_number":65,"context_line":"                            request_spec\u003dFalse, filter_properties\u003dFalse,"},{"line_number":66,"context_line":"                            spec_obj\u003dFalse):"},{"line_number":67,"context_line":"        \"\"\"Returns destinations(s) best suited for this RequestSpec."},{"line_number":68,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"5a710552_d7f3eb9c","line":65,"range":{"start_line":65,"start_character":28,"end_line":65,"end_character":46},"updated":"2015-12-16 11:59:13.000000000","message":"It\u0027s quite unusual to have a default argument that is not expected to be a Boolean type defaulted to False. IMHO we should really just be adding spec_obj as a kw argument, and checking if it\u0027s None to choose what to do.","commit_id":"74a3ebd90053a52ed6fb248a9d148d968d4c13f2"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"1bb2419679be6b97182080f66d2d50df6d373dcf","unresolved":false,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    @messaging.expected_exceptions(exception.NoValidHost)"},{"line_number":64,"context_line":"    def select_destinations(self, ctxt,"},{"line_number":65,"context_line":"                            request_spec\u003dFalse, filter_properties\u003dFalse,"},{"line_number":66,"context_line":"                            spec_obj\u003dFalse):"},{"line_number":67,"context_line":"        \"\"\"Returns destinations(s) best suited for this RequestSpec."},{"line_number":68,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"fa69d971_94ced0b2","line":65,"range":{"start_line":65,"start_character":28,"end_line":65,"end_character":46},"in_reply_to":"1a7b0d38_8409ca2e","updated":"2016-01-04 10:37:40.000000000","message":"I prefer this pattern, where you have some sentinel:\nhttps://github.com/openstack/nova/blob/1635086529c43edcb69966c48c41b5fca2d1b26c/nova/tests/unit/api/openstack/compute/test_networks.py#L121","commit_id":"74a3ebd90053a52ed6fb248a9d148d968d4c13f2"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"69592712010f7b91284e8c4e755e82a5e09ab82a","unresolved":false,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    @messaging.expected_exceptions(exception.NoValidHost)"},{"line_number":64,"context_line":"    def select_destinations(self, ctxt,"},{"line_number":65,"context_line":"                            request_spec\u003dFalse, filter_properties\u003dFalse,"},{"line_number":66,"context_line":"                            spec_obj\u003dFalse):"},{"line_number":67,"context_line":"        \"\"\"Returns destinations(s) best suited for this RequestSpec."},{"line_number":68,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"1a7b0d38_8409ca2e","line":65,"range":{"start_line":65,"start_character":28,"end_line":65,"end_character":46},"in_reply_to":"5a710552_d7f3eb9c","updated":"2015-12-24 13:14:08.000000000","message":"I don\u0027t want to have the RPCAPI providing a call like \n\n  select_dests(ctx, False, False, spec_obj\u003dMyObj)\n\nfor something new so I will leave None instead of False and use the kwargs.","commit_id":"74a3ebd90053a52ed6fb248a9d148d968d4c13f2"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"38975185e3791ad190907221c63a4bad4e701778","unresolved":false,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    @messaging.expected_exceptions(exception.NoValidHost)"},{"line_number":64,"context_line":"    def select_destinations(self, ctxt,"},{"line_number":65,"context_line":"                            request_spec\u003dNone, filter_properties\u003dNone, **kw):"},{"line_number":66,"context_line":"        \"\"\"Returns destinations(s) best suited for this RequestSpec."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        The result should be a list of dicts with \u0027host\u0027, \u0027nodename\u0027 and"}],"source_content_type":"text/x-python","patch_set":41,"id":"fa69d971_d17d3ae8","line":65,"updated":"2016-01-04 10:42:33.000000000","message":"I slightly prefer the previous patch over this one, although I don\u0027t really like either approach enough to +2 them.\n\nCould we not try a mix of the two, so we keep the semantic meaning of the default, while still keeping the list of possible arguments. Something like this:\nhttps://github.com/openstack/nova/blob/1635086529c43edcb69966c48c41b5fca2d1b26c/nova/tests/unit/api/openstack/compute/test_networks.py#L121","commit_id":"8226a3ba5971adff597382e65c2a44c118fddf50"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"10351fcca6b6fad057365283107f28b648ce822c","unresolved":false,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    @messaging.expected_exceptions(exception.NoValidHost)"},{"line_number":64,"context_line":"    def select_destinations(self, ctxt,"},{"line_number":65,"context_line":"                            request_spec\u003dNone, filter_properties\u003dNone, **kw):"},{"line_number":66,"context_line":"        \"\"\"Returns destinations(s) best suited for this RequestSpec."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        The result should be a list of dicts with \u0027host\u0027, \u0027nodename\u0027 and"}],"source_content_type":"text/x-python","patch_set":41,"id":"fa69d971_c3a248ae","line":65,"in_reply_to":"fa69d971_a30dd438","updated":"2016-01-04 12:38:33.000000000","message":"I was really just thinking a sentinal for the spec_obj, so we can tell if it has been passed. Maybe we never pass a None spec object, so None is just fine?\n\nI am just not keen on the **kw, when we don\u0027t need it. But its a nit pick really.","commit_id":"8226a3ba5971adff597382e65c2a44c118fddf50"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"c27b0942aecdc5e3b44556ea4e98042a2b5a55c0","unresolved":false,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    @messaging.expected_exceptions(exception.NoValidHost)"},{"line_number":64,"context_line":"    def select_destinations(self, ctxt,"},{"line_number":65,"context_line":"                            request_spec\u003dNone, filter_properties\u003dNone, **kw):"},{"line_number":66,"context_line":"        \"\"\"Returns destinations(s) best suited for this RequestSpec."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        The result should be a list of dicts with \u0027host\u0027, \u0027nodename\u0027 and"}],"source_content_type":"text/x-python","patch_set":41,"id":"fa69d971_cf01dc06","line":65,"in_reply_to":"fa69d971_c3a248ae","updated":"2016-01-05 11:09:46.000000000","message":"I tend to agree with John and provide explicitely the spec_obj arg with defaulting it to a sentinel value.\n\nWill respin it accordingly.","commit_id":"8226a3ba5971adff597382e65c2a44c118fddf50"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"e63768dd1805a62e42ffc31edbd2f425a2df189b","unresolved":false,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    @messaging.expected_exceptions(exception.NoValidHost)"},{"line_number":64,"context_line":"    def select_destinations(self, ctxt,"},{"line_number":65,"context_line":"                            request_spec\u003dNone, filter_properties\u003dNone, **kw):"},{"line_number":66,"context_line":"        \"\"\"Returns destinations(s) best suited for this RequestSpec."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        The result should be a list of dicts with \u0027host\u0027, \u0027nodename\u0027 and"}],"source_content_type":"text/x-python","patch_set":41,"id":"fa69d971_a30dd438","line":65,"in_reply_to":"fa69d971_d17d3ae8","updated":"2016-01-04 12:29:48.000000000","message":"It makes sense to have a sentinel only when an argument can have a meaningful None value. That is currently not the case so from that POV this is fine.\n\nActually it makes no sense for request_spec to ever be None unless spec_obj is not sent, so we should probably guard against that (previously the dispatching of the message would fail I think). This is fine as well as not having a request spec will never happen.","commit_id":"8226a3ba5971adff597382e65c2a44c118fddf50"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"452bcf19693d89375d8008b19783d90dd4ddbc39","unresolved":false,"context_lines":[{"line_number":62,"context_line":"    @messaging.expected_exceptions(exception.NoValidHost)"},{"line_number":63,"context_line":"    def select_destinations(self, ctxt,"},{"line_number":64,"context_line":"                            request_spec\u003dNone, filter_properties\u003dNone,"},{"line_number":65,"context_line":"                            spec_obj\u003d_sentinel):"},{"line_number":66,"context_line":"        \"\"\"Returns destinations(s) best suited for this RequestSpec."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        The result should be a list of dicts with \u0027host\u0027, \u0027nodename\u0027 and"}],"source_content_type":"text/x-python","patch_set":42,"id":"fa69d971_ff578023","line":65,"range":{"start_line":65,"start_character":28,"end_line":65,"end_character":46},"updated":"2016-01-06 10:24:06.000000000","message":"I think this is actually worse since it hints to the reader that None is somehow a valid value of spec_obj and it really isn\u0027t it can never be None unless it\u0027s from an old node.","commit_id":"40781fbae56c895aa7ff7a90eb527f51f3e19e1d"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"03ac179913409e5bf459440f36bfdfea663df69d","unresolved":false,"context_lines":[{"line_number":62,"context_line":"    @messaging.expected_exceptions(exception.NoValidHost)"},{"line_number":63,"context_line":"    def select_destinations(self, ctxt,"},{"line_number":64,"context_line":"                            request_spec\u003dNone, filter_properties\u003dNone,"},{"line_number":65,"context_line":"                            spec_obj\u003d_sentinel):"},{"line_number":66,"context_line":"        \"\"\"Returns destinations(s) best suited for this RequestSpec."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        The result should be a list of dicts with \u0027host\u0027, \u0027nodename\u0027 and"}],"source_content_type":"text/x-python","patch_set":42,"id":"fa69d971_1d1a9c17","line":65,"range":{"start_line":65,"start_character":28,"end_line":65,"end_character":46},"in_reply_to":"fa69d971_ff578023","updated":"2016-01-06 10:51:49.000000000","message":"Well, I would read the contrary, where the default behaviour would be to expect something for spec_obj (and not really caring of request_spec and filter_props) but we\u0027re probably nitpicking, are we ? :-)","commit_id":"40781fbae56c895aa7ff7a90eb527f51f3e19e1d"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"7168c63f3f1287351f9b34f972315233cc8118ff","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        \u0027limits\u0027 as keys."},{"line_number":70,"context_line":"        \"\"\""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        # TODO(sbauza): Change the method signature to only accept a spec_obj"},{"line_number":73,"context_line":"        # argument once API v5 is provided."},{"line_number":74,"context_line":"        if spec_obj is self._sentinel:"},{"line_number":75,"context_line":"            spec_obj \u003d objects.RequestSpec.from_primitives(ctxt,"}],"source_content_type":"text/x-python","patch_set":42,"id":"fa69d971_2440a006","line":72,"updated":"2016-01-06 13:58:26.000000000","message":"nit: require a spec_obj","commit_id":"40781fbae56c895aa7ff7a90eb527f51f3e19e1d"}],"nova/scheduler/rpcapi.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"4c4b3cebc826089e267319fcd18ed12518630481","unresolved":false,"context_lines":[{"line_number":126,"context_line":"        else:"},{"line_number":127,"context_line":"            return legacy"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    def select_destinations(self, ctxt, request_spec, filter_properties):"},{"line_number":130,"context_line":"        if not self.client.can_send_version(\u00275.0\u0027):"},{"line_number":131,"context_line":"            cctxt \u003d self.client.prepare(version\u003d\u00274.0\u0027)"},{"line_number":132,"context_line":"            return cctxt.call(ctxt, \u0027select_destinations\u0027,"}],"source_content_type":"text/x-python","patch_set":36,"id":"9a8ffd7b_6be761ac","line":129,"updated":"2015-11-27 14:38:59.000000000","message":"This should be the new interface here, and take objects, I think.\n\nThis method is the reverse of what we normally do for backwards compat here.\n\nYou can actually do that change before changing the wire protocol, if you like, actually I think its better that way around.","commit_id":"20daa897c5b0947d4c28cc6651d23dbde188752a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"485fee71fb2b519bbc32656ce2d83e0440ae29bf","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        return cctxt.call(ctxt, \u0027select_destinations\u0027, spec_obj\u003dspec_obj)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def update_aggregates(self, ctxt, aggregates):"},{"line_number":144,"context_line":"        version \u003d self._compat_ver(\u00275.0\u0027, \u00274.1\u0027)"},{"line_number":145,"context_line":"        # NOTE(sbauza): Yes, it\u0027s a fanout, we need to update all schedulers"},{"line_number":146,"context_line":"        cctxt \u003d self.client.prepare(fanout\u003dTrue, version\u003dversion)"},{"line_number":147,"context_line":"        cctxt.cast(ctxt, \u0027update_aggregates\u0027, aggregates\u003daggregates)"}],"source_content_type":"text/x-python","patch_set":36,"id":"9a8ffd7b_0b5015d3","line":144,"updated":"2015-11-27 14:34:36.000000000","message":"I think we can avoid all this till the very end of the release.","commit_id":"20daa897c5b0947d4c28cc6651d23dbde188752a"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"c19f60536afa5943f6e3c86ea6ed8b99cfa97cd7","unresolved":false,"context_lines":[{"line_number":97,"context_line":"        done such that they can handle the version_cap being set to"},{"line_number":98,"context_line":"        4.2."},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"        * 5.0 - Removed backwards compat for Juno and modify"},{"line_number":101,"context_line":"                select_destinations() signature by providing a RequestSpec obj"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"    \u0027\u0027\u0027"}],"source_content_type":"text/x-python","patch_set":37,"id":"7a740942_26019105","line":100,"range":{"start_line":100,"start_character":16,"end_line":100,"end_character":49},"updated":"2015-12-08 10:43:20.000000000","message":"I don\u0027t see any backwards compat being removed here??","commit_id":"bc46404c69186a675235c5cff5ca531ac49fc7fe"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"c19f60536afa5943f6e3c86ea6ed8b99cfa97cd7","unresolved":false,"context_lines":[{"line_number":103,"context_line":"    \u0027\u0027\u0027"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    VERSION_ALIASES \u003d {"},{"line_number":106,"context_line":"        \u0027grizzly\u0027: \u00272.6\u0027,"},{"line_number":107,"context_line":"        \u0027havana\u0027: \u00272.9\u0027,"},{"line_number":108,"context_line":"        \u0027icehouse\u0027: \u00273.0\u0027,"},{"line_number":109,"context_line":"        \u0027juno\u0027: \u00273.0\u0027,"},{"line_number":110,"context_line":"        \u0027kilo\u0027: \u00274.2\u0027,"},{"line_number":111,"context_line":"        \u0027liberty\u0027: \u00274.2\u0027,"},{"line_number":112,"context_line":"    }"}],"source_content_type":"text/x-python","patch_set":37,"id":"7a740942_86ede5b8","line":109,"range":{"start_line":106,"start_character":8,"end_line":109,"end_character":22},"updated":"2015-12-08 10:43:20.000000000","message":"Not realted but we should probably remove these as we can\u0027t send any of them anymore","commit_id":"bc46404c69186a675235c5cff5ca531ac49fc7fe"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"c19f60536afa5943f6e3c86ea6ed8b99cfa97cd7","unresolved":false,"context_lines":[{"line_number":126,"context_line":"        else:"},{"line_number":127,"context_line":"            return legacy"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    def select_destinations(self, ctxt, request_spec, filter_properties):"},{"line_number":130,"context_line":"        if not self.client.can_send_version(\u00275.0\u0027):"},{"line_number":131,"context_line":"            cctxt \u003d self.client.prepare(version\u003d\u00274.0\u0027)"},{"line_number":132,"context_line":"            return cctxt.call(ctxt, \u0027select_destinations\u0027,"}],"source_content_type":"text/x-python","patch_set":37,"id":"7a740942_26a3d1b3","line":129,"range":{"start_line":129,"start_character":8,"end_line":129,"end_character":73},"updated":"2015-12-08 10:43:20.000000000","message":"So I am not a big fan of this. If we are bumping a major version of the RPC api, then let\u0027s make sure this is reflected in the client side of the API as well (this class is the contract basically).\n\nSo this signature should accept the request object and \"primitivize\" it only if needed... which brings me to my next point.","commit_id":"bc46404c69186a675235c5cff5ca531ac49fc7fe"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"c19f60536afa5943f6e3c86ea6ed8b99cfa97cd7","unresolved":false,"context_lines":[{"line_number":127,"context_line":"            return legacy"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    def select_destinations(self, ctxt, request_spec, filter_properties):"},{"line_number":130,"context_line":"        if not self.client.can_send_version(\u00275.0\u0027):"},{"line_number":131,"context_line":"            cctxt \u003d self.client.prepare(version\u003d\u00274.0\u0027)"},{"line_number":132,"context_line":"            return cctxt.call(ctxt, \u0027select_destinations\u0027,"},{"line_number":133,"context_line":"                              request_spec\u003drequest_spec,"},{"line_number":134,"context_line":"                              filter_properties\u003dfilter_properties)"},{"line_number":135,"context_line":"        # TODO(sbauza): For the moment, hydrate here the RequestSpec until we"},{"line_number":136,"context_line":"        # provide directly the object from the conductor"},{"line_number":137,"context_line":"        spec_obj \u003d objects.RequestSpec.from_primitives(ctxt,"}],"source_content_type":"text/x-python","patch_set":37,"id":"7a740942_26b8b1c7","line":134,"range":{"start_line":130,"start_character":8,"end_line":134,"end_character":66},"updated":"2015-12-08 10:43:20.000000000","message":"When do we drop this? I was expecting to see this in the commit message, but it does not even mention that we are introducing a new major version of the API.\n\nWe should really have a comment here about dropping it.","commit_id":"bc46404c69186a675235c5cff5ca531ac49fc7fe"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"c19f60536afa5943f6e3c86ea6ed8b99cfa97cd7","unresolved":false,"context_lines":[{"line_number":132,"context_line":"            return cctxt.call(ctxt, \u0027select_destinations\u0027,"},{"line_number":133,"context_line":"                              request_spec\u003drequest_spec,"},{"line_number":134,"context_line":"                              filter_properties\u003dfilter_properties)"},{"line_number":135,"context_line":"        # TODO(sbauza): For the moment, hydrate here the RequestSpec until we"},{"line_number":136,"context_line":"        # provide directly the object from the conductor"},{"line_number":137,"context_line":"        spec_obj \u003d objects.RequestSpec.from_primitives(ctxt,"},{"line_number":138,"context_line":"                                                       request_spec,"},{"line_number":139,"context_line":"                                                       filter_properties)"},{"line_number":140,"context_line":"        cctxt \u003d self.client.prepare(version\u003d\u00275.0\u0027)"},{"line_number":141,"context_line":"        return cctxt.call(ctxt, \u0027select_destinations\u0027, spec_obj\u003dspec_obj)"},{"line_number":142,"context_line":""}],"source_content_type":"text/x-python","patch_set":37,"id":"7a740942_66c23936","line":139,"range":{"start_line":135,"start_character":7,"end_line":139,"end_character":73},"updated":"2015-12-08 10:43:20.000000000","message":"As discussed above - this should not be here.","commit_id":"bc46404c69186a675235c5cff5ca531ac49fc7fe"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"5408fb6e9264019ab5f4bd7503c7beaa1c11d4cf","unresolved":false,"context_lines":[{"line_number":115,"context_line":"            del kw[\u0027spec_obj\u0027]"},{"line_number":116,"context_line":"            kw[\u0027request_spec\u0027] \u003d spec_obj.to_legacy_request_spec_dict()"},{"line_number":117,"context_line":"            kw[\u0027filter_properties\u0027"},{"line_number":118,"context_line":"               ] \u003d spec_obj.to_legacy_filter_properties_dict()"},{"line_number":119,"context_line":"            version \u003d \u00274.0\u0027"},{"line_number":120,"context_line":"        cctxt \u003d self.client.prepare(version\u003dversion)"},{"line_number":121,"context_line":"        return cctxt.call(ctxt, \u0027select_destinations\u0027, **kw)"}],"source_content_type":"text/x-python","patch_set":38,"id":"7a740942_500a3321","line":118,"updated":"2015-12-14 13:58:19.000000000","message":"Note, if you PIN at version 4.0 our 4.3 code needs to support both of these things.","commit_id":"64b487b151f7dc7fb2c3b6eac3678531b98a6806"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"1dd3c62b1bd2aa8574e0bc24ac6b33cf2f5a4a80","unresolved":false,"context_lines":[{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def __init__(self):"},{"line_number":103,"context_line":"        super(SchedulerAPI, self).__init__()"},{"line_number":104,"context_line":"        target \u003d messaging.Target(topic\u003dCONF.scheduler_topic, version\u003d\u00275.0\u0027)"},{"line_number":105,"context_line":"        version_cap \u003d self.VERSION_ALIASES.get(CONF.upgrade_levels.scheduler,"},{"line_number":106,"context_line":"                                               CONF.upgrade_levels.scheduler)"},{"line_number":107,"context_line":"        serializer \u003d objects_base.NovaObjectSerializer()"}],"source_content_type":"text/x-python","patch_set":39,"id":"5a710552_37cb4ea7","line":104,"range":{"start_line":104,"start_character":71,"end_line":104,"end_character":74},"updated":"2015-12-16 11:59:13.000000000","message":"This is obviously wrong","commit_id":"74a3ebd90053a52ed6fb248a9d148d968d4c13f2"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"1dd3c62b1bd2aa8574e0bc24ac6b33cf2f5a4a80","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        self.client \u003d rpc.get_client(target, version_cap\u003dversion_cap,"},{"line_number":109,"context_line":"                                     serializer\u003dserializer)"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def select_destinations(self, ctxt, spec_obj):"},{"line_number":112,"context_line":"        version \u003d \u00274.3\u0027"},{"line_number":113,"context_line":"        kw \u003d {\u0027spec_obj\u0027: spec_obj}"},{"line_number":114,"context_line":"        if not self.client.can_send_version(version):"}],"source_content_type":"text/x-python","patch_set":39,"id":"5a710552_97d7a222","line":111,"range":{"start_line":111,"start_character":29,"end_line":111,"end_character":48},"updated":"2015-12-16 11:59:13.000000000","message":"We are not bumping the major version so we really should not be changing the signature in this way IMHO, but that\u0027s just my preference.","commit_id":"74a3ebd90053a52ed6fb248a9d148d968d4c13f2"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"38975185e3791ad190907221c63a4bad4e701778","unresolved":false,"context_lines":[{"line_number":118,"context_line":"               ] \u003d spec_obj.to_legacy_filter_properties_dict()"},{"line_number":119,"context_line":"            version \u003d \u00274.0\u0027"},{"line_number":120,"context_line":"        cctxt \u003d self.client.prepare(version\u003dversion)"},{"line_number":121,"context_line":"        return cctxt.call(ctxt, \u0027select_destinations\u0027, **kw)"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"    def update_aggregates(self, ctxt, aggregates):"},{"line_number":124,"context_line":"        # NOTE(sbauza): Yes, it\u0027s a fanout, we need to update all schedulers"}],"source_content_type":"text/x-python","patch_set":41,"id":"fa69d971_543e18b9","line":121,"updated":"2016-01-04 10:42:33.000000000","message":"Tiny, tiny nit, I wonder about msg_args or kwargs, rather than kw, for consistency:\nhttps://github.com/openstack/nova/blob/master/nova/compute/rpcapi.py#L632","commit_id":"8226a3ba5971adff597382e65c2a44c118fddf50"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"3113104e3dbdcfb113791e23e175a00c3b8c8fe0","unresolved":false,"context_lines":[{"line_number":115,"context_line":"            del msg_args[\u0027spec_obj\u0027]"},{"line_number":116,"context_line":"            msg_args[\u0027request_spec\u0027] \u003d spec_obj.to_legacy_request_spec_dict()"},{"line_number":117,"context_line":"            msg_args[\u0027filter_properties\u0027"},{"line_number":118,"context_line":"                     ] \u003d spec_obj.to_legacy_filter_properties_dict()"},{"line_number":119,"context_line":"            version \u003d \u00274.0\u0027"},{"line_number":120,"context_line":"        cctxt \u003d self.client.prepare(version\u003dversion)"},{"line_number":121,"context_line":"        return cctxt.call(ctxt, \u0027select_destinations\u0027, **msg_args)"}],"source_content_type":"text/x-python","patch_set":42,"id":"fa69d971_f97a35bc","line":118,"updated":"2016-01-06 13:54:29.000000000","message":"nit: I get that this is valid python, but this is one of those cases where a \\ would be a lot easier to mentally parse. Or use a temp var.","commit_id":"40781fbae56c895aa7ff7a90eb527f51f3e19e1d"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a3e4483541f554571e944fc432043997767fa24f","unresolved":false,"context_lines":[{"line_number":115,"context_line":"            del msg_args[\u0027spec_obj\u0027]"},{"line_number":116,"context_line":"            msg_args[\u0027request_spec\u0027] \u003d spec_obj.to_legacy_request_spec_dict()"},{"line_number":117,"context_line":"            msg_args[\u0027filter_properties\u0027"},{"line_number":118,"context_line":"                     ] \u003d spec_obj.to_legacy_filter_properties_dict()"},{"line_number":119,"context_line":"            version \u003d \u00274.0\u0027"},{"line_number":120,"context_line":"        cctxt \u003d self.client.prepare(version\u003dversion)"},{"line_number":121,"context_line":"        return cctxt.call(ctxt, \u0027select_destinations\u0027, **msg_args)"}],"source_content_type":"text/x-python","patch_set":42,"id":"fa69d971_84886c72","line":118,"in_reply_to":"fa69d971_f97a35bc","updated":"2016-01-06 13:55:26.000000000","message":"Agreed, I\u0027m probably yakshaving here for the beauty of Python","commit_id":"40781fbae56c895aa7ff7a90eb527f51f3e19e1d"}],"nova/tests/unit/scheduler/test_rpcapi.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"38975185e3791ad190907221c63a4bad4e701778","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        rpcapi \u003d scheduler_rpcapi.SchedulerAPI()"},{"line_number":79,"context_line":"        rpcapi.prepare \u003d mock.Mock()"},{"line_number":80,"context_line":"        with test.nested("},{"line_number":81,"context_line":"                mock.patch.object(rpcapi.client, \u0027can_send_version\u0027),"},{"line_number":82,"context_line":"                mock.patch.object(rpcapi.client, \u0027prepare\u0027),"},{"line_number":83,"context_line":"                mock.patch.object(rpcapi.client, \u0027call\u0027)) as (csv, prep, call):"},{"line_number":84,"context_line":"            csv.return_value \u003d _can_send_version"}],"source_content_type":"text/x-python","patch_set":41,"id":"fa69d971_b14be686","line":81,"updated":"2016-01-04 10:42:33.000000000","message":"Can\u0027t we just test this by just setting the flag, like this:\nhttps://github.com/openstack/nova/blob/master/nova/tests/unit/compute/test_rpcapi.py#L323\n\nI think thats easier to maintain, than this method.\n\nI would not normally worry, but this is the kind of pattern folks might try and copy into other tests, and we could get ourselves into a big mess.","commit_id":"8226a3ba5971adff597382e65c2a44c118fddf50"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"c27b0942aecdc5e3b44556ea4e98042a2b5a55c0","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        rpcapi \u003d scheduler_rpcapi.SchedulerAPI()"},{"line_number":79,"context_line":"        rpcapi.prepare \u003d mock.Mock()"},{"line_number":80,"context_line":"        with test.nested("},{"line_number":81,"context_line":"                mock.patch.object(rpcapi.client, \u0027can_send_version\u0027),"},{"line_number":82,"context_line":"                mock.patch.object(rpcapi.client, \u0027prepare\u0027),"},{"line_number":83,"context_line":"                mock.patch.object(rpcapi.client, \u0027call\u0027)) as (csv, prep, call):"},{"line_number":84,"context_line":"            csv.return_value \u003d _can_send_version"}],"source_content_type":"text/x-python","patch_set":41,"id":"fa69d971_dd9199f2","line":81,"in_reply_to":"fa69d971_b14be686","updated":"2016-01-05 11:09:46.000000000","message":"That sounds a wise advice, will reconsider my change","commit_id":"8226a3ba5971adff597382e65c2a44c118fddf50"}]}