)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8c7c5f6f3bc5e04968820e594a757398c2e3d28c","unresolved":false,"context_lines":[{"line_number":13,"context_line":"by the resource class name."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"TODO:"},{"line_number":16,"context_line":" - Tests and tests and tests."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: I94e800dabd5cc995728898dd6d8f6d42ba645312"},{"line_number":19,"context_line":"Partially-Implements: blueprint resource-providers-get-by-request"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"ba5da102_e80daae8","line":16,"updated":"2016-11-03 23:12:37.000000000","message":"Oh snap, not really true, I\u0027m done with tests.","commit_id":"fd7c5232e2159331b945797c8243484a4a3a3156"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"7074d39102921fd9be83df02d589a32441218e8d","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Sylvain Bauza \u003csbauza@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-11-07 12:55:52 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Provide an object facade for a subset of RPs"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Given the scheduler wants to know which RPs can support a set of different"},{"line_number":10,"context_line":"requests, each one having a resource class with an amount, we need some"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"9a629dbe_71154fe3","line":7,"updated":"2016-11-10 13:36:47.000000000","message":"Extend get_all_by_filters to support resources\n\nor something like that?","commit_id":"40a612b952e520c7d80b00485213669068e4b331"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"4123c016a098d34bca1537d0f19d38765a08ef5d","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     EdLeafe \u003ced@leafe.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-11-15 21:47:38 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Provide an object facade for a subset of RPs"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Given the scheduler wants to know which RPs can support a set of different"},{"line_number":10,"context_line":"requests, each one having a resource class with an amount, we need some"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"7a77a97e_f3b9b9cc","line":7,"updated":"2016-11-16 11:20:05.000000000","message":"I feel like this is a bad summary. It\u0027s just a method doing some stuff. facade is misleading.","commit_id":"4b70f6af11e663e99ca5e61c86db0109f2022fa1"}],"nova/db/sqlalchemy/resource_class_cache.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fdff6477b485c9eb1ba1a6b6413de4bfefd9918e","unresolved":false,"context_lines":[{"line_number":46,"context_line":"    res \u003d ctx.session.query(models.ResourceClass.id,"},{"line_number":47,"context_line":"                            models.ResourceClass.name).all()"},{"line_number":48,"context_line":"    cache.id_cache \u003d {r[1]: r[0] for r in res}"},{"line_number":49,"context_line":"    cache.str_cache \u003d {r[0]: r[1] for r in res}"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"class ResourceClassCache(object):"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba5da102_a80732c8","line":49,"updated":"2016-11-03 23:12:11.000000000","message":"This can be punted in case we merge soon https://review.openstack.org/#/c/393378/","commit_id":"fd7c5232e2159331b945797c8243484a4a3a3156"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e6c9b144d9fd6cb26e9118d6982fc80686e833dd","unresolved":false,"context_lines":[{"line_number":44,"context_line":"    :param cache: ResourceClassCache object to refresh."},{"line_number":45,"context_line":"    \"\"\""},{"line_number":46,"context_line":"    res \u003d ctx.session.query(models.ResourceClass.id,"},{"line_number":47,"context_line":"                            models.ResourceClass.name).all()"},{"line_number":48,"context_line":"    cache.id_cache \u003d {r[1]: r[0] for r in res}"},{"line_number":49,"context_line":"    cache.str_cache \u003d {r[0]: r[1] for r in res}"},{"line_number":50,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"ba5da102_88fd0eb3","line":47,"updated":"2016-11-03 23:14:41.000000000","message":"like I said in PS4, we can punt that when merging https://review.openstack.org/#/c/393378/","commit_id":"f3b2afd569bb507642e773eaf4155d241c5d3943"}],"nova/objects/resource_provider.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"c20e6aad6dfd1665261b861993d04382bbff9167","unresolved":false,"context_lines":[{"line_number":536,"context_line":"        if acceptable_rps:"},{"line_number":537,"context_line":"            query.where(models.ResourceProvider.uuid.in_("},{"line_number":538,"context_line":"                [rp[\u0027uuid\u0027] for rp in acceptable_rps]"},{"line_number":539,"context_line":"            ))"},{"line_number":540,"context_line":"        return query.all()"},{"line_number":541,"context_line":""},{"line_number":542,"context_line":"    @base.remotable_classmethod"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_39ae5d81","line":539,"updated":"2016-11-02 14:25:29.000000000","message":"The value of the acceptable_rps in the `in_` could be huge. See my comment on patchset 2 for a possible alternative that (if it is workable) might be a bit more readable and less complex.","commit_id":"b44bd6f85ec3b6f2628e5087ef01c8834c78f1bd"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"fb127584fc7b07a47888e4a760e6cd9663f0c66d","unresolved":false,"context_lines":[{"line_number":449,"context_line":""},{"line_number":450,"context_line":"    @staticmethod"},{"line_number":451,"context_line":"    @db_api.api_context_manager.reader"},{"line_number":452,"context_line":"    def _get_all_by_filters_from_db(context, filters):"},{"line_number":453,"context_line":"        \"\"\"Returns a list of `ResourceProvider` objects that could accept the"},{"line_number":454,"context_line":"        list of amounts per resource class provided as future allocations."},{"line_number":455,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7a77a97e_3068d8ba","line":452,"updated":"2016-11-14 20:57:58.000000000","message":"Instead of changing the existing get_all_by_filters() method, how about we simply have a new get_all_by_scheduler_request() that is specific to the call that will be made from the scheduler to match resource providers that have appropriate inventory capacity for a given set of resources (and eventually traits).","commit_id":"40a612b952e520c7d80b00485213669068e4b331"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"d3a931d2bd73564cbc73cdc6eb2b40dea1509886","unresolved":false,"context_lines":[{"line_number":449,"context_line":""},{"line_number":450,"context_line":"    @staticmethod"},{"line_number":451,"context_line":"    @db_api.api_context_manager.reader"},{"line_number":452,"context_line":"    def _get_all_by_filters_from_db(context, filters):"},{"line_number":453,"context_line":"        \"\"\"Returns a list of `ResourceProvider` objects that could accept the"},{"line_number":454,"context_line":"        list of amounts per resource class provided as future allocations."},{"line_number":455,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7a77a97e_eb3af1e6","line":452,"in_reply_to":"7a77a97e_3068d8ba","updated":"2016-11-14 21:44:00.000000000","message":"I think the desire for a get_all_by_filters was driven by the idea that the /resource_providers URL would accept a combination of resource and uuid and name parameters on a GET request and the user-agent could mix and match those filters as they saw fit and the the API would, as generally as possible, just handle it.\n\nI still believe that\u0027s the cleanest solution because it represents the semantics of the request (we want to filter by some stuff) in the most HTTP oriented way rather than being specific to the use cases that we are building.\n\nA \"filter by stuff\" has the best chance for longevity in the face of unplanned \"stuff\" and unplanned user-agents.","commit_id":"40a612b952e520c7d80b00485213669068e4b331"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"bbbf513300ecea19608ac01ef8825e76819a3c9c","unresolved":false,"context_lines":[{"line_number":449,"context_line":""},{"line_number":450,"context_line":"    @staticmethod"},{"line_number":451,"context_line":"    @db_api.api_context_manager.reader"},{"line_number":452,"context_line":"    def _get_all_by_filters_from_db(context, filters):"},{"line_number":453,"context_line":"        \"\"\"Returns a list of `ResourceProvider` objects that could accept the"},{"line_number":454,"context_line":"        list of amounts per resource class provided as future allocations."},{"line_number":455,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7a77a97e_dda5fcb5","line":452,"in_reply_to":"7a77a97e_eb3af1e6","updated":"2016-11-14 21:57:53.000000000","message":"OK, that\u0027s a good argument for this implementation then, sure.","commit_id":"40a612b952e520c7d80b00485213669068e4b331"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"7074d39102921fd9be83df02d589a32441218e8d","unresolved":false,"context_lines":[{"line_number":451,"context_line":"    @db_api.api_context_manager.reader"},{"line_number":452,"context_line":"    def _get_all_by_filters_from_db(context, filters):"},{"line_number":453,"context_line":"        \"\"\"Returns a list of `ResourceProvider` objects that could accept the"},{"line_number":454,"context_line":"        list of amounts per resource class provided as future allocations."},{"line_number":455,"context_line":""},{"line_number":456,"context_line":"        If no resource providers can\u0027t be found, the function will return an"},{"line_number":457,"context_line":"        empty list."}],"source_content_type":"text/x-python","patch_set":6,"id":"9a629dbe_b1bc47bc","line":454,"updated":"2016-11-10 13:36:47.000000000","message":"This description is too specific. There\u0027s the existing api of filtering by name and/or uuid.","commit_id":"40a612b952e520c7d80b00485213669068e4b331"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"fb127584fc7b07a47888e4a760e6cd9663f0c66d","unresolved":false,"context_lines":[{"line_number":453,"context_line":"        \"\"\"Returns a list of `ResourceProvider` objects that could accept the"},{"line_number":454,"context_line":"        list of amounts per resource class provided as future allocations."},{"line_number":455,"context_line":""},{"line_number":456,"context_line":"        If no resource providers can\u0027t be found, the function will return an"},{"line_number":457,"context_line":"        empty list."},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"        :param conn: SQLalchemy Connection object to use"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a77a97e_706250da","line":456,"updated":"2016-11-14 20:57:58.000000000","message":"s/can\u0027t be found/can be found/","commit_id":"40a612b952e520c7d80b00485213669068e4b331"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"7074d39102921fd9be83df02d589a32441218e8d","unresolved":false,"context_lines":[{"line_number":456,"context_line":"        If no resource providers can\u0027t be found, the function will return an"},{"line_number":457,"context_line":"        empty list."},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"        :param conn: SQLalchemy Connection object to use"},{"line_number":460,"context_line":"        :param filters: Dict of requested amounts keyed by resource classes,"},{"line_number":461,"context_line":"                        name and uuid being special cases."},{"line_number":462,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"9a629dbe_f1b23fe4","line":459,"updated":"2016-11-10 13:36:47.000000000","message":"you\u0027ve got conn here, and context above","commit_id":"40a612b952e520c7d80b00485213669068e4b331"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"bbbf513300ecea19608ac01ef8825e76819a3c9c","unresolved":false,"context_lines":[{"line_number":483,"context_line":"        #        total + reserved) * inv.allocation_ratio))"},{"line_number":484,"context_line":"        # GROUP BY rp.uuid"},{"line_number":485,"context_line":"        # HAVING"},{"line_number":486,"context_line":"        #  COUNT(DISTINCT(inv.resource_class_id)) \u003d\u003d len($RESOURCE_CLASSES)"},{"line_number":487,"context_line":"        #"},{"line_number":488,"context_line":"        # with a possible additional WHERE clause for the host and uuid"},{"line_number":489,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7a77a97e_7d76a83f","line":486,"updated":"2016-11-14 21:57:53.000000000","message":"You may want to move this particular SQL output comment to below line 513 so that it is contextually relevant to the part of the method that constructs the resources WHERE clauses...","commit_id":"40a612b952e520c7d80b00485213669068e4b331"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"7074d39102921fd9be83df02d589a32441218e8d","unresolved":false,"context_lines":[{"line_number":485,"context_line":"        # HAVING"},{"line_number":486,"context_line":"        #  COUNT(DISTINCT(inv.resource_class_id)) \u003d\u003d len($RESOURCE_CLASSES)"},{"line_number":487,"context_line":"        #"},{"line_number":488,"context_line":"        # with a possible additional WHERE clause for the host and uuid"},{"line_number":489,"context_line":""},{"line_number":490,"context_line":"        if not filters:"},{"line_number":491,"context_line":"            filters \u003d {}"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a629dbe_9105837b","line":488,"updated":"2016-11-10 13:36:47.000000000","message":"It\u0027s not a \u0027host\u0027, it\u0027s a \u0027name\u0027 because a resource provider isn\u0027t always going to be a host.","commit_id":"40a612b952e520c7d80b00485213669068e4b331"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"7074d39102921fd9be83df02d589a32441218e8d","unresolved":false,"context_lines":[{"line_number":490,"context_line":"        if not filters:"},{"line_number":491,"context_line":"            filters \u003d {}"},{"line_number":492,"context_line":"        else:"},{"line_number":493,"context_line":"            filters \u003d filters.copy()"},{"line_number":494,"context_line":"        name \u003d filters.pop(\u0027name\u0027, None)"},{"line_number":495,"context_line":"        uuid \u003d filters.pop(\u0027uuid\u0027, None)"},{"line_number":496,"context_line":"        can_host \u003d filters.pop(\u0027can_host\u0027, 0)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a629dbe_11ddf3df","line":493,"updated":"2016-11-10 13:36:47.000000000","message":"You might want to note why you are protecting the original filters?","commit_id":"40a612b952e520c7d80b00485213669068e4b331"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"fb127584fc7b07a47888e4a760e6cd9663f0c66d","unresolved":false,"context_lines":[{"line_number":490,"context_line":"        if not filters:"},{"line_number":491,"context_line":"            filters \u003d {}"},{"line_number":492,"context_line":"        else:"},{"line_number":493,"context_line":"            filters \u003d filters.copy()"},{"line_number":494,"context_line":"        name \u003d filters.pop(\u0027name\u0027, None)"},{"line_number":495,"context_line":"        uuid \u003d filters.pop(\u0027uuid\u0027, None)"},{"line_number":496,"context_line":"        can_host \u003d filters.pop(\u0027can_host\u0027, 0)"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a77a97e_10aafcd4","line":493,"in_reply_to":"9a629dbe_11ddf3df","updated":"2016-11-14 20:57:58.000000000","message":"Better yet, let\u0027s have a get_all_by_scheduler_request() that is specific to this particular need instead of trying to make one giant get_all_by_filters() method...","commit_id":"40a612b952e520c7d80b00485213669068e4b331"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"bbbf513300ecea19608ac01ef8825e76819a3c9c","unresolved":false,"context_lines":[{"line_number":490,"context_line":"        if not filters:"},{"line_number":491,"context_line":"            filters \u003d {}"},{"line_number":492,"context_line":"        else:"},{"line_number":493,"context_line":"            filters \u003d filters.copy()"},{"line_number":494,"context_line":"        name \u003d filters.pop(\u0027name\u0027, None)"},{"line_number":495,"context_line":"        uuid \u003d filters.pop(\u0027uuid\u0027, None)"},{"line_number":496,"context_line":"        can_host \u003d filters.pop(\u0027can_host\u0027, 0)"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a77a97e_7dadc887","line":493,"in_reply_to":"9a629dbe_11ddf3df","updated":"2016-11-14 21:57:53.000000000","message":"Yeah, you would need to use copy.deepcopy() anyway because of the nested nature of the filters dict.","commit_id":"40a612b952e520c7d80b00485213669068e4b331"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"fb127584fc7b07a47888e4a760e6cd9663f0c66d","unresolved":false,"context_lines":[{"line_number":515,"context_line":"        query \u003d query.join("},{"line_number":516,"context_line":"            models.Inventory,"},{"line_number":517,"context_line":"            models.ResourceProvider.id \u003d\u003d ("},{"line_number":518,"context_line":"                models.Inventory.resource_provider_id))"},{"line_number":519,"context_line":""},{"line_number":520,"context_line":"        # ...and the allocations table as an aggregated usage table"},{"line_number":521,"context_line":"        usage \u003d sa.select([_ALLOC_TBL.c.resource_provider_id,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a77a97e_70e7b040","line":518,"updated":"2016-11-14 20:57:58.000000000","message":"nit: you don\u0027t need the inner parens above. And actually, it would be a bit neater to just put the join clause on a separate line:\n\n join_clause \u003d _RP_TBL.c.id \u003d\u003d _INV_TBL.c.resource_provider_id\n query \u003d query.join(_INV_TBL, join_clause)","commit_id":"40a612b952e520c7d80b00485213669068e4b331"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"fb127584fc7b07a47888e4a760e6cd9663f0c66d","unresolved":false,"context_lines":[{"line_number":537,"context_line":"        where_clauses \u003d ["},{"line_number":538,"context_line":"            sa.and_("},{"line_number":539,"context_line":"                _INV_TBL.c.resource_class_id \u003d\u003d r_idx,"},{"line_number":540,"context_line":"                sa.not_(func.coalesce(usage.c.used, 0) + amount \u003e ("},{"line_number":541,"context_line":"                    _INV_TBL.c.total - _INV_TBL.c.reserved"},{"line_number":542,"context_line":"                ) * _INV_TBL.c.allocation_ratio))"},{"line_number":543,"context_line":"            for (r_idx, amount) in six.iteritems(resources)]"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a77a97e_10b03cf4","line":540,"updated":"2016-11-14 20:57:58.000000000","message":"You don\u0027t need the NOT clause here. You could just do:\n\n (func.coalesce(usage.c.used, 0) + amount \u003c\u003d\n     (_INV_TBL.c.total - _INV_TBL.c.reserved) *\n     _INV_TBL.c.allocation_ratio),\n\nMost SQL DB optimizers will automatically rewrite such an expression to the simpler positive expression, but it would be easier to read without the NOT in there...","commit_id":"40a612b952e520c7d80b00485213669068e4b331"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"fb127584fc7b07a47888e4a760e6cd9663f0c66d","unresolved":false,"context_lines":[{"line_number":539,"context_line":"                _INV_TBL.c.resource_class_id \u003d\u003d r_idx,"},{"line_number":540,"context_line":"                sa.not_(func.coalesce(usage.c.used, 0) + amount \u003e ("},{"line_number":541,"context_line":"                    _INV_TBL.c.total - _INV_TBL.c.reserved"},{"line_number":542,"context_line":"                ) * _INV_TBL.c.allocation_ratio))"},{"line_number":543,"context_line":"            for (r_idx, amount) in six.iteritems(resources)]"},{"line_number":544,"context_line":"        query \u003d query.filter(sa.or_(*where_clauses))"},{"line_number":545,"context_line":"        query \u003d query.group_by(_RP_TBL.c.uuid)"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a77a97e_50e4740c","line":542,"updated":"2016-11-14 20:57:58.000000000","message":"We will also need to add the min_unit, max_unit and step_size to the WHERE clauses:\n\n sa.and_(\n    ...,\n    _INV_TBL.c.min_unit \u003e\u003d amount,\n    _INV_TBL.c.max_unit \u003c\u003d amount,\n    amount % _INV_TBL.c.step_size \u003d\u003d 0,\n )","commit_id":"40a612b952e520c7d80b00485213669068e4b331"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"79d69192395f0da5888d549ea7e7262937d2ca3c","unresolved":false,"context_lines":[{"line_number":539,"context_line":"                _INV_TBL.c.resource_class_id \u003d\u003d r_idx,"},{"line_number":540,"context_line":"                sa.not_(func.coalesce(usage.c.used, 0) + amount \u003e ("},{"line_number":541,"context_line":"                    _INV_TBL.c.total - _INV_TBL.c.reserved"},{"line_number":542,"context_line":"                ) * _INV_TBL.c.allocation_ratio))"},{"line_number":543,"context_line":"            for (r_idx, amount) in six.iteritems(resources)]"},{"line_number":544,"context_line":"        query \u003d query.filter(sa.or_(*where_clauses))"},{"line_number":545,"context_line":"        query \u003d query.group_by(_RP_TBL.c.uuid)"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a77a97e_9426c766","line":542,"in_reply_to":"7a77a97e_50e4740c","updated":"2016-11-15 21:50:43.000000000","message":"You have the signs backwards :)\n\nAlso, adding these will require that we specify them in tests where we create inventory, since the default of 1, especially for max_unit, will cause most requests to fail.","commit_id":"40a612b952e520c7d80b00485213669068e4b331"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"7074d39102921fd9be83df02d589a32441218e8d","unresolved":false,"context_lines":[{"line_number":541,"context_line":"                    _INV_TBL.c.total - _INV_TBL.c.reserved"},{"line_number":542,"context_line":"                ) * _INV_TBL.c.allocation_ratio))"},{"line_number":543,"context_line":"            for (r_idx, amount) in six.iteritems(resources)]"},{"line_number":544,"context_line":"        query \u003d query.filter(sa.or_(*where_clauses))"},{"line_number":545,"context_line":"        query \u003d query.group_by(_RP_TBL.c.uuid)"},{"line_number":546,"context_line":"        # NOTE(sbauza): Only RPs having all the asked resources can be provided"},{"line_number":547,"context_line":"        query \u003d query.having(sql.func.count("}],"source_content_type":"text/x-python","patch_set":6,"id":"9a629dbe_7101cf05","line":544,"updated":"2016-11-10 13:36:47.000000000","message":"Maybe I\u0027m just not understanding the SQL correctly, but I\u0027m still confused about why this is an `or_`. We want all the inventories to match our request, so how does an or do that? Is this what the having clause is clearing up? It would be great for this to be somewhat more understandable. The display of the generated SQL above is a great step, but since it is itself so complex it might make sense to explain it in english too.","commit_id":"40a612b952e520c7d80b00485213669068e4b331"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"d3a931d2bd73564cbc73cdc6eb2b40dea1509886","unresolved":false,"context_lines":[{"line_number":541,"context_line":"                    _INV_TBL.c.total - _INV_TBL.c.reserved"},{"line_number":542,"context_line":"                ) * _INV_TBL.c.allocation_ratio))"},{"line_number":543,"context_line":"            for (r_idx, amount) in six.iteritems(resources)]"},{"line_number":544,"context_line":"        query \u003d query.filter(sa.or_(*where_clauses))"},{"line_number":545,"context_line":"        query \u003d query.group_by(_RP_TBL.c.uuid)"},{"line_number":546,"context_line":"        # NOTE(sbauza): Only RPs having all the asked resources can be provided"},{"line_number":547,"context_line":"        query \u003d query.having(sql.func.count("}],"source_content_type":"text/x-python","patch_set":6,"id":"7a77a97e_cbc0d5e3","line":544,"in_reply_to":"7a77a97e_9069ac8e","updated":"2016-11-14 21:44:00.000000000","message":"Thanks, I don\u0027t really get it, still, I\u0027d probably need a whiteboard and/or some sample data, but I\u0027ll happily take your word for it.","commit_id":"40a612b952e520c7d80b00485213669068e4b331"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"50c4b7051c464d90e58153d43204a90b70b7db89","unresolved":false,"context_lines":[{"line_number":541,"context_line":"                    _INV_TBL.c.total - _INV_TBL.c.reserved"},{"line_number":542,"context_line":"                ) * _INV_TBL.c.allocation_ratio))"},{"line_number":543,"context_line":"            for (r_idx, amount) in six.iteritems(resources)]"},{"line_number":544,"context_line":"        query \u003d query.filter(sa.or_(*where_clauses))"},{"line_number":545,"context_line":"        query \u003d query.group_by(_RP_TBL.c.uuid)"},{"line_number":546,"context_line":"        # NOTE(sbauza): Only RPs having all the asked resources can be provided"},{"line_number":547,"context_line":"        query \u003d query.having(sql.func.count("}],"source_content_type":"text/x-python","patch_set":6,"id":"7a77a97e_d4ce7f82","line":544,"in_reply_to":"7a77a97e_c5140ff7","updated":"2016-11-15 21:52:18.000000000","message":"Oh, never mind - I see that we added a hack to make sure the count of requested resource classes matches the output in 547-8","commit_id":"40a612b952e520c7d80b00485213669068e4b331"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"3fb5bce9a6e28951b7c40565b512d7cb259fddbd","unresolved":false,"context_lines":[{"line_number":541,"context_line":"                    _INV_TBL.c.total - _INV_TBL.c.reserved"},{"line_number":542,"context_line":"                ) * _INV_TBL.c.allocation_ratio))"},{"line_number":543,"context_line":"            for (r_idx, amount) in six.iteritems(resources)]"},{"line_number":544,"context_line":"        query \u003d query.filter(sa.or_(*where_clauses))"},{"line_number":545,"context_line":"        query \u003d query.group_by(_RP_TBL.c.uuid)"},{"line_number":546,"context_line":"        # NOTE(sbauza): Only RPs having all the asked resources can be provided"},{"line_number":547,"context_line":"        query \u003d query.having(sql.func.count("}],"source_content_type":"text/x-python","patch_set":6,"id":"7a77a97e_c5140ff7","line":544,"in_reply_to":"7a77a97e_cbc0d5e3","updated":"2016-11-15 19:44:24.000000000","message":"It is because a RP can provide several classes. In the case of a compute node, each of those WHERE clauses correspond to VCPU, disk, and RAM. There are no inventories that match all three at once (AND). I\u0027m concerned that if it matches disk but not RAM of VCPU, that it will still be returned. SQLA is just too odd for me to grok.","commit_id":"40a612b952e520c7d80b00485213669068e4b331"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"fb127584fc7b07a47888e4a760e6cd9663f0c66d","unresolved":false,"context_lines":[{"line_number":541,"context_line":"                    _INV_TBL.c.total - _INV_TBL.c.reserved"},{"line_number":542,"context_line":"                ) * _INV_TBL.c.allocation_ratio))"},{"line_number":543,"context_line":"            for (r_idx, amount) in six.iteritems(resources)]"},{"line_number":544,"context_line":"        query \u003d query.filter(sa.or_(*where_clauses))"},{"line_number":545,"context_line":"        query \u003d query.group_by(_RP_TBL.c.uuid)"},{"line_number":546,"context_line":"        # NOTE(sbauza): Only RPs having all the asked resources can be provided"},{"line_number":547,"context_line":"        query \u003d query.having(sql.func.count("}],"source_content_type":"text/x-python","patch_set":6,"id":"7a77a97e_9069ac8e","line":544,"in_reply_to":"9a629dbe_7101cf05","updated":"2016-11-14 20:57:58.000000000","message":"The SQL is correct as is the SQLAlchemy join condition collection here. Basically, for each resource class requested, we need to have an OR\u0027d WHERE condition that specifically matches the inventory and usage for that resource class out of the inventories table and allocs derived table (subquery in the FROM clause).\n\nThe key to this all working is the HAVING COUNT(*) \u003d\u003d \u003cNUM_RESOURCE_CLASSES\u003e. Alternately we could do self-joins to copies of the inventories table and multiple derived tables representing one requested resource class for each derived table. That would be more efficient for larger data sets but would be even more difficult for the code reader to understand ;)","commit_id":"40a612b952e520c7d80b00485213669068e4b331"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"4123c016a098d34bca1537d0f19d38765a08ef5d","unresolved":false,"context_lines":[{"line_number":554,"context_line":"        #  COUNT(DISTINCT(inv.resource_class_id)) \u003d\u003d len($RESOURCE_CLASSES)"},{"line_number":555,"context_line":"        #"},{"line_number":556,"context_line":"        # with a possible additional WHERE clause for the name and uuid"},{"line_number":557,"context_line":""},{"line_number":558,"context_line":"        return query.all()"},{"line_number":559,"context_line":""},{"line_number":560,"context_line":"    @base.remotable_classmethod"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a77a97e_f3d61926","line":557,"updated":"2016-11-16 11:20:05.000000000","message":"moving this down here seems like a good idea","commit_id":"4b70f6af11e663e99ca5e61c86db0109f2022fa1"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"af0281995adc81f3ea803a029e15996b46928944","unresolved":false,"context_lines":[{"line_number":508,"context_line":"        # ) AS usage"},{"line_number":509,"context_line":"        #     ON inv.resource_provider_id \u003d usage.resource_provider_id"},{"line_number":510,"context_line":"        #     AND inv.resource_class_id \u003d usage.resource_class_id"},{"line_number":511,"context_line":"        # WHERE rp.can_host \u003d\u003d 0"},{"line_number":512,"context_line":"        # AND (inv.resource_class_id \u003d $X AND (used + $AMOUNT_X \u003c\u003d ("},{"line_number":513,"context_line":"        #        total + reserved) * inv.allocation_ratio) AND"},{"line_number":514,"context_line":"        #        inv.min_unit \u003c\u003d $AMOUNT_X AND inv.max_unit \u003e\u003d $AMOUNT_X AND"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a77a97e_8a6f49c4","line":511,"updated":"2016-11-16 20:06:06.000000000","message":"You can remove this can_host \u003d\u003d 0 line.","commit_id":"33e3d7d959fd711a05cbf1e176eb02b254eea04f"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"fa27586396b03899f23da6f72ba66b828d31acbe","unresolved":false,"context_lines":[{"line_number":552,"context_line":"        :param context: `nova.context.RequestContext` that may be used to grab"},{"line_number":553,"context_line":"                        a DB connection."},{"line_number":554,"context_line":"        :param filters: Dict of requested amounts keyed by resource classes,"},{"line_number":555,"context_line":"                        name and uuid being special cases."},{"line_number":556,"context_line":"        \"\"\""},{"line_number":557,"context_line":"        if not filters:"},{"line_number":558,"context_line":"            filters \u003d {}"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a74a57a_b7be60fd","line":555,"updated":"2016-11-30 19:04:09.000000000","message":"Can we have a \u0027resources\u0027 element that is a dict, keyed by resource class, of amounts being requested? I dislike the special-casing thing here and using the top-level filters dict to house the resource class keys themselves.","commit_id":"89282f9060f790a78952ffc2560c6778cc75231d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"7d9971e60a0f08ee71dbe6f9c82e536430f7ca2a","unresolved":false,"context_lines":[{"line_number":552,"context_line":"        :param context: `nova.context.RequestContext` that may be used to grab"},{"line_number":553,"context_line":"                        a DB connection."},{"line_number":554,"context_line":"        :param filters: Dict of requested amounts keyed by resource classes,"},{"line_number":555,"context_line":"                        name and uuid being special cases."},{"line_number":556,"context_line":"        \"\"\""},{"line_number":557,"context_line":"        if not filters:"},{"line_number":558,"context_line":"            filters \u003d {}"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a74a57a_d2fe8aad","line":555,"in_reply_to":"5a74a57a_770cc842","updated":"2016-11-30 19:31:37.000000000","message":"sorry, no I meant to have the filters parameter look like this:\n\n filters \u003d {\n   \u0027name\u0027: \u003cname\u003e,\n   \u0027uuid\u0027: \u003cuuid\u003e,\n   \u0027resources\u0027: {\n     \u0027VCPU\u0027: 1,\n     \u0027MEMORY_MB\u0027: 1024\n   }\n }\n\nsorry for the confusion!","commit_id":"89282f9060f790a78952ffc2560c6778cc75231d"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"45d34811c14da21b969b1c7ce10644c82493d650","unresolved":false,"context_lines":[{"line_number":552,"context_line":"        :param context: `nova.context.RequestContext` that may be used to grab"},{"line_number":553,"context_line":"                        a DB connection."},{"line_number":554,"context_line":"        :param filters: Dict of requested amounts keyed by resource classes,"},{"line_number":555,"context_line":"                        name and uuid being special cases."},{"line_number":556,"context_line":"        \"\"\""},{"line_number":557,"context_line":"        if not filters:"},{"line_number":558,"context_line":"            filters \u003d {}"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a74a57a_770cc842","line":555,"in_reply_to":"5a74a57a_b7be60fd","updated":"2016-11-30 19:21:05.000000000","message":"Just to understand correctly your concern, you\u0027d prefer a signature like this :\n\n  def _get_all_by_filters_from_db(context, filters, resources\u003dNone):\n\n\ncorrect?","commit_id":"89282f9060f790a78952ffc2560c6778cc75231d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"fa27586396b03899f23da6f72ba66b828d31acbe","unresolved":false,"context_lines":[{"line_number":567,"context_line":"        # NOTE(sbauza): We want to key the dict by the resource class IDs."},{"line_number":568,"context_line":"        resources \u003d {_RC_CACHE.id_from_string(r_name): filters[r_name]"},{"line_number":569,"context_line":"                     for r_name in filters"},{"line_number":570,"context_line":"                     if _RC_CACHE.id_from_string(r_name) is not None}"},{"line_number":571,"context_line":"        query \u003d context.session.query(models.ResourceProvider)"},{"line_number":572,"context_line":"        if name:"},{"line_number":573,"context_line":"            query \u003d query.filter(models.ResourceProvider.name \u003d\u003d name)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a74a57a_b75580c7","line":570,"updated":"2016-11-30 19:04:09.000000000","message":"So, here would instead be:\n\n resources \u003d filters.pop(\u0027resources\u0027, {})\n resources \u003d {_RC_CACHE.id_from_string(r_name): amount\n              for r_name, amount in resources.items()}","commit_id":"89282f9060f790a78952ffc2560c6778cc75231d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"fa27586396b03899f23da6f72ba66b828d31acbe","unresolved":false,"context_lines":[{"line_number":658,"context_line":"        return query.all()"},{"line_number":659,"context_line":""},{"line_number":660,"context_line":"    @base.remotable_classmethod"},{"line_number":661,"context_line":"    def get_all_by_filters(cls, context, filters\u003dNone):"},{"line_number":662,"context_line":"        resource_providers \u003d cls._get_all_by_filters_from_db(context, filters)"},{"line_number":663,"context_line":"        return base.obj_make_list(context, cls(context),"},{"line_number":664,"context_line":"                                  objects.ResourceProvider, resource_providers)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a74a57a_375890fa","line":661,"updated":"2016-11-30 19:04:09.000000000","message":"nit: would be best to put the docstring from above on the public-facing method, not the internal one.","commit_id":"89282f9060f790a78952ffc2560c6778cc75231d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"4e3a541d93cda9f1a1cc6138946372cd934ed2d3","unresolved":false,"context_lines":[{"line_number":550,"context_line":"        #          \u0027VCPU\u0027: 1,"},{"line_number":551,"context_line":"        #          \u0027MEMORY_MB\u0027: 1024"},{"line_number":552,"context_line":"        #      }"},{"line_number":553,"context_line":"        #  }"},{"line_number":554,"context_line":"        if not filters:"},{"line_number":555,"context_line":"            filters \u003d {}"},{"line_number":556,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a71b18c_0f51ef0e","line":553,"updated":"2016-12-07 17:46:16.000000000","message":"++","commit_id":"14a7b858eac22041b44724eb2532b552afc590db"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"4e3a541d93cda9f1a1cc6138946372cd934ed2d3","unresolved":false,"context_lines":[{"line_number":651,"context_line":"        query \u003d query.group_by(_RP_TBL.c.uuid)"},{"line_number":652,"context_line":"        # NOTE(sbauza): Only RPs having all the asked resources can be provided"},{"line_number":653,"context_line":"        query \u003d query.having(sql.func.count("},{"line_number":654,"context_line":"            sa.distinct(_INV_TBL.c.resource_class_id)) \u003d\u003d len(resources))"},{"line_number":655,"context_line":""},{"line_number":656,"context_line":"        return query.all()"},{"line_number":657,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3a71b18c_4ff767f5","line":654,"updated":"2016-12-07 17:46:16.000000000","message":"++ all the above query structuring looks proper.","commit_id":"14a7b858eac22041b44724eb2532b552afc590db"}],"nova/tests/functional/db/test_resource_provider.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"af0281995adc81f3ea803a029e15996b46928944","unresolved":false,"context_lines":[{"line_number":598,"context_line":"        resource_providers \u003d objects.ResourceProviderList.get_all_by_filters("},{"line_number":599,"context_line":"            self.context, {\u0027name\u0027: \u0027rp_name_1\u0027,"},{"line_number":600,"context_line":"                           fields.ResourceClass.VCPU: 1})"},{"line_number":601,"context_line":"        self.assertEqual(1, len(resource_providers))"},{"line_number":602,"context_line":""},{"line_number":603,"context_line":""},{"line_number":604,"context_line":"class TestAllocation(ResourceProviderBaseCase):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a77a97e_0af339d7","line":601,"updated":"2016-11-16 20:06:06.000000000","message":"good start on these functional tests. I\u0027d also like to see some tests that stress the WHERE conditions that use the min_unit, max_unit, and step_size fields.","commit_id":"33e3d7d959fd711a05cbf1e176eb02b254eea04f"}]}