)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"731d150600ca5aef9a2b2df9c08253e63a0b8992","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This is the first phase of adding support for shared resource providers to the"},{"line_number":10,"context_line":"scheduler. We add a new function \"_get_providers_with_shared_capacity()\" to"},{"line_number":11,"context_line":"nova/objects/resource_provider.py that accepts a resource class identifier and"},{"line_number":12,"context_line":"a requested amount of that resource class and we return a set() of identifiers"},{"line_number":13,"context_line":"for resource providers that a) are marked as sharing their inventory via an"},{"line_number":14,"context_line":"aggregate (with the new MISC_SHARES_VIA_AGGREGATE trait) and b) have enough"},{"line_number":15,"context_line":"capacity to service the requested amount of the resource."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"3f044301_6359f858","line":12,"range":{"start_line":12,"start_character":58,"end_line":12,"end_character":63},"updated":"2017-05-04 16:36:26.000000000","message":"You\u0027re returning a list, not a set(). Should it return a set or should this say list? If you hadn\u0027t included the () I would assume you mean set in the non-mathematical sense, but...","commit_id":"dc21a3c78ae4902ce1b94a5c71e295a5d5b418bf"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3b9c9d850fd9a6f9931515717d60672aed3a7da0","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This is the first phase of adding support for shared resource providers to the"},{"line_number":10,"context_line":"scheduler. We add a new function \"_get_providers_with_shared_capacity()\" to"},{"line_number":11,"context_line":"nova/objects/resource_provider.py that accepts a resource class identifier and"},{"line_number":12,"context_line":"a requested amount of that resource class and we return a set() of identifiers"},{"line_number":13,"context_line":"for resource providers that a) are marked as sharing their inventory via an"},{"line_number":14,"context_line":"aggregate (with the new MISC_SHARES_VIA_AGGREGATE trait) and b) have enough"},{"line_number":15,"context_line":"capacity to service the requested amount of the resource."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"3f044301_860ad223","line":12,"range":{"start_line":12,"start_character":58,"end_line":12,"end_character":63},"in_reply_to":"3f044301_6359f858","updated":"2017-05-04 16:57:42.000000000","message":"Used to be a set. Will change to list.","commit_id":"dc21a3c78ae4902ce1b94a5c71e295a5d5b418bf"}],"nova/objects/resource_provider.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"08b489acf56c5c8137c116da654e8f71944ae920","unresolved":false,"context_lines":[{"line_number":589,"context_line":"@db_api.api_context_manager.reader"},{"line_number":590,"context_line":"def _get_providers_with_shared_capacity(ctx, rc_id, amount):"},{"line_number":591,"context_line":"    \"\"\"Returns a set of resource provider IDs (internal IDs, not UUIDs)"},{"line_number":592,"context_line":"    have capacity for a requested amount of a resource and indicate that they"},{"line_number":593,"context_line":"    share resource via an aggregate association."},{"line_number":594,"context_line":""},{"line_number":595,"context_line":"    Shared resource providers are marked with a standard trait called"}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_c89a8760","line":592,"updated":"2017-04-28 13:18:53.000000000","message":"\"that have\" or \"having\"?","commit_id":"0fdcd25b5ed96121a119e46b518c9bbb86eb2d5b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"bd0b1a3cf3e810a1184725c35164d8b04c452bf0","unresolved":false,"context_lines":[{"line_number":589,"context_line":"@db_api.api_context_manager.reader"},{"line_number":590,"context_line":"def _get_providers_with_shared_capacity(ctx, rc_id, amount):"},{"line_number":591,"context_line":"    \"\"\"Returns a set of resource provider IDs (internal IDs, not UUIDs)"},{"line_number":592,"context_line":"    have capacity for a requested amount of a resource and indicate that they"},{"line_number":593,"context_line":"    share resource via an aggregate association."},{"line_number":594,"context_line":""},{"line_number":595,"context_line":"    Shared resource providers are marked with a standard trait called"}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_cb584132","line":592,"in_reply_to":"5ff73747_c89a8760","updated":"2017-04-28 13:36:20.000000000","message":"Done","commit_id":"0fdcd25b5ed96121a119e46b518c9bbb86eb2d5b"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"08b489acf56c5c8137c116da654e8f71944ae920","unresolved":false,"context_lines":[{"line_number":618,"context_line":""},{"line_number":619,"context_line":"    To facilitate that matching query, this function returns all providers that"},{"line_number":620,"context_line":"    indicate they share their inventory with other providers in the same"},{"line_number":621,"context_line":"    aggregate and have enough capacity for the requested amount of a resource."},{"line_number":622,"context_line":""},{"line_number":623,"context_line":"    To follow the example above, if we were to call"},{"line_number":624,"context_line":"    _get_providers_with_shared_capacity(ctx, \"DISK_GB\", 100), we would want to"}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_4853173a","line":621,"updated":"2017-04-28 13:18:53.000000000","message":"Not yet it doesn\u0027t, unless I\u0027m missing something.\n\nAs far as I can tell, the aggregate isn\u0027t an input to this and nowhere in the SQL are aggregate uuids involved.\n\nWhat we\u0027re returning here is a list of resource providers that can share inventory and have enough to cover amount. aggregate is not involved.","commit_id":"0fdcd25b5ed96121a119e46b518c9bbb86eb2d5b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b4ea852b775b378fbde27d9de4db90cfe5ff4124","unresolved":false,"context_lines":[{"line_number":618,"context_line":""},{"line_number":619,"context_line":"    To facilitate that matching query, this function returns all providers that"},{"line_number":620,"context_line":"    indicate they share their inventory with other providers in the same"},{"line_number":621,"context_line":"    aggregate and have enough capacity for the requested amount of a resource."},{"line_number":622,"context_line":""},{"line_number":623,"context_line":"    To follow the example above, if we were to call"},{"line_number":624,"context_line":"    _get_providers_with_shared_capacity(ctx, \"DISK_GB\", 100), we would want to"}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_ce3e0ac0","line":621,"in_reply_to":"5ff73747_0ea757bf","updated":"2017-05-01 14:26:01.000000000","message":"No, #1 wouldn\u0027t predispose workloads to prefer local disk. And on #2, instead of a config option, we could have some little cached boolean that would be triggered if, say, the placement API sees that most (all?) queries for resource providers that contain a \"resources\" filter result in needing to look for shared providers.","commit_id":"0fdcd25b5ed96121a119e46b518c9bbb86eb2d5b"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"b450074e4f90d89e3620da4c0c27e1d9ae2e5572","unresolved":false,"context_lines":[{"line_number":618,"context_line":""},{"line_number":619,"context_line":"    To facilitate that matching query, this function returns all providers that"},{"line_number":620,"context_line":"    indicate they share their inventory with other providers in the same"},{"line_number":621,"context_line":"    aggregate and have enough capacity for the requested amount of a resource."},{"line_number":622,"context_line":""},{"line_number":623,"context_line":"    To follow the example above, if we were to call"},{"line_number":624,"context_line":"    _get_providers_with_shared_capacity(ctx, \"DISK_GB\", 100), we would want to"}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_0ea757bf","line":621,"in_reply_to":"5ff73747_2b09e594","updated":"2017-04-28 13:59:00.000000000","message":"Would (1) predispose workloads to prefer local disk and (2) would somewhat randomize things? Is that something people will not want or want to control?\n\nAll else being equal, avoiding yet more config options is a win.","commit_id":"0fdcd25b5ed96121a119e46b518c9bbb86eb2d5b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"bd0b1a3cf3e810a1184725c35164d8b04c452bf0","unresolved":false,"context_lines":[{"line_number":618,"context_line":""},{"line_number":619,"context_line":"    To facilitate that matching query, this function returns all providers that"},{"line_number":620,"context_line":"    indicate they share their inventory with other providers in the same"},{"line_number":621,"context_line":"    aggregate and have enough capacity for the requested amount of a resource."},{"line_number":622,"context_line":""},{"line_number":623,"context_line":"    To follow the example above, if we were to call"},{"line_number":624,"context_line":"    _get_providers_with_shared_capacity(ctx, \"DISK_GB\", 100), we would want to"}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_2b09e594","line":621,"in_reply_to":"5ff73747_4853173a","updated":"2017-04-28 13:36:20.000000000","message":"@cdent: Yeah, I think the problem is mostly with wording. I was trying to say \"get providers that have capacity for the supplied amount of a resource and that indicate they share those resources with other providers in *any* aggregate\". pff, words! :)\n\nTo your comment on the general review...\n\nSo, I had two ideas on how to integrate this little function into the RPList.get_all_by_filters() code. I\u0027m on the fence regarding which is \"best\":\n\n1) Keep get_all_by_filters() pretty much as is, but put a little conditional at the end of that function that checks if no providers were found that meet the original SQL query (which, as you\u0027re aware, checks for providers that have *all* of the requested resources). If no providers were found, then go into a second function that builds the more complicated SQL expression that follows the pattern highlighted in the code comments here:\n\nhttps://github.com/jaypipes/placement-bench/blob/master/db.py#L276-L308\n\nand adds in another JOIN to:\n\n (\n SELECT aggregate_id FROM resource_provider_aggregates\n WHERE resource_provider_id IN \u003cset of IDs returned from _get_providers_with_shared_capacity()\u003e\n )\n\n2) Change the get_all_by_filters() to use the more complicated SQL construction in the link above and always grab the list of resource providers that share inventory with other providers in an aggregate.\n\nLike I said, I\u0027m on the fence about it... #1 would be simpler and less queries in the non-shared-disk-storage deployments but #2 would be more efficient in the shared-disk-storage deployments.\n\nI suppose we could get the best of both worlds by putting in a config option too...","commit_id":"0fdcd25b5ed96121a119e46b518c9bbb86eb2d5b"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"34eb479f03c0f1499197f67c696e84eb4bd162f5","unresolved":false,"context_lines":[{"line_number":618,"context_line":""},{"line_number":619,"context_line":"    To facilitate that matching query, this function returns all providers that"},{"line_number":620,"context_line":"    indicate they share their inventory with other providers in the same"},{"line_number":621,"context_line":"    aggregate and have enough capacity for the requested amount of a resource."},{"line_number":622,"context_line":""},{"line_number":623,"context_line":"    To follow the example above, if we were to call"},{"line_number":624,"context_line":"    _get_providers_with_shared_capacity(ctx, \"DISK_GB\", 100), we would want to"}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_755567f9","line":621,"in_reply_to":"5ff73747_ce3e0ac0","updated":"2017-05-01 16:24:31.000000000","message":"I\u0027m still confused. On #1 you say \"If no providers were found, then go into a second function\" where the first function is checking \"for providers that have *all* of the requested resources\".\n\nIf we return after the first function, we are returning only those providers that all the requested resources, meaning resource providers which have vcpus, ram and disk in an of themselves, meaning compute nodes, meaning local disk.\n\nSo while there is enough local disk available for workloads, we will always choose it (because we return after the first function) until we run out of local disk.\n\nIs that analysis correct? If so, I would guess that could be considered undesirable.","commit_id":"0fdcd25b5ed96121a119e46b518c9bbb86eb2d5b"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"df8f971722c7f6b0165518f9210955363dbbff41","unresolved":false,"context_lines":[{"line_number":665,"context_line":"        rp_to_rpt_join, t_tbl,"},{"line_number":666,"context_line":"        sa.and_("},{"line_number":667,"context_line":"            rpt_tbl.c.trait_id \u003d\u003d t_tbl.c.id,"},{"line_number":668,"context_line":"            t_tbl.c.name \u003d\u003d \u0027MISC_SHARES_VIA_AGGREGATE\u0027,"},{"line_number":669,"context_line":"        ),"},{"line_number":670,"context_line":"    )"},{"line_number":671,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_c8b727f5","line":668,"range":{"start_line":668,"start_character":28,"end_line":668,"end_character":55},"updated":"2017-04-28 13:12:35.000000000","message":"I know they are similar, but if we\u0027re going to hav ethe os_traits library, we should use it, and make the right side of this expression: os_traits.MISC_SHARES_VIA_AGGREGATE","commit_id":"0fdcd25b5ed96121a119e46b518c9bbb86eb2d5b"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"08b489acf56c5c8137c116da654e8f71944ae920","unresolved":false,"context_lines":[{"line_number":665,"context_line":"        rp_to_rpt_join, t_tbl,"},{"line_number":666,"context_line":"        sa.and_("},{"line_number":667,"context_line":"            rpt_tbl.c.trait_id \u003d\u003d t_tbl.c.id,"},{"line_number":668,"context_line":"            t_tbl.c.name \u003d\u003d \u0027MISC_SHARES_VIA_AGGREGATE\u0027,"},{"line_number":669,"context_line":"        ),"},{"line_number":670,"context_line":"    )"},{"line_number":671,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_68d93320","line":668,"updated":"2017-04-28 13:18:53.000000000","message":"When your os_traits change merges that adds this, would we use the symbol here instead? If not, why?","commit_id":"0fdcd25b5ed96121a119e46b518c9bbb86eb2d5b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"bd0b1a3cf3e810a1184725c35164d8b04c452bf0","unresolved":false,"context_lines":[{"line_number":665,"context_line":"        rp_to_rpt_join, t_tbl,"},{"line_number":666,"context_line":"        sa.and_("},{"line_number":667,"context_line":"            rpt_tbl.c.trait_id \u003d\u003d t_tbl.c.id,"},{"line_number":668,"context_line":"            t_tbl.c.name \u003d\u003d \u0027MISC_SHARES_VIA_AGGREGATE\u0027,"},{"line_number":669,"context_line":"        ),"},{"line_number":670,"context_line":"    )"},{"line_number":671,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_cb0a811e","line":668,"range":{"start_line":668,"start_character":28,"end_line":668,"end_character":55},"in_reply_to":"5ff73747_c8b727f5","updated":"2017-04-28 13:36:20.000000000","message":"Ya, totes agree :) It\u0027s just that https://review.openstack.org/#/c/460281/ wasn\u0027t merged yet so os-traits with that patch hasn\u0027t been released and therefore I didn\u0027t have this trait in the locally-installed os-traits during testing.\n\nIs there a way I can do a depends-on thing that basically brings in a patch from os-traits repo as the dependent os-traits library version?","commit_id":"0fdcd25b5ed96121a119e46b518c9bbb86eb2d5b"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"b450074e4f90d89e3620da4c0c27e1d9ae2e5572","unresolved":false,"context_lines":[{"line_number":665,"context_line":"        rp_to_rpt_join, t_tbl,"},{"line_number":666,"context_line":"        sa.and_("},{"line_number":667,"context_line":"            rpt_tbl.c.trait_id \u003d\u003d t_tbl.c.id,"},{"line_number":668,"context_line":"            t_tbl.c.name \u003d\u003d \u0027MISC_SHARES_VIA_AGGREGATE\u0027,"},{"line_number":669,"context_line":"        ),"},{"line_number":670,"context_line":"    )"},{"line_number":671,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_ce428f0f","line":668,"in_reply_to":"5ff73747_cb0a811e","updated":"2017-04-28 13:59:00.000000000","message":"I don\u0027t think we can depend on os-traits because it would be pip installed. But it\u0027s not in requirements.txt yet, either, so...","commit_id":"0fdcd25b5ed96121a119e46b518c9bbb86eb2d5b"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"df8f971722c7f6b0165518f9210955363dbbff41","unresolved":false,"context_lines":[{"line_number":666,"context_line":"        sa.and_("},{"line_number":667,"context_line":"            rpt_tbl.c.trait_id \u003d\u003d t_tbl.c.id,"},{"line_number":668,"context_line":"            t_tbl.c.name \u003d\u003d \u0027MISC_SHARES_VIA_AGGREGATE\u0027,"},{"line_number":669,"context_line":"        ),"},{"line_number":670,"context_line":"    )"},{"line_number":671,"context_line":""},{"line_number":672,"context_line":"    rp_to_inv_join \u003d sa.join("}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_081aafca","line":669,"updated":"2017-04-28 13:12:35.000000000","message":"Oh, wait, I just saw your TODO in the test. This is fine for now, then, but please add a similar TODO here.","commit_id":"0fdcd25b5ed96121a119e46b518c9bbb86eb2d5b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"bd0b1a3cf3e810a1184725c35164d8b04c452bf0","unresolved":false,"context_lines":[{"line_number":666,"context_line":"        sa.and_("},{"line_number":667,"context_line":"            rpt_tbl.c.trait_id \u003d\u003d t_tbl.c.id,"},{"line_number":668,"context_line":"            t_tbl.c.name \u003d\u003d \u0027MISC_SHARES_VIA_AGGREGATE\u0027,"},{"line_number":669,"context_line":"        ),"},{"line_number":670,"context_line":"    )"},{"line_number":671,"context_line":""},{"line_number":672,"context_line":"    rp_to_inv_join \u003d sa.join("}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_4b16714b","line":669,"in_reply_to":"5ff73747_081aafca","updated":"2017-04-28 13:36:20.000000000","message":"Done","commit_id":"0fdcd25b5ed96121a119e46b518c9bbb86eb2d5b"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"21cc05b53d21e753173c6d00fb83ba4f63a5e6db","unresolved":false,"context_lines":[{"line_number":618,"context_line":""},{"line_number":619,"context_line":"    To facilitate that matching query, this function returns all providers that"},{"line_number":620,"context_line":"    indicate they share their inventory with providers in some aggregate and"},{"line_number":621,"context_line":"    have enough capacity for the requested amount of a resource."},{"line_number":622,"context_line":""},{"line_number":623,"context_line":"    To follow the example above, if we were to call"},{"line_number":624,"context_line":"    _get_providers_with_shared_capacity(ctx, \"DISK_GB\", 100), we would want to"}],"source_content_type":"text/x-python","patch_set":2,"id":"5ff73747_753fa786","line":621,"updated":"2017-05-01 16:40:49.000000000","message":"At what stage in the process does the association between a resource that shares and the resources that are associated (by aggregate) with that resource get made?\n\nSo far we\u0027ve got talk about aggregates but no actual poking at either of the AGG tables.\n\nThe result of this query is every resource provider that SHARES_VIA and has sufficient inventory of the desired resource class.","commit_id":"1fc560d863e672f3ecdf283c5154e551f97c871c"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f5dc8d7cd674986cbf3fc869936055332e7d556f","unresolved":false,"context_lines":[{"line_number":618,"context_line":""},{"line_number":619,"context_line":"    To facilitate that matching query, this function returns all providers that"},{"line_number":620,"context_line":"    indicate they share their inventory with providers in some aggregate and"},{"line_number":621,"context_line":"    have enough capacity for the requested amount of a resource."},{"line_number":622,"context_line":""},{"line_number":623,"context_line":"    To follow the example above, if we were to call"},{"line_number":624,"context_line":"    _get_providers_with_shared_capacity(ctx, \"DISK_GB\", 100), we would want to"}],"source_content_type":"text/x-python","patch_set":2,"id":"5ff73747_a8eb9210","line":621,"in_reply_to":"5ff73747_086726eb","updated":"2017-05-01 17:12:32.000000000","message":"Not sure where you get \"potentially giant\" in clause? Typically, there will be only a single rp id returned from this function for a single resource class.","commit_id":"1fc560d863e672f3ecdf283c5154e551f97c871c"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"ab45667fc0fa8d3621fcbe10b5be9e812c34985e","unresolved":false,"context_lines":[{"line_number":618,"context_line":""},{"line_number":619,"context_line":"    To facilitate that matching query, this function returns all providers that"},{"line_number":620,"context_line":"    indicate they share their inventory with providers in some aggregate and"},{"line_number":621,"context_line":"    have enough capacity for the requested amount of a resource."},{"line_number":622,"context_line":""},{"line_number":623,"context_line":"    To follow the example above, if we were to call"},{"line_number":624,"context_line":"    _get_providers_with_shared_capacity(ctx, \"DISK_GB\", 100), we would want to"}],"source_content_type":"text/x-python","patch_set":2,"id":"5ff73747_086726eb","line":621,"in_reply_to":"5ff73747_3527dfcc","updated":"2017-05-01 17:09:50.000000000","message":"Ah, okay, this explains my confusion. Not in a millions years would I have expected you to use two separate sql queries for this, and for the second one to include a potentially giant in_.\n\nThat just doesn\u0027t fit my my jay mental model, so I was trying to extract \"where\u0027s the magic\"?","commit_id":"1fc560d863e672f3ecdf283c5154e551f97c871c"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1539f4fa113d40e50f1367d47169ef6e874ae0aa","unresolved":false,"context_lines":[{"line_number":618,"context_line":""},{"line_number":619,"context_line":"    To facilitate that matching query, this function returns all providers that"},{"line_number":620,"context_line":"    indicate they share their inventory with providers in some aggregate and"},{"line_number":621,"context_line":"    have enough capacity for the requested amount of a resource."},{"line_number":622,"context_line":""},{"line_number":623,"context_line":"    To follow the example above, if we were to call"},{"line_number":624,"context_line":"    _get_providers_with_shared_capacity(ctx, \"DISK_GB\", 100), we would want to"}],"source_content_type":"text/x-python","patch_set":2,"id":"5ff73747_3527dfcc","line":621,"in_reply_to":"5ff73747_753fa786","updated":"2017-05-01 17:00:13.000000000","message":"Yeah, I am trying to keep the amount of code in this patch to a manageable number...\n\nAs mentioned in comments on the previous revision, the results of this function will be used in another join operation in the primary ResourceProviderList.get_all_by_filters().\n\nEssentially, what this function returns, as you point out, is all the resource providers that share a particular resource. So, the code in ResourceProviderList.get_all_by_filters() will end up doing the following (only if the results of the \"get me providers that meet ALL these requests\" returns 0 records:\n\n providers_sharing \u003d _get_providers_with_shared_capacity(rc_id, amount)\n sel \u003d sa.select([_RP_AGG_TBL.c.aggregate_id])\n sel \u003d sel.where(_RP_AGG_TBL.c.resource_provider_id.in_(providers_sharing))\n query \u003d query.outerjoin(_RP_AGG_TBL,\n     sa.and_(\n         _RP_AGG_TBL.c.resource_provider_id \u003d\u003d _RP_TBL.c.id,\n         _RP_AGG_TBL.c.aggregate_id.in_(sel)\n     ),\n )\n\nBut again, I wanted to keep the amount of code in this patch manageable...","commit_id":"1fc560d863e672f3ecdf283c5154e551f97c871c"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"087bd09bd2dbb2615b8fc50d468e6d0183a59ae0","unresolved":false,"context_lines":[{"line_number":618,"context_line":""},{"line_number":619,"context_line":"    To facilitate that matching query, this function returns all providers that"},{"line_number":620,"context_line":"    indicate they share their inventory with providers in some aggregate and"},{"line_number":621,"context_line":"    have enough capacity for the requested amount of a resource."},{"line_number":622,"context_line":""},{"line_number":623,"context_line":"    To follow the example above, if we were to call"},{"line_number":624,"context_line":"    _get_providers_with_shared_capacity(ctx, \"DISK_GB\", 100), we would want to"}],"source_content_type":"text/x-python","patch_set":2,"id":"5ff73747_c8b81e12","line":621,"in_reply_to":"5ff73747_a8eb9210","updated":"2017-05-01 17:17:55.000000000","message":"Theoretically I could have 10,000 different shared disk providers (because I\u0027m pathological), each one associated with my 10,000 different racks? If each of those have amount DISK_GB available, then we\u0027ll return all of them from this query, yes?","commit_id":"1fc560d863e672f3ecdf283c5154e551f97c871c"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"742327369c05c804d3e6cb306de91578d175266c","unresolved":false,"context_lines":[{"line_number":702,"context_line":"        ),"},{"line_number":703,"context_line":"    )"},{"line_number":704,"context_line":"    sel \u003d sel.group_by(rp_tbl.c.id)"},{"line_number":705,"context_line":"    return set(r[0] for r in ctx.session.execute(sel))"},{"line_number":706,"context_line":""},{"line_number":707,"context_line":""},{"line_number":708,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":3,"id":"5ff73747_ac8cab7f","line":705,"range":{"start_line":705,"start_character":11,"end_line":705,"end_character":14},"updated":"2017-05-02 13:35:24.000000000","message":"Not clear on why this is a set, since you don\u0027t use any set methods on the result here or in the next patch. If it\u0027s just to make things unique, it might be cleaner to use set() to get the uniqueness, and then cast the return value to a list.\n\nThis isn\u0027t terribly important, as it\u0027s an internal method, but I\u0027m asking more out of curiosity than any concern about the code itself.","commit_id":"ebc1d88c0efd39d39bbb2e5b55c0e2eaf339aec5"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"37fe79c5ee7c2f7960eb5cb154f04b5d7c29d9b6","unresolved":false,"context_lines":[{"line_number":702,"context_line":"        ),"},{"line_number":703,"context_line":"    )"},{"line_number":704,"context_line":"    sel \u003d sel.group_by(rp_tbl.c.id)"},{"line_number":705,"context_line":"    return set(r[0] for r in ctx.session.execute(sel))"},{"line_number":706,"context_line":""},{"line_number":707,"context_line":""},{"line_number":708,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":3,"id":"5ff73747_97ba1cdf","line":705,"range":{"start_line":705,"start_character":11,"end_line":705,"end_character":14},"in_reply_to":"5ff73747_ac8cab7f","updated":"2017-05-02 14:06:40.000000000","message":"It should be a list, not set, sorry was just a copy/paste mistake.","commit_id":"ebc1d88c0efd39d39bbb2e5b55c0e2eaf339aec5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ba44021c2b827e057160dba04a2032652dbc07b2","unresolved":false,"context_lines":[{"line_number":587,"context_line":""},{"line_number":588,"context_line":""},{"line_number":589,"context_line":"@db_api.api_context_manager.reader"},{"line_number":590,"context_line":"def _get_providers_with_shared_capacity(ctx, rc_id, amount):"},{"line_number":591,"context_line":"    \"\"\"Returns a list of resource provider IDs (internal IDs, not UUIDs)"},{"line_number":592,"context_line":"    that have capacity for a requested amount of a resource and indicate that"},{"line_number":593,"context_line":"    they share resource via an aggregate association."}],"source_content_type":"text/x-python","patch_set":6,"id":"3f044301_54d454bd","line":590,"updated":"2017-05-05 14:55:46.000000000","message":"I have questions on how you would integrate that call with the existing SQL query done below L807.\n\nWhile I agree with the below SQL-fu, it just provides RPs having shared capacity while I think we could be just integrating traits into the existing SQL query without needing to duplicate the SQL-fu.","commit_id":"6d5b39d80bb98ebb3733aa59271a089f12482dd8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5aff24a1fbdd1f48beb51e54b762a21d77aeabdd","unresolved":false,"context_lines":[{"line_number":587,"context_line":""},{"line_number":588,"context_line":""},{"line_number":589,"context_line":"@db_api.api_context_manager.reader"},{"line_number":590,"context_line":"def _get_providers_with_shared_capacity(ctx, rc_id, amount):"},{"line_number":591,"context_line":"    \"\"\"Returns a list of resource provider IDs (internal IDs, not UUIDs)"},{"line_number":592,"context_line":"    that have capacity for a requested amount of a resource and indicate that"},{"line_number":593,"context_line":"    they share resource via an aggregate association."}],"source_content_type":"text/x-python","patch_set":6,"id":"3f044301_f777e239","line":590,"in_reply_to":"3f044301_54d454bd","updated":"2017-05-05 15:21:07.000000000","message":"Just as a follow-up after seeing older comments, I still think we need to make sure that it integrates correctly, because for example, say someone would ask for two RCs (say the \u0027resources\u0027 dict having 2 RC keys), then we would need to call twice that private helper method and we would need to make sure to only return RPs that have *both* RCs in their inventories.\n\nIn the existing DB query, we instead directly verify that in a single SQL query using an HAVING count(RC.id) \u003d\u003d len(resources.keys()) and then only executing once.","commit_id":"6d5b39d80bb98ebb3733aa59271a089f12482dd8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5aff24a1fbdd1f48beb51e54b762a21d77aeabdd","unresolved":false,"context_lines":[{"line_number":702,"context_line":"        ),"},{"line_number":703,"context_line":"    )"},{"line_number":704,"context_line":"    sel \u003d sel.group_by(rp_tbl.c.id)"},{"line_number":705,"context_line":"    return [r[0] for r in ctx.session.execute(sel)]"},{"line_number":706,"context_line":""},{"line_number":707,"context_line":""},{"line_number":708,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f044301_b7b83ac8","line":705,"range":{"start_line":705,"start_character":30,"end_line":705,"end_character":51},"updated":"2017-05-05 15:21:07.000000000","message":"see, the problem is that we execute the SQL statement here, preventing us to refine the SQL query with other verifications before calling the DB.","commit_id":"6d5b39d80bb98ebb3733aa59271a089f12482dd8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5aff24a1fbdd1f48beb51e54b762a21d77aeabdd","unresolved":false,"context_lines":[{"line_number":839,"context_line":"            for (r_idx, amount) in resources.items()]"},{"line_number":840,"context_line":"        query \u003d query.filter(sa.or_(*where_clauses))"},{"line_number":841,"context_line":"        query \u003d query.group_by(_RP_TBL.c.id)"},{"line_number":842,"context_line":"        # NOTE(sbauza): Only RPs having all the asked resources can be provided"},{"line_number":843,"context_line":"        query \u003d query.having(sql.func.count("},{"line_number":844,"context_line":"            sa.distinct(_INV_TBL.c.resource_class_id)) \u003d\u003d len(resources))"},{"line_number":845,"context_line":""},{"line_number":846,"context_line":"        return query.all()"},{"line_number":847,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3f044301_77007268","line":844,"range":{"start_line":842,"start_character":0,"end_line":844,"end_character":73},"updated":"2017-05-05 15:21:07.000000000","message":"that\u0027s what I\u0027m talking about above.\nIf we execute the SQL statement when calling the private method, it prevents us to only return RPs having *both RCs, and we rather need to call that method for every RC and then trying to python-check it in memory.","commit_id":"6d5b39d80bb98ebb3733aa59271a089f12482dd8"}],"nova/tests/functional/db/test_resource_provider.py":[{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"df8f971722c7f6b0165518f9210955363dbbff41","unresolved":false,"context_lines":[{"line_number":1749,"context_line":"    \"\"\""},{"line_number":1750,"context_line":""},{"line_number":1751,"context_line":"    def test_shared_provider_capacity(self):"},{"line_number":1752,"context_line":"        \"\"\"We set up two compute nodes with VCPU and MEMORY_MB only, a shared"},{"line_number":1753,"context_line":"        resource provider having DISK_GB inventory, and associate all of them"},{"line_number":1754,"context_line":"        with an aggregate. We then call the"},{"line_number":1755,"context_line":"        _get_providers_with_shared_capacity() function to ensure that we return"}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_88f7df8b","line":1752,"range":{"start_line":1752,"start_character":21,"end_line":1752,"end_character":38},"updated":"2017-04-28 13:12:35.000000000","message":"It would be helpful to also set up some nodes that are similar to these, but a) are in another aggregate, or b) are in an aggregate whose shared resource is not sufficient, to demonstrate that they are not matched.","commit_id":"0fdcd25b5ed96121a119e46b518c9bbb86eb2d5b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b4ea852b775b378fbde27d9de4db90cfe5ff4124","unresolved":false,"context_lines":[{"line_number":1749,"context_line":"    \"\"\""},{"line_number":1750,"context_line":""},{"line_number":1751,"context_line":"    def test_shared_provider_capacity(self):"},{"line_number":1752,"context_line":"        \"\"\"We set up two compute nodes with VCPU and MEMORY_MB only, a shared"},{"line_number":1753,"context_line":"        resource provider having DISK_GB inventory, and associate all of them"},{"line_number":1754,"context_line":"        with an aggregate. We then call the"},{"line_number":1755,"context_line":"        _get_providers_with_shared_capacity() function to ensure that we return"}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_2ed8ce78","line":1752,"range":{"start_line":1752,"start_character":21,"end_line":1752,"end_character":38},"in_reply_to":"5ff73747_88f7df8b","updated":"2017-05-01 14:26:01.000000000","message":"Done","commit_id":"0fdcd25b5ed96121a119e46b518c9bbb86eb2d5b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"bd0b1a3cf3e810a1184725c35164d8b04c452bf0","unresolved":false,"context_lines":[{"line_number":1749,"context_line":"    \"\"\""},{"line_number":1750,"context_line":""},{"line_number":1751,"context_line":"    def test_shared_provider_capacity(self):"},{"line_number":1752,"context_line":"        \"\"\"We set up two compute nodes with VCPU and MEMORY_MB only, a shared"},{"line_number":1753,"context_line":"        resource provider having DISK_GB inventory, and associate all of them"},{"line_number":1754,"context_line":"        with an aggregate. We then call the"},{"line_number":1755,"context_line":"        _get_providers_with_shared_capacity() function to ensure that we return"}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_8bf7d98b","line":1752,"range":{"start_line":1752,"start_character":21,"end_line":1752,"end_character":38},"in_reply_to":"5ff73747_88f7df8b","updated":"2017-04-28 13:36:20.000000000","message":"excellent suggestions. will do.","commit_id":"0fdcd25b5ed96121a119e46b518c9bbb86eb2d5b"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"08b489acf56c5c8137c116da654e8f71944ae920","unresolved":false,"context_lines":[{"line_number":1843,"context_line":"        # same aggregate"},{"line_number":1844,"context_line":"        cn1.set_aggregates([agg_uuid])"},{"line_number":1845,"context_line":"        cn2.set_aggregates([agg_uuid])"},{"line_number":1846,"context_line":"        ss.set_aggregates([agg_uuid])"},{"line_number":1847,"context_line":""},{"line_number":1848,"context_line":"        # OK, now that has all been set up, let\u0027s verify that we get the ID of"},{"line_number":1849,"context_line":"        # the shared storage pool when we ask for 10 DISK_GB"}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_88223fb8","line":1846,"updated":"2017-04-28 13:18:53.000000000","message":"this association is not actually used?","commit_id":"0fdcd25b5ed96121a119e46b518c9bbb86eb2d5b"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"21cc05b53d21e753173c6d00fb83ba4f63a5e6db","unresolved":false,"context_lines":[{"line_number":1751,"context_line":"    def test_shared_provider_capacity(self):"},{"line_number":1752,"context_line":"        \"\"\"We set up two compute nodes with VCPU and MEMORY_MB only, a shared"},{"line_number":1753,"context_line":"        resource provider having DISK_GB inventory, and associate all of them"},{"line_number":1754,"context_line":"        with an aggregate. We then call the"},{"line_number":1755,"context_line":"        _get_providers_with_shared_capacity() function to ensure that we return"},{"line_number":1756,"context_line":"        the resource provider ID of the shared storage pool."},{"line_number":1757,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"5ff73747_55deeb3d","line":1754,"updated":"2017-05-01 16:40:49.000000000","message":"Since their association in an aggregate is never actually used this test is doing extra work to test nothing. Either the code being test needs to start doing something aggregate info, or this test can stop doing the aggregate associating?\n\nOr is there some hidden magic here that I\u0027m missing because I\u0027m blind from too much reading?","commit_id":"1fc560d863e672f3ecdf283c5154e551f97c871c"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1539f4fa113d40e50f1367d47169ef6e874ae0aa","unresolved":false,"context_lines":[{"line_number":1751,"context_line":"    def test_shared_provider_capacity(self):"},{"line_number":1752,"context_line":"        \"\"\"We set up two compute nodes with VCPU and MEMORY_MB only, a shared"},{"line_number":1753,"context_line":"        resource provider having DISK_GB inventory, and associate all of them"},{"line_number":1754,"context_line":"        with an aggregate. We then call the"},{"line_number":1755,"context_line":"        _get_providers_with_shared_capacity() function to ensure that we return"},{"line_number":1756,"context_line":"        the resource provider ID of the shared storage pool."},{"line_number":1757,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"5ff73747_d5279bc6","line":1754,"in_reply_to":"5ff73747_55deeb3d","updated":"2017-05-01 17:00:13.000000000","message":"ok, I\u0027ll remove the extra work and add it back in the next patch...","commit_id":"1fc560d863e672f3ecdf283c5154e551f97c871c"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"21cc05b53d21e753173c6d00fb83ba4f63a5e6db","unresolved":false,"context_lines":[{"line_number":1827,"context_line":"        # provider associated via aggregate"},{"line_number":1828,"context_line":"        t \u003d objects.Trait("},{"line_number":1829,"context_line":"            self.context,"},{"line_number":1830,"context_line":"            name\u003d\"MISC_SHARES_VIA_AGGREGATE\","},{"line_number":1831,"context_line":"        )"},{"line_number":1832,"context_line":"        # TODO(jaypipes): Once MISC_SHARES_VIA_AGGREGATE is a standard"},{"line_number":1833,"context_line":"        # os-traits trait, we won\u0027t need to create() here. Instead, we will"}],"source_content_type":"text/x-python","patch_set":2,"id":"5ff73747_958fd328","line":1830,"updated":"2017-05-01 16:40:49.000000000","message":"The db is set up to want unicode for trait name, so produces a warning on this, if we care.","commit_id":"1fc560d863e672f3ecdf283c5154e551f97c871c"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1539f4fa113d40e50f1367d47169ef6e874ae0aa","unresolved":false,"context_lines":[{"line_number":1827,"context_line":"        # provider associated via aggregate"},{"line_number":1828,"context_line":"        t \u003d objects.Trait("},{"line_number":1829,"context_line":"            self.context,"},{"line_number":1830,"context_line":"            name\u003d\"MISC_SHARES_VIA_AGGREGATE\","},{"line_number":1831,"context_line":"        )"},{"line_number":1832,"context_line":"        # TODO(jaypipes): Once MISC_SHARES_VIA_AGGREGATE is a standard"},{"line_number":1833,"context_line":"        # os-traits trait, we won\u0027t need to create() here. Instead, we will"}],"source_content_type":"text/x-python","patch_set":2,"id":"5ff73747_75120764","line":1830,"in_reply_to":"5ff73747_958fd328","updated":"2017-05-01 17:00:13.000000000","message":"Done","commit_id":"1fc560d863e672f3ecdf283c5154e551f97c871c"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"21cc05b53d21e753173c6d00fb83ba4f63a5e6db","unresolved":false,"context_lines":[{"line_number":1843,"context_line":"        # same aggregate"},{"line_number":1844,"context_line":"        cn1.set_aggregates([agg_uuid])"},{"line_number":1845,"context_line":"        cn2.set_aggregates([agg_uuid])"},{"line_number":1846,"context_line":"        ss.set_aggregates([agg_uuid])"},{"line_number":1847,"context_line":""},{"line_number":1848,"context_line":"        # OK, now that has all been set up, let\u0027s verify that we get the ID of"},{"line_number":1849,"context_line":"        # the shared storage pool when we ask for 10 DISK_GB"}],"source_content_type":"text/x-python","patch_set":2,"id":"5ff73747_1592a357","line":1846,"updated":"2017-05-01 16:40:49.000000000","message":"If we comment out these three lines the behavior of the test is exactly the same.","commit_id":"1fc560d863e672f3ecdf283c5154e551f97c871c"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1539f4fa113d40e50f1367d47169ef6e874ae0aa","unresolved":false,"context_lines":[{"line_number":1843,"context_line":"        # same aggregate"},{"line_number":1844,"context_line":"        cn1.set_aggregates([agg_uuid])"},{"line_number":1845,"context_line":"        cn2.set_aggregates([agg_uuid])"},{"line_number":1846,"context_line":"        ss.set_aggregates([agg_uuid])"},{"line_number":1847,"context_line":""},{"line_number":1848,"context_line":"        # OK, now that has all been set up, let\u0027s verify that we get the ID of"},{"line_number":1849,"context_line":"        # the shared storage pool when we ask for 10 DISK_GB"}],"source_content_type":"text/x-python","patch_set":2,"id":"5ff73747_f505f714","line":1846,"in_reply_to":"5ff73747_1592a357","updated":"2017-05-01 17:00:13.000000000","message":"I\u0027ll take it out of this patch then and add it back in the next patch.","commit_id":"1fc560d863e672f3ecdf283c5154e551f97c871c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"855342af5d523d5afb7c3c9355b5e81e534c8b72","unresolved":false,"context_lines":[{"line_number":1771,"context_line":"        )"},{"line_number":1772,"context_line":"        cn2.create()"},{"line_number":1773,"context_line":""},{"line_number":1774,"context_line":"        # Populate the two compute node providers with inventory, sans DISK_GB"},{"line_number":1775,"context_line":"        for cn in (cn1, cn2):"},{"line_number":1776,"context_line":"            disk_gb \u003d objects.Inventory("},{"line_number":1777,"context_line":"                resource_provider\u003dcn,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f044301_29e4f8ab","line":1774,"range":{"start_line":1774,"start_character":66,"end_line":1774,"end_character":70},"updated":"2017-05-04 14:25:08.000000000","message":"Sans DISK_GB? These are _only_ DISK_GB right?","commit_id":"502552b68ad0d3cfffc259319336f1764a8a9fe1"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"22526ce2fcd4e8e10fcf895bdec436f62d63481c","unresolved":false,"context_lines":[{"line_number":1771,"context_line":"        )"},{"line_number":1772,"context_line":"        cn2.create()"},{"line_number":1773,"context_line":""},{"line_number":1774,"context_line":"        # Populate the two compute node providers with inventory, sans DISK_GB"},{"line_number":1775,"context_line":"        for cn in (cn1, cn2):"},{"line_number":1776,"context_line":"            disk_gb \u003d objects.Inventory("},{"line_number":1777,"context_line":"                resource_provider\u003dcn,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f044301_8445311a","line":1774,"range":{"start_line":1774,"start_character":66,"end_line":1774,"end_character":70},"in_reply_to":"3f044301_29e4f8ab","updated":"2017-05-04 14:33:42.000000000","message":"Doh. Copy/paste mistake when I rebased this :( Fixing up now...","commit_id":"502552b68ad0d3cfffc259319336f1764a8a9fe1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"855342af5d523d5afb7c3c9355b5e81e534c8b72","unresolved":false,"context_lines":[{"line_number":1784,"context_line":"                allocation_ratio\u003d1.0,"},{"line_number":1785,"context_line":"            )"},{"line_number":1786,"context_line":"            inv_list \u003d objects.InventoryList(objects\u003d[disk_gb])"},{"line_number":1787,"context_line":"            cn.set_inventory(inv_list)"},{"line_number":1788,"context_line":""},{"line_number":1789,"context_line":"        # Create the shared storage pool"},{"line_number":1790,"context_line":"        ss_uuid \u003d uuidsentinel.ss"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f044301_29abd81d","line":1787,"updated":"2017-05-04 14:25:08.000000000","message":"AFAICT, you\u0027ve just created CN providers with _only_ DISK_GB, 500GB of it to be exact.","commit_id":"502552b68ad0d3cfffc259319336f1764a8a9fe1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"855342af5d523d5afb7c3c9355b5e81e534c8b72","unresolved":false,"context_lines":[{"line_number":1827,"context_line":"        ss.set_traits(objects.TraitList(objects\u003d[t]))"},{"line_number":1828,"context_line":""},{"line_number":1829,"context_line":"        # OK, now that has all been set up, let\u0027s verify that we get the ID of"},{"line_number":1830,"context_line":"        # the shared storage pool when we ask for 10 DISK_GB"},{"line_number":1831,"context_line":"        got_ids \u003d rp_obj._get_providers_with_shared_capacity("},{"line_number":1832,"context_line":"            self.context,"},{"line_number":1833,"context_line":"            fields.ResourceClass.STANDARD.index(fields.ResourceClass.DISK_GB),"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f044301_e94bb084","line":1830,"range":{"start_line":1830,"start_character":50,"end_line":1830,"end_character":52},"updated":"2017-05-04 14:25:08.000000000","message":"You\u0027re asking for 100 not 10 right?","commit_id":"502552b68ad0d3cfffc259319336f1764a8a9fe1"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"90db709ea5dad432ca65f6a3bf295bdb918f8a3c","unresolved":false,"context_lines":[{"line_number":1827,"context_line":"        ss.set_traits(objects.TraitList(objects\u003d[t]))"},{"line_number":1828,"context_line":""},{"line_number":1829,"context_line":"        # OK, now that has all been set up, let\u0027s verify that we get the ID of"},{"line_number":1830,"context_line":"        # the shared storage pool when we ask for 10 DISK_GB"},{"line_number":1831,"context_line":"        got_ids \u003d rp_obj._get_providers_with_shared_capacity("},{"line_number":1832,"context_line":"            self.context,"},{"line_number":1833,"context_line":"            fields.ResourceClass.STANDARD.index(fields.ResourceClass.DISK_GB),"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f044301_84af7193","line":1830,"range":{"start_line":1830,"start_character":50,"end_line":1830,"end_character":52},"in_reply_to":"3f044301_e94bb084","updated":"2017-05-04 14:38:33.000000000","message":"Done","commit_id":"502552b68ad0d3cfffc259319336f1764a8a9fe1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"855342af5d523d5afb7c3c9355b5e81e534c8b72","unresolved":false,"context_lines":[{"line_number":1833,"context_line":"            fields.ResourceClass.STANDARD.index(fields.ResourceClass.DISK_GB),"},{"line_number":1834,"context_line":"            100,"},{"line_number":1835,"context_line":"        )"},{"line_number":1836,"context_line":"        self.assertEqual([ss.id], got_ids)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f044301_695fa041","line":1836,"updated":"2017-05-04 14:25:08.000000000","message":"So, this worked despite having inventory for the CNs above. I\u0027m kinda confused. Shouldn\u0027t this have returned them all?","commit_id":"502552b68ad0d3cfffc259319336f1764a8a9fe1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8382492cb92ca84b61b1753c70a13f6dd501a5ab","unresolved":false,"context_lines":[{"line_number":1833,"context_line":"            fields.ResourceClass.STANDARD.index(fields.ResourceClass.DISK_GB),"},{"line_number":1834,"context_line":"            100,"},{"line_number":1835,"context_line":"        )"},{"line_number":1836,"context_line":"        self.assertEqual([ss.id], got_ids)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f044301_646905bd","line":1836,"in_reply_to":"3f044301_695fa041","updated":"2017-05-04 14:30:40.000000000","message":"Ah, I guess this worked because of the trait? Anyway, I think the rest is still valid (i.e. wrong).","commit_id":"502552b68ad0d3cfffc259319336f1764a8a9fe1"}]}