)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"281cc5e207c508164cb586b3c47f86e77a0533e3","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"[1]"},{"line_number":31,"context_line":"http://lists.openstack.org/pipermail/openstack-dev/2018-January/126283.html"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"Change-Id: Ib0cbb58d0adbbcfe83ee48d2ff6c9af1a516a7ae"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"3fce034c_e4a39e4e","line":32,"updated":"2019-04-12 13:16:06.000000000","message":"Looks like this would be related to or fix bug 1784932 (which we probably need to move to storyboard at this point).","commit_id":"35d3301b78084d57a61ed808f6b10d14e17cb422"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"acd02e004e9f8a2e091b167510ce622f7ea08a7a","unresolved":false,"context_lines":[{"line_number":25,"context_line":"[1]"},{"line_number":26,"context_line":"http://lists.openstack.org/pipermail/openstack-dev/2018-January/126283.html"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Change-Id: Ib0cbb58d0adbbcfe83ee48d2ff6c9af1a516a7ae"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"7faddb67_b419ccd7","line":28,"updated":"2019-08-01 02:50:32.000000000","message":"I\u0027ve opened a story for this. I\u0027ll update the commit message for this.\nhttps://storyboard.openstack.org/#!/story/2006318","commit_id":"0e9f46d8bf27f169676e6e20148dd6fbd365c5e3"}],"osc_placement/resources/aggregate.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"dc0c60824184d2c2e202b8682899fa4dee7b90c2","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        return parser"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    @version.check(version.ge(\u00271.3\u0027))"},{"line_number":144,"context_line":"    def take_action(self, parsed_args):"},{"line_number":145,"context_line":"        http \u003d self.app.client_manager.placement"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"        filters \u003d {\u0027member_of\u0027: parsed_args.uuid}"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_628edc28","line":144,"updated":"2019-03-05 23:14:42.000000000","message":"I had originally meant to add input validation (for the --resource \u003cclass\u003e\u003d\u003cratio\u003e format here. Will do it if I need to respin.","commit_id":"0387e9ab9a47ff969256f32004f5953ddf029004"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"95d36b874bd4881d516feaa4ad9498a94b1e411c","unresolved":false,"context_lines":[{"line_number":155,"context_line":"            inventories \u003d payload[\u0027inventories\u0027]"},{"line_number":156,"context_line":"            for resource in parsed_args.resource:"},{"line_number":157,"context_line":"                resource_cls, ratio \u003d resource.split(\u0027\u003d\u0027)"},{"line_number":158,"context_line":"                inventories[resource_cls].update(allocation_ratio\u003dfloat(ratio))"},{"line_number":159,"context_line":"            all_payloads.append((rp[\u0027name\u0027], rp[\u0027uuid\u0027],"},{"line_number":160,"context_line":"                                 json.dumps(payload, indent\u003d2,"},{"line_number":161,"context_line":"                                            separators\u003d(\u0027,\u0027, \u0027: \u0027))))"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_0dff1168","line":158,"updated":"2019-03-05 17:54:19.000000000","message":"smooooooth","commit_id":"0387e9ab9a47ff969256f32004f5953ddf029004"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"993014f8ea5a343da5a4799a73daaf3d15fbc547","unresolved":false,"context_lines":[{"line_number":158,"context_line":"                inventories[resource_cls].update(allocation_ratio\u003dfloat(ratio))"},{"line_number":159,"context_line":"            all_payloads.append((rp[\u0027name\u0027], rp[\u0027uuid\u0027],"},{"line_number":160,"context_line":"                                 json.dumps(payload, indent\u003d2,"},{"line_number":161,"context_line":"                                            separators\u003d(\u0027,\u0027, \u0027: \u0027))))"},{"line_number":162,"context_line":"            if not parsed_args.dry_run:"},{"line_number":163,"context_line":"                http.request(\u0027PUT\u0027, url, json\u003dpayload)"},{"line_number":164,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_0d087126","line":161,"updated":"2019-03-05 17:20:37.000000000","message":"I could add a code comment here that this is for pretty-printing the payload preview when --dry-run was passed.","commit_id":"0387e9ab9a47ff969256f32004f5953ddf029004"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"53280349aa63781a5c86b0011baab648ab38e864","unresolved":false,"context_lines":[{"line_number":158,"context_line":"                inventories[resource_cls].update(allocation_ratio\u003dfloat(ratio))"},{"line_number":159,"context_line":"            all_payloads.append((rp[\u0027name\u0027], rp[\u0027uuid\u0027],"},{"line_number":160,"context_line":"                                 json.dumps(payload, indent\u003d2,"},{"line_number":161,"context_line":"                                            separators\u003d(\u0027,\u0027, \u0027: \u0027))))"},{"line_number":162,"context_line":"            if not parsed_args.dry_run:"},{"line_number":163,"context_line":"                http.request(\u0027PUT\u0027, url, json\u003dpayload)"},{"line_number":164,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_6b745936","line":161,"in_reply_to":"5fc1f717_0d087126","updated":"2019-03-07 00:34:31.000000000","message":"i can see why you did it this way.\ni proably would have only contructed the all_payloads list if\n--dry-run was passed. that said there is not a lot of overhead to this.","commit_id":"0387e9ab9a47ff969256f32004f5953ddf029004"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0a8721368a046b82b649d63fa3faf91140073c7c","unresolved":false,"context_lines":[{"line_number":158,"context_line":"                inventories[resource_cls].update(allocation_ratio\u003dfloat(ratio))"},{"line_number":159,"context_line":"            all_payloads.append((rp[\u0027name\u0027], rp[\u0027uuid\u0027],"},{"line_number":160,"context_line":"                                 json.dumps(payload, indent\u003d2,"},{"line_number":161,"context_line":"                                            separators\u003d(\u0027,\u0027, \u0027: \u0027))))"},{"line_number":162,"context_line":"            if not parsed_args.dry_run:"},{"line_number":163,"context_line":"                http.request(\u0027PUT\u0027, url, json\u003dpayload)"},{"line_number":164,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_abd7a1bf","line":161,"in_reply_to":"5fc1f717_6b745936","updated":"2019-03-07 00:51:59.000000000","message":"Yeah, that\u0027s fair.","commit_id":"0387e9ab9a47ff969256f32004f5953ddf029004"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"95d36b874bd4881d516feaa4ad9498a94b1e411c","unresolved":false,"context_lines":[{"line_number":165,"context_line":"        # Sort table rows by resource provider name"},{"line_number":166,"context_line":"        all_payloads.sort(key\u003ditemgetter(0))"},{"line_number":167,"context_line":"        if not parsed_args.dry_run:"},{"line_number":168,"context_line":"            return ([\u0027Resource Provider Inventories Updated\u0027],"},{"line_number":169,"context_line":"                    [[len(all_payloads)]])"},{"line_number":170,"context_line":"        return [\u0027Resource Provider Name\u0027, \u0027Resource Provider UUID\u0027,"},{"line_number":171,"context_line":"                \u0027Payload Preview\u0027], all_payloads"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_ad2a7d8d","line":168,"updated":"2019-03-05 17:54:19.000000000","message":"I\u0027m not sure about this header. It\u0027s counting payloads which has one to many individual \u0027inventory\u0027 in it, but is one \u0027inventories\u0027 per payload.\n\nIt\u0027s probably okay but wanted to flag it in case somebody else had a better idea.","commit_id":"0387e9ab9a47ff969256f32004f5953ddf029004"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"15225ea59e5a85bce975e10d1dc8c5f4cde901e5","unresolved":false,"context_lines":[{"line_number":165,"context_line":"        # Sort table rows by resource provider name"},{"line_number":166,"context_line":"        all_payloads.sort(key\u003ditemgetter(0))"},{"line_number":167,"context_line":"        if not parsed_args.dry_run:"},{"line_number":168,"context_line":"            return ([\u0027Resource Provider Inventories Updated\u0027],"},{"line_number":169,"context_line":"                    [[len(all_payloads)]])"},{"line_number":170,"context_line":"        return [\u0027Resource Provider Name\u0027, \u0027Resource Provider UUID\u0027,"},{"line_number":171,"context_line":"                \u0027Payload Preview\u0027], all_payloads"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_100e4ae5","line":168,"in_reply_to":"5fc1f717_ad2a7d8d","updated":"2019-03-05 18:04:59.000000000","message":"Oh, yeah, good point. I think what I was really going for was, \"Number of Resource Providers Updated\", but technically they were not updated, their inventories were. So I ended up with this clunky header.\n\nI\u0027m open to a better idea.","commit_id":"0387e9ab9a47ff969256f32004f5953ddf029004"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"53280349aa63781a5c86b0011baab648ab38e864","unresolved":false,"context_lines":[{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        # Sort table rows by resource provider name"},{"line_number":166,"context_line":"        all_payloads.sort(key\u003ditemgetter(0))"},{"line_number":167,"context_line":"        if not parsed_args.dry_run:"},{"line_number":168,"context_line":"            return ([\u0027Resource Provider Inventories Updated\u0027],"},{"line_number":169,"context_line":"                    [[len(all_payloads)]])"},{"line_number":170,"context_line":"        return [\u0027Resource Provider Name\u0027, \u0027Resource Provider UUID\u0027,"},{"line_number":171,"context_line":"                \u0027Payload Preview\u0027], all_payloads"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_2b941137","line":171,"range":{"start_line":167,"start_character":8,"end_line":171,"end_character":48},"updated":"2019-03-07 00:34:31.000000000","message":"why not always print the payload since you have built it up already.","commit_id":"0387e9ab9a47ff969256f32004f5953ddf029004"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0a8721368a046b82b649d63fa3faf91140073c7c","unresolved":false,"context_lines":[{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        # Sort table rows by resource provider name"},{"line_number":166,"context_line":"        all_payloads.sort(key\u003ditemgetter(0))"},{"line_number":167,"context_line":"        if not parsed_args.dry_run:"},{"line_number":168,"context_line":"            return ([\u0027Resource Provider Inventories Updated\u0027],"},{"line_number":169,"context_line":"                    [[len(all_payloads)]])"},{"line_number":170,"context_line":"        return [\u0027Resource Provider Name\u0027, \u0027Resource Provider UUID\u0027,"},{"line_number":171,"context_line":"                \u0027Payload Preview\u0027], all_payloads"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_ebee0987","line":171,"range":{"start_line":167,"start_character":8,"end_line":171,"end_character":48},"in_reply_to":"5fc1f717_2b941137","updated":"2019-03-07 00:51:59.000000000","message":"I was thinking a giant dump of the info might be a weird thing to do sans --dry-run and instead show a short summary. I\u0027m open to the idea of changing it though. Maybe I could add a --verbose option to control that.","commit_id":"0387e9ab9a47ff969256f32004f5953ddf029004"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"15b7967d830d4debf47cceb2eec14d2e39b6d480","unresolved":false,"context_lines":[{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        # Sort table rows by resource provider name"},{"line_number":166,"context_line":"        all_payloads.sort(key\u003ditemgetter(0))"},{"line_number":167,"context_line":"        if not parsed_args.dry_run:"},{"line_number":168,"context_line":"            return ([\u0027Resource Provider Inventories Updated\u0027],"},{"line_number":169,"context_line":"                    [[len(all_payloads)]])"},{"line_number":170,"context_line":"        return [\u0027Resource Provider Name\u0027, \u0027Resource Provider UUID\u0027,"},{"line_number":171,"context_line":"                \u0027Payload Preview\u0027], all_payloads"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_c2f9a680","line":171,"range":{"start_line":167,"start_character":8,"end_line":171,"end_character":48},"in_reply_to":"5fc1f717_ebee0987","updated":"2019-03-07 12:51:25.000000000","message":"ya that could work. it just seamed a waste to throw it away","commit_id":"0387e9ab9a47ff969256f32004f5953ddf029004"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"81809d76c1b9ce28481733aea8c5394e5f50490c","unresolved":false,"context_lines":[{"line_number":184,"context_line":"            num_payloads +\u003d 1"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"        # Sort table rows by resource provider name"},{"line_number":187,"context_line":"        if parsed_args.dry_run or parsed_args.show_payloads:"},{"line_number":188,"context_line":"            all_payloads.sort(key\u003ditemgetter(0))"},{"line_number":189,"context_line":"        if not parsed_args.dry_run and not parsed_args.show_payloads:"},{"line_number":190,"context_line":"            # Only output the number of RPs whose inventories were updated"}],"source_content_type":"text/x-python","patch_set":5,"id":"5fc1f717_a4343894","line":187,"range":{"start_line":187,"start_character":11,"end_line":187,"end_character":59},"updated":"2019-03-13 14:38:39.000000000","message":"nit: this could be made DRY (lines 177, 187, and 189) with something like:\n\n  payload_output \u003d parsed_args.dry_run or parsed_args.show_payloads","commit_id":"c350cd19156be0f2207ba5c73c5faa4517da2505"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"81809d76c1b9ce28481733aea8c5394e5f50490c","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        # Sort table rows by resource provider name"},{"line_number":187,"context_line":"        if parsed_args.dry_run or parsed_args.show_payloads:"},{"line_number":188,"context_line":"            all_payloads.sort(key\u003ditemgetter(0))"},{"line_number":189,"context_line":"        if not parsed_args.dry_run and not parsed_args.show_payloads:"},{"line_number":190,"context_line":"            # Only output the number of RPs whose inventories were updated"},{"line_number":191,"context_line":"            return ([\u0027Resource Providers Updated\u0027], [[num_payloads]])"},{"line_number":192,"context_line":"        return [\u0027Resource Provider Name\u0027, \u0027Resource Provider UUID\u0027,"}],"source_content_type":"text/x-python","patch_set":5,"id":"5fc1f717_44da344b","line":189,"range":{"start_line":189,"start_character":11,"end_line":189,"end_character":68},"updated":"2019-03-13 14:38:39.000000000","message":"This would shorten to:\n\n  if not payload_output:","commit_id":"c350cd19156be0f2207ba5c73c5faa4517da2505"}],"osc_placement/resources/inventory.py":[{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"fe9c4009b4208bf574e653214d12113841909e9c","unresolved":false,"context_lines":[{"line_number":120,"context_line":"        parser.add_argument("},{"line_number":121,"context_line":"            \u0027uuid\u0027,"},{"line_number":122,"context_line":"            metavar\u003d\u0027\u003cuuid\u003e\u0027,"},{"line_number":123,"context_line":"            help\u003d\u0027UUID of the resource provider\u0027"},{"line_number":124,"context_line":"        )"},{"line_number":125,"context_line":"        fields_help \u003d \u0027\\n\u0027.join("},{"line_number":126,"context_line":"            \u0027{} - {}\u0027.format(f, INVENTORY_FIELDS[f][\u0027help\u0027].lower())"}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_ad743369","line":123,"range":{"start_line":123,"start_character":18,"end_line":123,"end_character":47},"updated":"2019-07-31 06:00:27.000000000","message":"or UUID of the aggregate to which the resource providers belong","commit_id":"b182a6b8e886a35b1fd641156a3a76014eaa398e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5f3a5f4034b5b3557cf34b6ab5cc17e8b5d1b681","unresolved":false,"context_lines":[{"line_number":120,"context_line":"        parser.add_argument("},{"line_number":121,"context_line":"            \u0027uuid\u0027,"},{"line_number":122,"context_line":"            metavar\u003d\u0027\u003cuuid\u003e\u0027,"},{"line_number":123,"context_line":"            help\u003d\u0027UUID of the resource provider\u0027"},{"line_number":124,"context_line":"        )"},{"line_number":125,"context_line":"        fields_help \u003d \u0027\\n\u0027.join("},{"line_number":126,"context_line":"            \u0027{} - {}\u0027.format(f, INVENTORY_FIELDS[f][\u0027help\u0027].lower())"}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_9a53a439","line":123,"range":{"start_line":123,"start_character":18,"end_line":123,"end_character":47},"in_reply_to":"7faddb67_ad743369","updated":"2019-07-31 16:19:49.000000000","message":"Ah yup, thanks, will add.","commit_id":"b182a6b8e886a35b1fd641156a3a76014eaa398e"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"fe9c4009b4208bf574e653214d12113841909e9c","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            help\u003d\u0027If this option is specified, the inventories for all \u0027"},{"line_number":146,"context_line":"                 \u0027resource providers that are members of the aggregate will \u0027"},{"line_number":147,"context_line":"                 \u0027be amended. This option requires at least \u0027"},{"line_number":148,"context_line":"                 \u0027``--os-placement-api-version 1.3`` and requires use of the \u0027"},{"line_number":149,"context_line":"                 \u0027--amend option\u0027"},{"line_number":150,"context_line":"        )"},{"line_number":151,"context_line":"        parser.add_argument("},{"line_number":152,"context_line":"            \u0027--dry-run\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_4d901f4b","line":149,"range":{"start_line":148,"start_character":52,"end_line":149,"end_character":32},"updated":"2019-07-31 06:00:27.000000000","message":"Does this mean this aggregate option can be used only when amend option is also specified? I don\u0027t think either there is a restriction in what is done here or this restriction is necessary.","commit_id":"b182a6b8e886a35b1fd641156a3a76014eaa398e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1a4fb108da69abe45d52ede532d85d39853534c1","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            help\u003d\u0027If this option is specified, the inventories for all \u0027"},{"line_number":146,"context_line":"                 \u0027resource providers that are members of the aggregate will \u0027"},{"line_number":147,"context_line":"                 \u0027be amended. This option requires at least \u0027"},{"line_number":148,"context_line":"                 \u0027``--os-placement-api-version 1.3`` and requires use of the \u0027"},{"line_number":149,"context_line":"                 \u0027--amend option\u0027"},{"line_number":150,"context_line":"        )"},{"line_number":151,"context_line":"        parser.add_argument("},{"line_number":152,"context_line":"            \u0027--dry-run\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_95713547","line":149,"range":{"start_line":148,"start_character":52,"end_line":149,"end_character":32},"in_reply_to":"7faddb67_4d901f4b","updated":"2019-07-31 16:44:31.000000000","message":"Yes, I had been thinking that there is a low chance that anyone would intend to do full replacement of all inventories in an aggregate and might result in users making a mistake they didn\u0027t want (wiping out some settings by accident over a batch). Maybe I\u0027m being paranoid.","commit_id":"b182a6b8e886a35b1fd641156a3a76014eaa398e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7380f4bfa45f30a436b95804d572357c38bb2bb0","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            help\u003d\u0027If this option is specified, the inventories for all \u0027"},{"line_number":146,"context_line":"                 \u0027resource providers that are members of the aggregate will \u0027"},{"line_number":147,"context_line":"                 \u0027be amended. This option requires at least \u0027"},{"line_number":148,"context_line":"                 \u0027``--os-placement-api-version 1.3`` and requires use of the \u0027"},{"line_number":149,"context_line":"                 \u0027--amend option\u0027"},{"line_number":150,"context_line":"        )"},{"line_number":151,"context_line":"        parser.add_argument("},{"line_number":152,"context_line":"            \u0027--dry-run\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_35072185","line":149,"range":{"start_line":148,"start_character":52,"end_line":149,"end_character":32},"in_reply_to":"7faddb67_95713547","updated":"2019-07-31 16:47:50.000000000","message":"Looking again, I see now that I didn\u0027t even restrict it in the implementation! (like you said). I intended to but didn\u0027t. I\u0027m just going to remove this part of the help and let it stay unrestricted.","commit_id":"b182a6b8e886a35b1fd641156a3a76014eaa398e"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"fe9c4009b4208bf574e653214d12113841909e9c","unresolved":false,"context_lines":[{"line_number":151,"context_line":"        parser.add_argument("},{"line_number":152,"context_line":"            \u0027--dry-run\u0027,"},{"line_number":153,"context_line":"            action\u003d\u0027store_true\u0027,"},{"line_number":154,"context_line":"            help\u003d\u0027If this option is specified, the inventories that would be \u0027"},{"line_number":155,"context_line":"                 \u0027be updated will be returned without actually updating any \u0027"},{"line_number":156,"context_line":"                 \u0027inventories\u0027"},{"line_number":157,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_cd716f78","line":154,"range":{"start_line":154,"start_character":74,"end_line":154,"end_character":76},"updated":"2019-07-31 06:00:27.000000000","message":"nix! (\"be\" is duplicated here)","commit_id":"b182a6b8e886a35b1fd641156a3a76014eaa398e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e4a3a5b91896701903245fb4117d96cba73350f2","unresolved":false,"context_lines":[{"line_number":152,"context_line":"            \u0027--dry-run\u0027,"},{"line_number":153,"context_line":"            action\u003d\u0027store_true\u0027,"},{"line_number":154,"context_line":"            help\u003d\u0027If this option is specified, the inventories that would be \u0027"},{"line_number":155,"context_line":"                 \u0027be updated will be returned without actually updating any \u0027"},{"line_number":156,"context_line":"                 \u0027inventories\u0027"},{"line_number":157,"context_line":"        )"},{"line_number":158,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_56eaf9d7","line":155,"range":{"start_line":155,"start_character":21,"end_line":155,"end_character":28},"updated":"2019-07-30 19:01:08.000000000","message":"set","commit_id":"b182a6b8e886a35b1fd641156a3a76014eaa398e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e4a3a5b91896701903245fb4117d96cba73350f2","unresolved":false,"context_lines":[{"line_number":152,"context_line":"            \u0027--dry-run\u0027,"},{"line_number":153,"context_line":"            action\u003d\u0027store_true\u0027,"},{"line_number":154,"context_line":"            help\u003d\u0027If this option is specified, the inventories that would be \u0027"},{"line_number":155,"context_line":"                 \u0027be updated will be returned without actually updating any \u0027"},{"line_number":156,"context_line":"                 \u0027inventories\u0027"},{"line_number":157,"context_line":"        )"},{"line_number":158,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_16c2415a","line":155,"range":{"start_line":155,"start_character":63,"end_line":155,"end_character":71},"updated":"2019-07-30 19:01:08.000000000","message":"setting","commit_id":"b182a6b8e886a35b1fd641156a3a76014eaa398e"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"fe9c4009b4208bf574e653214d12113841909e9c","unresolved":false,"context_lines":[{"line_number":160,"context_line":""},{"line_number":161,"context_line":"    def take_action(self, parsed_args):"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        inventories \u003d defaultdict(dict)"},{"line_number":164,"context_line":"        http \u003d self.app.client_manager.placement"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"        if parsed_args.aggregate:"}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_8d537739","line":163,"range":{"start_line":163,"start_character":8,"end_line":163,"end_character":39},"updated":"2019-07-31 06:00:27.000000000","message":"This initialization should be done within the \"for rp in rps\" loop at L175.\n\nOtherwise, this would produce a bug when for example there are 2 providers  in the aggregate and the first rp has many inventories and the second has no inventory.\n\nSince the updated inventory is not initialized between the first rp and the second rp loop.\n\nCan we have a test case for this?","commit_id":"b182a6b8e886a35b1fd641156a3a76014eaa398e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5f3a5f4034b5b3557cf34b6ab5cc17e8b5d1b681","unresolved":false,"context_lines":[{"line_number":160,"context_line":""},{"line_number":161,"context_line":"    def take_action(self, parsed_args):"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        inventories \u003d defaultdict(dict)"},{"line_number":164,"context_line":"        http \u003d self.app.client_manager.placement"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"        if parsed_args.aggregate:"}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_9a288497","line":163,"range":{"start_line":163,"start_character":8,"end_line":163,"end_character":39},"in_reply_to":"7faddb67_8d537739","updated":"2019-07-31 16:19:49.000000000","message":"Yikes, thanks for catching that. Will add test coverage.","commit_id":"b182a6b8e886a35b1fd641156a3a76014eaa398e"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"fe9c4009b4208bf574e653214d12113841909e9c","unresolved":false,"context_lines":[{"line_number":166,"context_line":"        if parsed_args.aggregate:"},{"line_number":167,"context_line":"            filters \u003d {\u0027member_of\u0027: parsed_args.uuid}"},{"line_number":168,"context_line":"            url \u003d common.url_with_filters(RP_BASE_URL, filters)"},{"line_number":169,"context_line":"            rps \u003d http.request(\u0027GET\u0027, url).json()[\u0027resource_providers\u0027]"},{"line_number":170,"context_line":"        else:"},{"line_number":171,"context_line":"            url \u003d RP_BASE_URL + \u0027/\u0027 + parsed_args.uuid"},{"line_number":172,"context_line":"            rps \u003d [http.request(\u0027GET\u0027, url).json()]"}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_cda08f93","line":169,"updated":"2019-07-31 06:00:27.000000000","message":"Any chance to have an exception \"no rps found in that aggregate\"?\nNow it returns an empty list so the message is not kindful.\n\n  $ openstack resource provider inventory set \u003cbad_agg_uuid\u003e --resource VGPU\u003d16 --amend --aggregate --dry-run\n  list index out of range","commit_id":"b182a6b8e886a35b1fd641156a3a76014eaa398e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5f3a5f4034b5b3557cf34b6ab5cc17e8b5d1b681","unresolved":false,"context_lines":[{"line_number":166,"context_line":"        if parsed_args.aggregate:"},{"line_number":167,"context_line":"            filters \u003d {\u0027member_of\u0027: parsed_args.uuid}"},{"line_number":168,"context_line":"            url \u003d common.url_with_filters(RP_BASE_URL, filters)"},{"line_number":169,"context_line":"            rps \u003d http.request(\u0027GET\u0027, url).json()[\u0027resource_providers\u0027]"},{"line_number":170,"context_line":"        else:"},{"line_number":171,"context_line":"            url \u003d RP_BASE_URL + \u0027/\u0027 + parsed_args.uuid"},{"line_number":172,"context_line":"            rps \u003d [http.request(\u0027GET\u0027, url).json()]"}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_d5100d51","line":169,"in_reply_to":"7faddb67_cda08f93","updated":"2019-07-31 16:19:49.000000000","message":"Oh, good call. Will add that + test coverage for it.","commit_id":"b182a6b8e886a35b1fd641156a3a76014eaa398e"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"fe9c4009b4208bf574e653214d12113841909e9c","unresolved":false,"context_lines":[{"line_number":169,"context_line":"            rps \u003d http.request(\u0027GET\u0027, url).json()[\u0027resource_providers\u0027]"},{"line_number":170,"context_line":"        else:"},{"line_number":171,"context_line":"            url \u003d RP_BASE_URL + \u0027/\u0027 + parsed_args.uuid"},{"line_number":172,"context_line":"            rps \u003d [http.request(\u0027GET\u0027, url).json()]"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"        resources_list \u003d []"},{"line_number":175,"context_line":"        for rp in rps:"}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_4dbedfb4","line":172,"updated":"2019-07-31 06:00:27.000000000","message":"FWIW, this is kindful enough.\n\n  $ openstack resource provider inventory set 6cac0a04-19de-4755-b22c-c500f18b1bb0 --resource VGPU\u003d16 --dry-run\n  No resource provider with uuid 6cac0a04-19de-4755-b22c-c500f18b1bb0 found (HTTP 404)","commit_id":"b182a6b8e886a35b1fd641156a3a76014eaa398e"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"fe9c4009b4208bf574e653214d12113841909e9c","unresolved":false,"context_lines":[{"line_number":182,"context_line":"            else:"},{"line_number":183,"context_line":"                payload \u003d {\u0027inventories\u0027: inventories,"},{"line_number":184,"context_line":"                           \u0027resource_provider_generation\u0027: rp[\u0027generation\u0027]}"},{"line_number":185,"context_line":"            for r in parsed_args.resource:"},{"line_number":186,"context_line":"                name, field, value \u003d parse_resource_argument(r)"},{"line_number":187,"context_line":"                inventories[name][field] \u003d value"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"            if not parsed_args.dry_run:"},{"line_number":190,"context_line":"                resources \u003d http.request(\u0027PUT\u0027, url, json\u003dpayload).json()"}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_18290789","line":187,"range":{"start_line":185,"start_character":0,"end_line":187,"end_character":48},"updated":"2019-07-31 06:00:27.000000000","message":"nit: *personally* I would feel better if this block is moved before inventories assignment to payload i.e. before L177-184","commit_id":"b182a6b8e886a35b1fd641156a3a76014eaa398e"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"40a6202b51f4ef61f93ab7c239f38eeb6407a929","unresolved":false,"context_lines":[{"line_number":182,"context_line":"            else:"},{"line_number":183,"context_line":"                payload \u003d {\u0027inventories\u0027: inventories,"},{"line_number":184,"context_line":"                           \u0027resource_provider_generation\u0027: rp[\u0027generation\u0027]}"},{"line_number":185,"context_line":"            for r in parsed_args.resource:"},{"line_number":186,"context_line":"                name, field, value \u003d parse_resource_argument(r)"},{"line_number":187,"context_line":"                inventories[name][field] \u003d value"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"            if not parsed_args.dry_run:"},{"line_number":190,"context_line":"                resources \u003d http.request(\u0027PUT\u0027, url, json\u003dpayload).json()"}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_58745f36","line":187,"range":{"start_line":185,"start_character":0,"end_line":187,"end_character":48},"in_reply_to":"7faddb67_18290789","updated":"2019-07-31 07:04:59.000000000","message":"Oh, I was misunderstanding about this. please ignore this comment.","commit_id":"b182a6b8e886a35b1fd641156a3a76014eaa398e"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"fe9c4009b4208bf574e653214d12113841909e9c","unresolved":false,"context_lines":[{"line_number":204,"context_line":"            return rows"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"        fields \u003d (\u0027resource_class\u0027, ) + FIELDS"},{"line_number":207,"context_line":"        if len(resources_list) \u003e 1:"},{"line_number":208,"context_line":"            # If this is an aggregate batch, create output that will include"},{"line_number":209,"context_line":"            # resource provider as the first field to differentiate the values"},{"line_number":210,"context_line":"            rows \u003d ()"}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_0d022770","line":207,"range":{"start_line":207,"start_character":11,"end_line":207,"end_character":34},"updated":"2019-07-31 06:00:27.000000000","message":"I would reuse `parsed_args.aggregate` here since there could be cases when only one resource provider is found in the aggregate and in that case we\u0027d like to see the rp uuid.\n\nIf we don\u0027t have to care so much about the compatibility I would just always show rp uuid regardless of whether per rp uuid request or per aggregate request.","commit_id":"b182a6b8e886a35b1fd641156a3a76014eaa398e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5f3a5f4034b5b3557cf34b6ab5cc17e8b5d1b681","unresolved":false,"context_lines":[{"line_number":204,"context_line":"            return rows"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"        fields \u003d (\u0027resource_class\u0027, ) + FIELDS"},{"line_number":207,"context_line":"        if len(resources_list) \u003e 1:"},{"line_number":208,"context_line":"            # If this is an aggregate batch, create output that will include"},{"line_number":209,"context_line":"            # resource provider as the first field to differentiate the values"},{"line_number":210,"context_line":"            rows \u003d ()"}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_152465ae","line":207,"range":{"start_line":207,"start_character":11,"end_line":207,"end_character":34},"in_reply_to":"7faddb67_0d022770","updated":"2019-07-31 16:19:49.000000000","message":"Makes sense, parsed_args.aggregate would be better here.\n\nYeah, that\u0027s what I was thinking. I was being conservative in case not to change the pre-existing behavior when --aggregate is not used.","commit_id":"b182a6b8e886a35b1fd641156a3a76014eaa398e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"52bc5658d5ab06cc72f81885b71e4092b1f1d6c8","unresolved":false,"context_lines":[{"line_number":136,"context_line":"            action\u003d\u0027append\u0027"},{"line_number":137,"context_line":"        )"},{"line_number":138,"context_line":"        parser.add_argument("},{"line_number":139,"context_line":"            \u0027--amend\u0027,"},{"line_number":140,"context_line":"            action\u003d\u0027store_true\u0027,"},{"line_number":141,"context_line":"            help\u003d\u0027If this option is specified, the inventories will be \u0027"},{"line_number":142,"context_line":"                 \u0027amended instead of being fully replaced\u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"7faddb67_0860b9ce","line":139,"updated":"2019-08-01 14:53:26.000000000","message":"nit: seems this could be a separate change without the --aggregate option right? I\u0027m not saying you have to, but this is a fairly large and complicated change since you\u0027re adding essentially three new features into one patch. I would have probably broken the story down into three tasks, one for each option.","commit_id":"515e331d3e057a3d3c857218b7474a61fe1343b5"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d1eb8f79621dba1a1fcaf93a97afe9c6dfa030d","unresolved":false,"context_lines":[{"line_number":136,"context_line":"            action\u003d\u0027append\u0027"},{"line_number":137,"context_line":"        )"},{"line_number":138,"context_line":"        parser.add_argument("},{"line_number":139,"context_line":"            \u0027--amend\u0027,"},{"line_number":140,"context_line":"            action\u003d\u0027store_true\u0027,"},{"line_number":141,"context_line":"            help\u003d\u0027If this option is specified, the inventories will be \u0027"},{"line_number":142,"context_line":"                 \u0027amended instead of being fully replaced\u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"7faddb67_c26038a8","line":139,"in_reply_to":"7faddb67_0860b9ce","updated":"2019-08-01 20:23:18.000000000","message":"Yes, it could be split up. It stayed together as a result of some rounds of review feedback and since I didn\u0027t know which version people were going to be OK with, I didn\u0027t bother splitting anything if it\u0027s just going to be NAK\u0027d again. It began as an allocation ratios specific command for aggregates (this was my actual goal). Then it was changed to a new \u0027resource provider inventory update\u0027 command, then it was changed to a --amend option to the \u0027set\u0027 command. Again just trying to set allocation ratios for an aggregate. And --dry-run was just trying to offer something nice for a batch command. Sometimes people would rather see a preview first before blasting a command for a batch of items. \n\nI could split it up into separate changes, but Tetsuro already reviewed it and I\u0027d rather avoid making more work for him. But if he\u0027d rather it be split up too, I can do it.","commit_id":"515e331d3e057a3d3c857218b7474a61fe1343b5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"38c0f8c5c05fe69f79dfb9ff57f7dc35f4ff2fca","unresolved":false,"context_lines":[{"line_number":136,"context_line":"            action\u003d\u0027append\u0027"},{"line_number":137,"context_line":"        )"},{"line_number":138,"context_line":"        parser.add_argument("},{"line_number":139,"context_line":"            \u0027--amend\u0027,"},{"line_number":140,"context_line":"            action\u003d\u0027store_true\u0027,"},{"line_number":141,"context_line":"            help\u003d\u0027If this option is specified, the inventories will be \u0027"},{"line_number":142,"context_line":"                 \u0027amended instead of being fully replaced\u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"7faddb67_2d8c9134","line":139,"in_reply_to":"7faddb67_12ea1fd7","updated":"2019-08-02 15:46:37.000000000","message":"For those of us that haven\u0027t reviewed it in depth yet, a series of smaller changes is easier, and easier to revert if one of them causes some nasty issue. Having said that I haven\u0027t had a chance yet to kick this around in devstack yet (my devstack vms keep having stupid problems and crashing services and I haven\u0027t figured that out yet).","commit_id":"515e331d3e057a3d3c857218b7474a61fe1343b5"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"69bdecf5ea99905dd731176554a36a8902c6acd9","unresolved":false,"context_lines":[{"line_number":136,"context_line":"            action\u003d\u0027append\u0027"},{"line_number":137,"context_line":"        )"},{"line_number":138,"context_line":"        parser.add_argument("},{"line_number":139,"context_line":"            \u0027--amend\u0027,"},{"line_number":140,"context_line":"            action\u003d\u0027store_true\u0027,"},{"line_number":141,"context_line":"            help\u003d\u0027If this option is specified, the inventories will be \u0027"},{"line_number":142,"context_line":"                 \u0027amended instead of being fully replaced\u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"7faddb67_12ea1fd7","line":139,"in_reply_to":"7faddb67_c26038a8","updated":"2019-08-02 00:47:43.000000000","message":"I agree that smaller patches would have been better, but now since we are here, I don\u0027t think it is worth having additional work to split up.","commit_id":"515e331d3e057a3d3c857218b7474a61fe1343b5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"52bc5658d5ab06cc72f81885b71e4092b1f1d6c8","unresolved":false,"context_lines":[{"line_number":150,"context_line":"                 \u0027``--os-placement-api-version 1.3``\u0027"},{"line_number":151,"context_line":"        )"},{"line_number":152,"context_line":"        parser.add_argument("},{"line_number":153,"context_line":"            \u0027--dry-run\u0027,"},{"line_number":154,"context_line":"            action\u003d\u0027store_true\u0027,"},{"line_number":155,"context_line":"            help\u003d\u0027If this option is specified, the inventories that would be \u0027"},{"line_number":156,"context_line":"                 \u0027set will be returned without actually setting any \u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"7faddb67_28449546","line":153,"updated":"2019-08-01 14:53:26.000000000","message":"nit: seems this could have been split out to a separate change to make this simpler.","commit_id":"515e331d3e057a3d3c857218b7474a61fe1343b5"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"aa0c5d603fe3527e652f7f2fad717d5703ea7f67","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                inventories[name][field] \u003d value"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"            if not parsed_args.dry_run:"},{"line_number":197,"context_line":"                resources \u003d http.request(\u0027PUT\u0027, url, json\u003dpayload).json()"},{"line_number":198,"context_line":"            else:"},{"line_number":199,"context_line":"                resources \u003d payload"},{"line_number":200,"context_line":"            resources_list.append((rp[\u0027uuid\u0027], resources))"}],"source_content_type":"text/x-python","patch_set":9,"id":"7faddb67_4f956131","line":197,"updated":"2019-08-01 03:04:48.000000000","message":"nit: maybe we will want to have try - logging error and continue to iterate the next rp exception operation here later in a follow up?","commit_id":"515e331d3e057a3d3c857218b7474a61fe1343b5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7282c8ae45902d0d29bb3f7630500a913d029bdb","unresolved":false,"context_lines":[{"line_number":139,"context_line":"            action\u003d\u0027store_true\u0027,"},{"line_number":140,"context_line":"            help\u003d\u0027If this option is specified, the inventories for all \u0027"},{"line_number":141,"context_line":"                 \u0027resource providers that are members of the aggregate will \u0027"},{"line_number":142,"context_line":"                 \u0027be amended. This option requires at least \u0027"},{"line_number":143,"context_line":"                 \u0027``--os-placement-api-version 1.3``\u0027"},{"line_number":144,"context_line":"        )"},{"line_number":145,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_d380edf9","line":142,"range":{"start_line":142,"start_character":21,"end_line":142,"end_character":28},"updated":"2019-08-20 16:51:11.000000000","message":"amended or set, as in overwritten? This might be leftover from when --amend was part of the same patch? I\u0027m just thinking about the part of the command description above that says:\n\n\u003e Note that this is a full replacement of the existing inventory. If you want to retain the existing inventory and add a new resource class inventory, you must specify all resource class inventory, old and new.","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0278ca0a0c2fa3c7b6492c4ea39d0455292b97e6","unresolved":false,"context_lines":[{"line_number":139,"context_line":"            action\u003d\u0027store_true\u0027,"},{"line_number":140,"context_line":"            help\u003d\u0027If this option is specified, the inventories for all \u0027"},{"line_number":141,"context_line":"                 \u0027resource providers that are members of the aggregate will \u0027"},{"line_number":142,"context_line":"                 \u0027be amended. This option requires at least \u0027"},{"line_number":143,"context_line":"                 \u0027``--os-placement-api-version 1.3``\u0027"},{"line_number":144,"context_line":"        )"},{"line_number":145,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_653cc4be","line":142,"range":{"start_line":142,"start_character":21,"end_line":142,"end_character":28},"in_reply_to":"7faddb67_d380edf9","updated":"2019-08-20 23:17:32.000000000","message":"Oops, yes this is a leftover from the old monopatch. It should say \u0027set\u0027 instead.","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"ccda86223b88e515a803b155aeadc3c6ed31c31c","unresolved":false,"context_lines":[{"line_number":140,"context_line":"            help\u003d\u0027If this option is specified, the inventories for all \u0027"},{"line_number":141,"context_line":"                 \u0027resource providers that are members of the aggregate will \u0027"},{"line_number":142,"context_line":"                 \u0027be amended. This option requires at least \u0027"},{"line_number":143,"context_line":"                 \u0027``--os-placement-api-version 1.3``\u0027"},{"line_number":144,"context_line":"        )"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"        return parser"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_d5a03445","line":143,"updated":"2019-08-14 08:44:31.000000000","message":"Good to note this is a non-atomic and stop-at-first-failure action if we go that way, but please see my comments below.","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7282c8ae45902d0d29bb3f7630500a913d029bdb","unresolved":false,"context_lines":[{"line_number":149,"context_line":""},{"line_number":150,"context_line":"        http \u003d self.app.client_manager.placement"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        if parsed_args.aggregate:"},{"line_number":153,"context_line":"            filters \u003d {\u0027member_of\u0027: parsed_args.uuid}"},{"line_number":154,"context_line":"            url \u003d common.url_with_filters(RP_BASE_URL, filters)"},{"line_number":155,"context_line":"            rps \u003d http.request(\u0027GET\u0027, url).json()[\u0027resource_providers\u0027]"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_b351b164","line":152,"updated":"2019-08-20 16:51:11.000000000","message":"If --aggregate is specified we should assert that the --os-plcaement-api-version is \u003e\u003d 1.3 otherwise you\u0027ll get a mostly unhelpful message:\n\n\u003e $ openstack resource provider inventory set --aggregate `uuidgen`\nInvalid query string parameters: Additional properties are not allowed (u\u0027member_of\u0027 was unexpected)  Failed validating \u0027additionalProperties\u0027 in schema:     {\u0027additionalProperties\u0027: False,      \u0027properties\u0027: {\u0027name\u0027: {\u0027type\u0027: \u0027string\u0027},                     \u0027uuid\u0027: {\u0027format\u0027: \u0027uuid\u0027, \u0027type\u0027: \u0027string\u0027}},      \u0027type\u0027: \u0027object\u0027}  On instance:     {u\u0027member_of\u0027: u\u0027b41581d2-ea14-4a5f-9a10-314d96482673\u0027} (HTTP 400)\n\nYou\u0027ll have to add the CheckerMixin to the class but then you can just do this:\n\n  self.check_version(version.ge(\u00271.3\u0027))","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0278ca0a0c2fa3c7b6492c4ea39d0455292b97e6","unresolved":false,"context_lines":[{"line_number":149,"context_line":""},{"line_number":150,"context_line":"        http \u003d self.app.client_manager.placement"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        if parsed_args.aggregate:"},{"line_number":153,"context_line":"            filters \u003d {\u0027member_of\u0027: parsed_args.uuid}"},{"line_number":154,"context_line":"            url \u003d common.url_with_filters(RP_BASE_URL, filters)"},{"line_number":155,"context_line":"            rps \u003d http.request(\u0027GET\u0027, url).json()[\u0027resource_providers\u0027]"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_856c60d0","line":152,"in_reply_to":"7faddb67_0985f459","updated":"2019-08-20 23:17:32.000000000","message":"OK, will add.","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9e7184d488d2538f8bd929299e24d3445f46c2f9","unresolved":false,"context_lines":[{"line_number":149,"context_line":""},{"line_number":150,"context_line":"        http \u003d self.app.client_manager.placement"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        if parsed_args.aggregate:"},{"line_number":153,"context_line":"            filters \u003d {\u0027member_of\u0027: parsed_args.uuid}"},{"line_number":154,"context_line":"            url \u003d common.url_with_filters(RP_BASE_URL, filters)"},{"line_number":155,"context_line":"            rps \u003d http.request(\u0027GET\u0027, url).json()[\u0027resource_providers\u0027]"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_0985f459","line":152,"in_reply_to":"7faddb67_b351b164","updated":"2019-08-20 17:21:08.000000000","message":"We should probably have a simple test for this as well.","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"ccda86223b88e515a803b155aeadc3c6ed31c31c","unresolved":false,"context_lines":[{"line_number":173,"context_line":"                name, field, value \u003d parse_resource_argument(r)"},{"line_number":174,"context_line":"                inventories[name][field] \u003d value"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"            resources \u003d http.request(\u0027PUT\u0027, url, json\u003dpayload).json()"},{"line_number":177,"context_line":"            resources_list.append((rp[\u0027uuid\u0027], resources))"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"        def get_rows(fields, resources, rp_uuid\u003dNone):"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_d5f4b4f3","line":176,"range":{"start_line":176,"start_character":11,"end_line":176,"end_character":69},"updated":"2019-08-14 08:44:31.000000000","message":"When this fails in the iteration, no such (fields, rows)@L199 is displayed and operators can\u0027t know which rps are already processed or not yet processed, which I think warrants -1.\n\nWhy not have something like:\n\n  try:\n      resources \u003d http.request(\u0027PUT\u0027, url, json\u003dpayload).json()\n  except Exception as e:\n      self.log.warning(\"Failed to update %(rp)s\u0027: %(e)s\",\n                      {\u0027rp\u0027: rp[\u0027uuid\u0027], \u0027e\u0027: e})\n      continue\n\nhere?","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"aae27171451d42bec98a16899e4a85994db3fcf7","unresolved":false,"context_lines":[{"line_number":173,"context_line":"                name, field, value \u003d parse_resource_argument(r)"},{"line_number":174,"context_line":"                inventories[name][field] \u003d value"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"            resources \u003d http.request(\u0027PUT\u0027, url, json\u003dpayload).json()"},{"line_number":177,"context_line":"            resources_list.append((rp[\u0027uuid\u0027], resources))"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"        def get_rows(fields, resources, rp_uuid\u003dNone):"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_d6ae9b7f","line":176,"range":{"start_line":176,"start_character":11,"end_line":176,"end_character":69},"in_reply_to":"7faddb67_d5f4b4f3","updated":"2019-08-20 16:59:00.000000000","message":"Yeah good point about handling per-provider failures if we\u0027re looping. We may also want some logic in here to only log if we\u0027ve got more than one provider, otherwise just let it fail like it would without using --aggregate. Otherwise if we\u0027ve got multiple providers in the list, set a flag if any fail and make sure to fail the command at the end after dumping which providers were updated.","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0278ca0a0c2fa3c7b6492c4ea39d0455292b97e6","unresolved":false,"context_lines":[{"line_number":173,"context_line":"                name, field, value \u003d parse_resource_argument(r)"},{"line_number":174,"context_line":"                inventories[name][field] \u003d value"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"            resources \u003d http.request(\u0027PUT\u0027, url, json\u003dpayload).json()"},{"line_number":177,"context_line":"            resources_list.append((rp[\u0027uuid\u0027], resources))"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"        def get_rows(fields, resources, rp_uuid\u003dNone):"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_6565a4e0","line":176,"range":{"start_line":176,"start_character":11,"end_line":176,"end_character":69},"in_reply_to":"7faddb67_d6ae9b7f","updated":"2019-08-20 23:17:32.000000000","message":"OK, will add.","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9e7184d488d2538f8bd929299e24d3445f46c2f9","unresolved":false,"context_lines":[{"line_number":195,"context_line":"            for rp_uuid, resources in resources_list:"},{"line_number":196,"context_line":"                subrows \u003d get_rows(fields, resources, rp_uuid\u003drp_uuid)"},{"line_number":197,"context_line":"                rows \u003d itertools.chain(rows, subrows)"},{"line_number":198,"context_line":"            fields \u003d (\u0027resource_provider\u0027, ) + fields"},{"line_number":199,"context_line":"            return fields, rows"},{"line_number":200,"context_line":"        else:"},{"line_number":201,"context_line":"            # If this was not an aggregate batch, show output for the one"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_89dd6471","line":198,"updated":"2019-08-20 17:21:08.000000000","message":"In test_with_aggregate, can we assert that this is the first column in the output? Or just somehow know that in the aggregate case we dump the providers in the aggregates differently in the output.","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0278ca0a0c2fa3c7b6492c4ea39d0455292b97e6","unresolved":false,"context_lines":[{"line_number":195,"context_line":"            for rp_uuid, resources in resources_list:"},{"line_number":196,"context_line":"                subrows \u003d get_rows(fields, resources, rp_uuid\u003drp_uuid)"},{"line_number":197,"context_line":"                rows \u003d itertools.chain(rows, subrows)"},{"line_number":198,"context_line":"            fields \u003d (\u0027resource_provider\u0027, ) + fields"},{"line_number":199,"context_line":"            return fields, rows"},{"line_number":200,"context_line":"        else:"},{"line_number":201,"context_line":"            # If this was not an aggregate batch, show output for the one"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_c59b38c6","line":198,"in_reply_to":"7faddb67_89dd6471","updated":"2019-08-20 23:17:32.000000000","message":"I assume that can be done. Will try to add.\n\nNote that on an earlier PS, Tetsuro suggested just making the output the same in either case, but I was worried about changing the already existing format when --aggregate is not passed. If you think we can go ahead and change the output format for without --aggregate, I can do that.","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fd81973749b1b7bf7fd18a57495703859a71ef68","unresolved":false,"context_lines":[{"line_number":195,"context_line":"            for rp_uuid, resources in resources_list:"},{"line_number":196,"context_line":"                subrows \u003d get_rows(fields, resources, rp_uuid\u003drp_uuid)"},{"line_number":197,"context_line":"                rows \u003d itertools.chain(rows, subrows)"},{"line_number":198,"context_line":"            fields \u003d (\u0027resource_provider\u0027, ) + fields"},{"line_number":199,"context_line":"            return fields, rows"},{"line_number":200,"context_line":"        else:"},{"line_number":201,"context_line":"            # If this was not an aggregate batch, show output for the one"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_645914fb","line":198,"in_reply_to":"7faddb67_c59b38c6","updated":"2019-08-21 18:55:27.000000000","message":"I was just asking that in the --aggregate case we assert that resource_provider is a column in the output, not suggesting that we merge the output formats for the agg and non-agg cases.","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d3cacb320b9a2acde8342ed25fcb41f822015ca","unresolved":false,"context_lines":[{"line_number":178,"context_line":"                resources \u003d http.request(\u0027PUT\u0027, url, json\u003dpayload).json()"},{"line_number":179,"context_line":"            except Exception as exp:"},{"line_number":180,"context_line":"                if parsed_args.aggregate:"},{"line_number":181,"context_line":"                    self.log.warning("},{"line_number":182,"context_line":"                        \u0027Failed to set inventory for resource provider \u0027"},{"line_number":183,"context_line":"                        \u0027%(rp)s: %(exp)s\u0027, {\u0027rp\u0027: rp[\u0027uuid\u0027], \u0027exp\u0027: exp})"},{"line_number":184,"context_line":"                    continue"}],"source_content_type":"text/x-python","patch_set":11,"id":"7faddb67_e4f42457","line":181,"range":{"start_line":181,"start_character":29,"end_line":181,"end_character":36},"updated":"2019-08-21 19:24:45.000000000","message":"Seems like error would be more appropriate here.","commit_id":"18d75cd8192af9aab5604a51deabbbd196b7ab86"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"facfc90e6648c11daf13e5aa7370402f8ecdefbc","unresolved":false,"context_lines":[{"line_number":178,"context_line":"                resources \u003d http.request(\u0027PUT\u0027, url, json\u003dpayload).json()"},{"line_number":179,"context_line":"            except Exception as exp:"},{"line_number":180,"context_line":"                if parsed_args.aggregate:"},{"line_number":181,"context_line":"                    self.log.warning("},{"line_number":182,"context_line":"                        \u0027Failed to set inventory for resource provider \u0027"},{"line_number":183,"context_line":"                        \u0027%(rp)s: %(exp)s\u0027, {\u0027rp\u0027: rp[\u0027uuid\u0027], \u0027exp\u0027: exp})"},{"line_number":184,"context_line":"                    continue"}],"source_content_type":"text/x-python","patch_set":11,"id":"7faddb67_596b3084","line":181,"range":{"start_line":181,"start_character":29,"end_line":181,"end_character":36},"in_reply_to":"7faddb67_e4f42457","updated":"2019-08-28 19:50:26.000000000","message":"Done","commit_id":"18d75cd8192af9aab5604a51deabbbd196b7ab86"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ec5a7859d681294c118648281f52003928435a1c","unresolved":false,"context_lines":[{"line_number":181,"context_line":"                    self.log.warning("},{"line_number":182,"context_line":"                        \u0027Failed to set inventory for resource provider \u0027"},{"line_number":183,"context_line":"                        \u0027%(rp)s: %(exp)s\u0027, {\u0027rp\u0027: rp[\u0027uuid\u0027], \u0027exp\u0027: exp})"},{"line_number":184,"context_line":"                    continue"},{"line_number":185,"context_line":"                else:"},{"line_number":186,"context_line":"                    raise"},{"line_number":187,"context_line":"            resources_list.append((rp[\u0027uuid\u0027], resources))"}],"source_content_type":"text/x-python","patch_set":11,"id":"7faddb67_447f38de","line":184,"updated":"2019-08-21 19:22:33.000000000","message":"Shouldn\u0027t we keep track if anything failed and fail the overall command, i.e. we shouldn\u0027t exit with 0 if something failed, right? So we PUT what we can, log what we can\u0027t but then exit with 1 or something?","commit_id":"18d75cd8192af9aab5604a51deabbbd196b7ab86"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"facfc90e6648c11daf13e5aa7370402f8ecdefbc","unresolved":false,"context_lines":[{"line_number":181,"context_line":"                    self.log.warning("},{"line_number":182,"context_line":"                        \u0027Failed to set inventory for resource provider \u0027"},{"line_number":183,"context_line":"                        \u0027%(rp)s: %(exp)s\u0027, {\u0027rp\u0027: rp[\u0027uuid\u0027], \u0027exp\u0027: exp})"},{"line_number":184,"context_line":"                    continue"},{"line_number":185,"context_line":"                else:"},{"line_number":186,"context_line":"                    raise"},{"line_number":187,"context_line":"            resources_list.append((rp[\u0027uuid\u0027], resources))"}],"source_content_type":"text/x-python","patch_set":11,"id":"7faddb67_f90f7cdd","line":184,"in_reply_to":"7faddb67_447f38de","updated":"2019-08-28 19:50:26.000000000","message":"Oh, ok, I wasn\u0027t thinking about the exit code last time you mentioned it, so let me add that. I had been only thinking about not exiting the command prematurely.\n\n(later) I found some handy documentation for exactly this case \\o/\n\nhttps://docs.openstack.org/python-openstackclient/latest/contributor/command-errors.html#multiple-rest-api-calls","commit_id":"18d75cd8192af9aab5604a51deabbbd196b7ab86"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e858a81e80df4f20a363fa3dd5d15b8413d5cf32","unresolved":false,"context_lines":[{"line_number":183,"context_line":"                        \u0027%(rp)s: %(exp)s\u0027, {\u0027rp\u0027: rp[\u0027uuid\u0027], \u0027exp\u0027: exp})"},{"line_number":184,"context_line":"                    continue"},{"line_number":185,"context_line":"                else:"},{"line_number":186,"context_line":"                    raise"},{"line_number":187,"context_line":"            resources_list.append((rp[\u0027uuid\u0027], resources))"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        def get_rows(fields, resources, rp_uuid\u003dNone):"}],"source_content_type":"text/x-python","patch_set":11,"id":"7faddb67_d63770af","line":186,"updated":"2019-08-21 04:42:00.000000000","message":"Could someone please suggest a good way to test this part? I don\u0027t find other examples of mocking in the func tests. Should it be a unit test?","commit_id":"18d75cd8192af9aab5604a51deabbbd196b7ab86"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ec5a7859d681294c118648281f52003928435a1c","unresolved":false,"context_lines":[{"line_number":182,"context_line":"                        \u0027Failed to set inventory for resource provider \u0027"},{"line_number":183,"context_line":"                        \u0027%(rp)s: %(exp)s\u0027, {\u0027rp\u0027: rp[\u0027uuid\u0027], \u0027exp\u0027: exp})"},{"line_number":184,"context_line":"                    continue"},{"line_number":185,"context_line":"                else:"},{"line_number":186,"context_line":"                    raise"},{"line_number":187,"context_line":"            resources_list.append((rp[\u0027uuid\u0027], resources))"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        def get_rows(fields, resources, rp_uuid\u003dNone):"}],"source_content_type":"text/x-python","patch_set":11,"id":"7faddb67_645b5485","line":186,"range":{"start_line":185,"start_character":16,"end_line":186,"end_character":25},"updated":"2019-08-21 19:22:33.000000000","message":"Instead of raising explicitly, seems you should use excutils.save_and_reraise_exception:\n\nwith excutils.save_and_reraise_exception() as err_ctx:\n    if parsed_args.aggregate:\n        ...\n        err_ctx.reraise \u003d False\n        continue\n\nNote that osc-placement doesn\u0027t explicitly require oslo.utils yet but it implicitly does because we require osc-lib which requires oslo.utils. So you\u0027d have to add oslo.utils to requirements.txt but my point is it\u0027s not a big deal to add the new dependency since we\u0027ve already implicitly required oslo.utils via transitive dependency.","commit_id":"18d75cd8192af9aab5604a51deabbbd196b7ab86"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e797e076e446b4f0a4df77ca9aa733d11bb7bf96","unresolved":false,"context_lines":[{"line_number":183,"context_line":"                        \u0027%(rp)s: %(exp)s\u0027, {\u0027rp\u0027: rp[\u0027uuid\u0027], \u0027exp\u0027: exp})"},{"line_number":184,"context_line":"                    continue"},{"line_number":185,"context_line":"                else:"},{"line_number":186,"context_line":"                    raise"},{"line_number":187,"context_line":"            resources_list.append((rp[\u0027uuid\u0027], resources))"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        def get_rows(fields, resources, rp_uuid\u003dNone):"}],"source_content_type":"text/x-python","patch_set":11,"id":"7faddb67_bf187975","line":186,"in_reply_to":"7faddb67_640d948d","updated":"2019-08-21 19:34:43.000000000","message":"Slight correction on the failure test, it looks like placement will let you exceed capacity by dropping inventory below what\u0027s allocated, it only logs a warning. What you can\u0027t do is drop a resource class from inventory that has allocations altogether, so to test that you could just set a CUSTOM_FOO resource class inventory on one of the providers, create an allocation for CUSTOM_FOO on that provider, and then when you set inventories in aggregate you just omit CUSTOM_FOO so it\u0027s dropped which should trigger the error.","commit_id":"18d75cd8192af9aab5604a51deabbbd196b7ab86"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"facfc90e6648c11daf13e5aa7370402f8ecdefbc","unresolved":false,"context_lines":[{"line_number":182,"context_line":"                        \u0027Failed to set inventory for resource provider \u0027"},{"line_number":183,"context_line":"                        \u0027%(rp)s: %(exp)s\u0027, {\u0027rp\u0027: rp[\u0027uuid\u0027], \u0027exp\u0027: exp})"},{"line_number":184,"context_line":"                    continue"},{"line_number":185,"context_line":"                else:"},{"line_number":186,"context_line":"                    raise"},{"line_number":187,"context_line":"            resources_list.append((rp[\u0027uuid\u0027], resources))"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        def get_rows(fields, resources, rp_uuid\u003dNone):"}],"source_content_type":"text/x-python","patch_set":11,"id":"7faddb67_394d1499","line":186,"range":{"start_line":185,"start_character":16,"end_line":186,"end_character":25},"in_reply_to":"7faddb67_645b5485","updated":"2019-08-28 19:50:26.000000000","message":"Done","commit_id":"18d75cd8192af9aab5604a51deabbbd196b7ab86"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ec5a7859d681294c118648281f52003928435a1c","unresolved":false,"context_lines":[{"line_number":183,"context_line":"                        \u0027%(rp)s: %(exp)s\u0027, {\u0027rp\u0027: rp[\u0027uuid\u0027], \u0027exp\u0027: exp})"},{"line_number":184,"context_line":"                    continue"},{"line_number":185,"context_line":"                else:"},{"line_number":186,"context_line":"                    raise"},{"line_number":187,"context_line":"            resources_list.append((rp[\u0027uuid\u0027], resources))"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        def get_rows(fields, resources, rp_uuid\u003dNone):"}],"source_content_type":"text/x-python","patch_set":11,"id":"7faddb67_640d948d","line":186,"in_reply_to":"7faddb67_d63770af","updated":"2019-08-21 19:22:33.000000000","message":"I was wondering the same. I think one way might be to post allocations against one of the providers in the aggregate and then try to set one of the inventories below what the allocation needs, e.g. let\u0027s say you start with initial total VCPU of 8, then some consumer takes 6 VCPU and then you try to set VCPU total to 4, it should fail and only for that one provider that has the allocation against it.","commit_id":"18d75cd8192af9aab5604a51deabbbd196b7ab86"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"525003ecf8bb295e1a2299e72f4cf7d6c1f35d66","unresolved":false,"context_lines":[{"line_number":165,"context_line":"            rps \u003d [http.request(\u0027GET\u0027, url).json()]"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"        resources_list \u003d []"},{"line_number":168,"context_line":"        ret \u003d 0"},{"line_number":169,"context_line":"        for rp in rps:"},{"line_number":170,"context_line":"            inventories \u003d defaultdict(dict)"},{"line_number":171,"context_line":"            url \u003d BASE_URL.format(uuid\u003drp[\u0027uuid\u0027])"}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_e2e6bbe7","line":168,"range":{"start_line":168,"start_character":8,"end_line":168,"end_character":11},"updated":"2019-08-30 15:38:16.000000000","message":"nit: failure_counts?\n\nThis looks as if it was a return code.\n(Or you could have used \"len(rps) - len(resources_list)\" in L193 instead)","commit_id":"f08cb3981315d60ddc611f0229fbdca495641660"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"74a57d93758bf7273f3024dfce5adcaa31c568f3","unresolved":false,"context_lines":[{"line_number":165,"context_line":"            rps \u003d [http.request(\u0027GET\u0027, url).json()]"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"        resources_list \u003d []"},{"line_number":168,"context_line":"        ret \u003d 0"},{"line_number":169,"context_line":"        for rp in rps:"},{"line_number":170,"context_line":"            inventories \u003d defaultdict(dict)"},{"line_number":171,"context_line":"            url \u003d BASE_URL.format(uuid\u003drp[\u0027uuid\u0027])"}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_a46bf01c","line":168,"range":{"start_line":168,"start_character":8,"end_line":168,"end_character":11},"in_reply_to":"7faddb67_e2e6bbe7","updated":"2019-08-30 15:58:23.000000000","message":"Mel was following the docs on how to handle errors across multiple API calls:\n\nhttps://docs.openstack.org/python-openstackclient/latest/contributor/command-errors.html#multiple-rest-api-calls","commit_id":"f08cb3981315d60ddc611f0229fbdca495641660"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"525003ecf8bb295e1a2299e72f4cf7d6c1f35d66","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        if ret \u003e 0:"},{"line_number":194,"context_line":"            msg \u003d _(\u0027Failed to set inventory for %(ret)s of %(total)s \u0027"},{"line_number":195,"context_line":"                    \u0027resource providers.\u0027) % {\u0027ret\u0027: ret, \u0027total\u0027: len(rps)}"},{"line_number":196,"context_line":"            raise exceptions.CommandError(msg)"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"        def get_rows(fields, resources, rp_uuid\u003dNone):"},{"line_number":199,"context_line":"            inventories \u003d ["}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_60408ba3","line":196,"updated":"2019-08-30 15:38:16.000000000","message":"Nice! Thanks for adding these error handlings.","commit_id":"f08cb3981315d60ddc611f0229fbdca495641660"}],"osc_placement/tests/functional/base.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a48ac1cf97508f175f5dbb14a80ee46f0d267b99","unresolved":false,"context_lines":[{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    def resource_inventory_set(self, uuid, *resources, **kwargs):"},{"line_number":266,"context_line":"        opts \u003d []"},{"line_number":267,"context_line":"        if kwargs.get(\u0027aggregate\u0027):"},{"line_number":268,"context_line":"            opts.append(\u0027--aggregate\u0027)"},{"line_number":269,"context_line":"        fmt \u003d \u0027resource provider inventory set {uuid} {resources} {opts}\u0027"},{"line_number":270,"context_line":"        cmd \u003d fmt.format("}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_4919ac77","line":267,"range":{"start_line":267,"start_character":11,"end_line":267,"end_character":17},"updated":"2019-08-20 17:18:06.000000000","message":"nit: rather than kwargs can we just have a aggregate\u003dFalse kwarg? Or is that not possible if you have *resources before it?","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0278ca0a0c2fa3c7b6492c4ea39d0455292b97e6","unresolved":false,"context_lines":[{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    def resource_inventory_set(self, uuid, *resources, **kwargs):"},{"line_number":266,"context_line":"        opts \u003d []"},{"line_number":267,"context_line":"        if kwargs.get(\u0027aggregate\u0027):"},{"line_number":268,"context_line":"            opts.append(\u0027--aggregate\u0027)"},{"line_number":269,"context_line":"        fmt \u003d \u0027resource provider inventory set {uuid} {resources} {opts}\u0027"},{"line_number":270,"context_line":"        cmd \u003d fmt.format("}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_a57ddc6d","line":267,"range":{"start_line":267,"start_character":11,"end_line":267,"end_character":17},"in_reply_to":"7faddb67_4919ac77","updated":"2019-08-20 23:17:32.000000000","message":"It\u0027s not possible if we have *resources before it. I did play around with having it be (self, uuid, resources, aggregate\u003dFalse) but that requires including a lot of unrelated change damage to go into this patch. So really to go that way, it\u0027d be best to stack a refactor patch underneath this and change all the call sites to pass lists. I was thinking that didn\u0027t seem worth it.","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fd81973749b1b7bf7fd18a57495703859a71ef68","unresolved":false,"context_lines":[{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    def resource_inventory_set(self, uuid, *resources, **kwargs):"},{"line_number":266,"context_line":"        opts \u003d []"},{"line_number":267,"context_line":"        if kwargs.get(\u0027aggregate\u0027):"},{"line_number":268,"context_line":"            opts.append(\u0027--aggregate\u0027)"},{"line_number":269,"context_line":"        fmt \u003d \u0027resource provider inventory set {uuid} {resources} {opts}\u0027"},{"line_number":270,"context_line":"        cmd \u003d fmt.format("}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_840530c4","line":267,"range":{"start_line":267,"start_character":11,"end_line":267,"end_character":17},"in_reply_to":"7faddb67_a57ddc6d","updated":"2019-08-21 18:55:27.000000000","message":"Agree it\u0027s not worth that complexity.","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"}],"osc_placement/tests/functional/test_inventory.py":[{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"40a6202b51f4ef61f93ab7c239f38eeb6407a929","unresolved":false,"context_lines":[{"line_number":265,"context_line":"                \u0027DISK_GB:allocation_ratio\u003d1.5\u0027,"},{"line_number":266,"context_line":"                \u0027DISK_GB:min_unit\u003d2\u0027,"},{"line_number":267,"context_line":"                \u0027DISK_GB:step_size\u003d2\u0027)"},{"line_number":268,"context_line":"            old_inventories.append({r[\u0027resource_class\u0027]: r for r in resp})"},{"line_number":269,"context_line":"        # Put both resource providers in the same aggregate"},{"line_number":270,"context_line":"        agg \u003d str(uuid.uuid4())"},{"line_number":271,"context_line":"        for rp in rps:"}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_18b907cb","line":268,"updated":"2019-07-31 07:04:59.000000000","message":"Yes, it is better to have different inventories here like:\n\n$ git diff\ndiff --git a/osc_placement/tests/functional/test_inventory.py b/osc_placement/tests/functional/test_inventory.py\nindex a720028..aea5b12 100644\n--- a/osc_placement/tests/functional/test_inventory.py\n+++ b/osc_placement/tests/functional/test_inventory.py\n@@ -250,21 +250,24 @@ class TestAmendInventory(base.BaseTestCase):\n     def test_amend_with_aggregate(self):\n         # Set up some existing inventories with two resource providers\n         rps \u003d []\n+        inventory2 \u003d [\u0027VCPU\u003d8\u0027,\n+                      \u0027VCPU:max_unit\u003d4\u0027,\n+                      \u0027VCPU:allocation_ratio\u003d16.0\u0027,\n+                      \u0027MEMORY_MB\u003d1024\u0027,\n+                      \u0027MEMORY_MB:reserved\u003d256\u0027,\n+                      \u0027MEMORY_MB:allocation_ratio\u003d2.5\u0027,\n+                      \u0027DISK_GB\u003d16\u0027,\n+                      \u0027DISK_GB:allocation_ratio\u003d1.5\u0027,\n+                      \u0027DISK_GB:min_unit\u003d2\u0027,\n+                      \u0027DISK_GB:step_size\u003d2\u0027]\n+        inventory1 \u003d inventory2 + [\u0027VGPU\u003d8\u0027,\n+                                   \u0027VGPU:allocation_ratio\u003d1.0\u0027,\n+                                   \u0027VGPU:min_unit\u003d2\u0027,\n+                                   \u0027VGPU:step_size\u003d2\u0027]\n         old_inventories \u003d []\n-        for i in range(2):\n+        for i, inventory in enumerate([inventory1, inventory2]):\n             rps.append(self.resource_provider_create())\n-            resp \u003d self.resource_inventory_set(\n-                rps[i][\u0027uuid\u0027],\n-                \u0027VCPU\u003d8\u0027,\n-                \u0027VCPU:max_unit\u003d4\u0027,\n-                \u0027VCPU:allocation_ratio\u003d16.0\u0027,\n-                \u0027MEMORY_MB\u003d1024\u0027,\n-                \u0027MEMORY_MB:reserved\u003d256\u0027,\n-                \u0027MEMORY_MB:allocation_ratio\u003d2.5\u0027,\n-                \u0027DISK_GB\u003d16\u0027,\n-                \u0027DISK_GB:allocation_ratio\u003d1.5\u0027,\n-                \u0027DISK_GB:min_unit\u003d2\u0027,\n-                \u0027DISK_GB:step_size\u003d2\u0027)\n+            resp \u003d self.resource_inventory_set(rps[i][\u0027uuid\u0027], *inventory)\n             old_inventories.append({r[\u0027resource_class\u0027]: r for r in resp})\n         # Put both resource providers in the same aggregate\n         agg \u003d str(uuid.uuid4())\n\nwhich describes the bug I mentioned at L163","commit_id":"b182a6b8e886a35b1fd641156a3a76014eaa398e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5f3a5f4034b5b3557cf34b6ab5cc17e8b5d1b681","unresolved":false,"context_lines":[{"line_number":265,"context_line":"                \u0027DISK_GB:allocation_ratio\u003d1.5\u0027,"},{"line_number":266,"context_line":"                \u0027DISK_GB:min_unit\u003d2\u0027,"},{"line_number":267,"context_line":"                \u0027DISK_GB:step_size\u003d2\u0027)"},{"line_number":268,"context_line":"            old_inventories.append({r[\u0027resource_class\u0027]: r for r in resp})"},{"line_number":269,"context_line":"        # Put both resource providers in the same aggregate"},{"line_number":270,"context_line":"        agg \u003d str(uuid.uuid4())"},{"line_number":271,"context_line":"        for rp in rps:"}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_753279f4","line":268,"in_reply_to":"7faddb67_18b907cb","updated":"2019-07-31 16:19:49.000000000","message":"Thanks, I will change it.","commit_id":"b182a6b8e886a35b1fd641156a3a76014eaa398e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a48ac1cf97508f175f5dbb14a80ee46f0d267b99","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            new_inventory \u003d defaultdict(dict)"},{"line_number":228,"context_line":"            new_inventory.update(copy.deepcopy(old_inventory))"},{"line_number":229,"context_line":"            for resource in resources:"},{"line_number":230,"context_line":"                cls, keyval \u003d resource.split(\u0027:\u0027)"},{"line_number":231,"context_line":"                key, val \u003d keyval.split(\u0027\u003d\u0027)"},{"line_number":232,"context_line":"                val \u003d float(val) if \u0027.\u0027 in val else int(val)"},{"line_number":233,"context_line":"                new_inventory[cls][key] \u003d val"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_960ee338","line":230,"range":{"start_line":230,"start_character":16,"end_line":230,"end_character":19},"updated":"2019-08-20 17:18:06.000000000","message":"nit: rc or resource_class since cls is a keyword","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e858a81e80df4f20a363fa3dd5d15b8413d5cf32","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            new_inventory \u003d defaultdict(dict)"},{"line_number":228,"context_line":"            new_inventory.update(copy.deepcopy(old_inventory))"},{"line_number":229,"context_line":"            for resource in resources:"},{"line_number":230,"context_line":"                cls, keyval \u003d resource.split(\u0027:\u0027)"},{"line_number":231,"context_line":"                key, val \u003d keyval.split(\u0027\u003d\u0027)"},{"line_number":232,"context_line":"                val \u003d float(val) if \u0027.\u0027 in val else int(val)"},{"line_number":233,"context_line":"                new_inventory[cls][key] \u003d val"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_c82b4fe8","line":230,"range":{"start_line":230,"start_character":16,"end_line":230,"end_character":19},"in_reply_to":"7faddb67_4598a8be","updated":"2019-08-21 04:42:00.000000000","message":"Actually, cls doesn\u0027t appear to be a keyword, but will change anyway to make it more obvious that it\u0027s resource_class.","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0278ca0a0c2fa3c7b6492c4ea39d0455292b97e6","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            new_inventory \u003d defaultdict(dict)"},{"line_number":228,"context_line":"            new_inventory.update(copy.deepcopy(old_inventory))"},{"line_number":229,"context_line":"            for resource in resources:"},{"line_number":230,"context_line":"                cls, keyval \u003d resource.split(\u0027:\u0027)"},{"line_number":231,"context_line":"                key, val \u003d keyval.split(\u0027\u003d\u0027)"},{"line_number":232,"context_line":"                val \u003d float(val) if \u0027.\u0027 in val else int(val)"},{"line_number":233,"context_line":"                new_inventory[cls][key] \u003d val"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_4598a8be","line":230,"range":{"start_line":230,"start_character":16,"end_line":230,"end_character":19},"in_reply_to":"7faddb67_960ee338","updated":"2019-08-20 23:17:32.000000000","message":"Done","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fd81973749b1b7bf7fd18a57495703859a71ef68","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            new_inventory \u003d defaultdict(dict)"},{"line_number":228,"context_line":"            new_inventory.update(copy.deepcopy(old_inventory))"},{"line_number":229,"context_line":"            for resource in resources:"},{"line_number":230,"context_line":"                cls, keyval \u003d resource.split(\u0027:\u0027)"},{"line_number":231,"context_line":"                key, val \u003d keyval.split(\u0027\u003d\u0027)"},{"line_number":232,"context_line":"                val \u003d float(val) if \u0027.\u0027 in val else int(val)"},{"line_number":233,"context_line":"                new_inventory[cls][key] \u003d val"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_640ef4e4","line":230,"range":{"start_line":230,"start_character":16,"end_line":230,"end_character":19},"in_reply_to":"7faddb67_c82b4fe8","updated":"2019-08-21 18:55:27.000000000","message":"Maybe not keyword, but cls like self is a thing. Anyway, I noticed gerrit highlighted it.","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a48ac1cf97508f175f5dbb14a80ee46f0d267b99","unresolved":false,"context_lines":[{"line_number":229,"context_line":"            for resource in resources:"},{"line_number":230,"context_line":"                cls, keyval \u003d resource.split(\u0027:\u0027)"},{"line_number":231,"context_line":"                key, val \u003d keyval.split(\u0027\u003d\u0027)"},{"line_number":232,"context_line":"                val \u003d float(val) if \u0027.\u0027 in val else int(val)"},{"line_number":233,"context_line":"                new_inventory[cls][key] \u003d val"},{"line_number":234,"context_line":"                # The resource_class field is added by the osc_placement CLI,"},{"line_number":235,"context_line":"                # so add it to our expected inventories"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_5604eb17","line":232,"updated":"2019-08-20 17:18:06.000000000","message":"nit: maybe add a comment like:\n\n# Handle allocation ratio which is a float.","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0278ca0a0c2fa3c7b6492c4ea39d0455292b97e6","unresolved":false,"context_lines":[{"line_number":229,"context_line":"            for resource in resources:"},{"line_number":230,"context_line":"                cls, keyval \u003d resource.split(\u0027:\u0027)"},{"line_number":231,"context_line":"                key, val \u003d keyval.split(\u0027\u003d\u0027)"},{"line_number":232,"context_line":"                val \u003d float(val) if \u0027.\u0027 in val else int(val)"},{"line_number":233,"context_line":"                new_inventory[cls][key] \u003d val"},{"line_number":234,"context_line":"                # The resource_class field is added by the osc_placement CLI,"},{"line_number":235,"context_line":"                # so add it to our expected inventories"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_659364a4","line":232,"in_reply_to":"7faddb67_5604eb17","updated":"2019-08-20 23:17:32.000000000","message":"Done","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a48ac1cf97508f175f5dbb14a80ee46f0d267b99","unresolved":false,"context_lines":[{"line_number":265,"context_line":"                                   \u0027VGPU:allocation_ratio\u003d1.0\u0027,"},{"line_number":266,"context_line":"                                   \u0027VGPU:min_unit\u003d2\u0027,"},{"line_number":267,"context_line":"                                   \u0027VGPU:step_size\u003d2\u0027]"},{"line_number":268,"context_line":"        old_inventories \u003d []"},{"line_number":269,"context_line":"        for i, inventory in enumerate([inventory1, inventory2]):"},{"line_number":270,"context_line":"            rps.append(self.resource_provider_create())"},{"line_number":271,"context_line":"            resp \u003d self.resource_inventory_set(rps[i][\u0027uuid\u0027], *inventory)"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_a965c0e2","line":268,"range":{"start_line":268,"start_character":8,"end_line":268,"end_character":23},"updated":"2019-08-20 17:18:06.000000000","message":"This isn\u0027t used so what\u0027s the point?","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0278ca0a0c2fa3c7b6492c4ea39d0455292b97e6","unresolved":false,"context_lines":[{"line_number":265,"context_line":"                                   \u0027VGPU:allocation_ratio\u003d1.0\u0027,"},{"line_number":266,"context_line":"                                   \u0027VGPU:min_unit\u003d2\u0027,"},{"line_number":267,"context_line":"                                   \u0027VGPU:step_size\u003d2\u0027]"},{"line_number":268,"context_line":"        old_inventories \u003d []"},{"line_number":269,"context_line":"        for i, inventory in enumerate([inventory1, inventory2]):"},{"line_number":270,"context_line":"            rps.append(self.resource_provider_create())"},{"line_number":271,"context_line":"            resp \u003d self.resource_inventory_set(rps[i][\u0027uuid\u0027], *inventory)"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_2589ec6d","line":268,"range":{"start_line":268,"start_character":8,"end_line":268,"end_character":23},"in_reply_to":"7faddb67_a965c0e2","updated":"2019-08-20 23:17:32.000000000","message":"Probably another leftover.","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a48ac1cf97508f175f5dbb14a80ee46f0d267b99","unresolved":false,"context_lines":[{"line_number":277,"context_line":"        # Now, go ahead and update the allocation ratios and verify"},{"line_number":278,"context_line":"        self.resource_inventory_set("},{"line_number":279,"context_line":"            agg,"},{"line_number":280,"context_line":"            \u0027VCPU:allocation_ratio\u003d5.0\u0027,"},{"line_number":281,"context_line":"            \u0027VCPU:total\u003d8\u0027,"},{"line_number":282,"context_line":"            \u0027MEMORY_MB:allocation_ratio\u003d6.0\u0027,"},{"line_number":283,"context_line":"            \u0027MEMORY_MB:total\u003d1024\u0027,"},{"line_number":284,"context_line":"            \u0027DISK_GB:allocation_ratio\u003d7.0\u0027,"},{"line_number":285,"context_line":"            \u0027DISK_GB:total\u003d16\u0027,"},{"line_number":286,"context_line":"            aggregate\u003dTrue)"},{"line_number":287,"context_line":"        new_inventories \u003d self._get_expected_inventories("},{"line_number":288,"context_line":"            # Since inventories are expected to be fully replaced,"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_099c14be","line":285,"range":{"start_line":280,"start_character":12,"end_line":285,"end_character":31},"updated":"2019-08-20 17:18:06.000000000","message":"Can we throw this...","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a48ac1cf97508f175f5dbb14a80ee46f0d267b99","unresolved":false,"context_lines":[{"line_number":288,"context_line":"            # Since inventories are expected to be fully replaced,"},{"line_number":289,"context_line":"            # use empty dicts for old inventories"},{"line_number":290,"context_line":"            [{}, {}],"},{"line_number":291,"context_line":"            [\u0027VCPU:allocation_ratio\u003d5.0\u0027,"},{"line_number":292,"context_line":"             \u0027VCPU:total\u003d8\u0027,"},{"line_number":293,"context_line":"             \u0027MEMORY_MB:allocation_ratio\u003d6.0\u0027,"},{"line_number":294,"context_line":"             \u0027MEMORY_MB:total\u003d1024\u0027,"},{"line_number":295,"context_line":"             \u0027DISK_GB:allocation_ratio\u003d7.0\u0027,"},{"line_number":296,"context_line":"             \u0027DISK_GB:total\u003d16\u0027,"},{"line_number":297,"context_line":"             # Placement will default the following internally"},{"line_number":298,"context_line":"             \u0027VCPU:max_unit\u003d2147483647\u0027,"},{"line_number":299,"context_line":"             \u0027VCPU:min_unit\u003d1\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_e9a41809","line":296,"range":{"start_line":291,"start_character":13,"end_line":296,"end_character":32},"updated":"2019-08-20 17:18:06.000000000","message":"...and this into a variable so we don\u0027t have to copy/paste it? Call it new_resources or something?","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0278ca0a0c2fa3c7b6492c4ea39d0455292b97e6","unresolved":false,"context_lines":[{"line_number":288,"context_line":"            # Since inventories are expected to be fully replaced,"},{"line_number":289,"context_line":"            # use empty dicts for old inventories"},{"line_number":290,"context_line":"            [{}, {}],"},{"line_number":291,"context_line":"            [\u0027VCPU:allocation_ratio\u003d5.0\u0027,"},{"line_number":292,"context_line":"             \u0027VCPU:total\u003d8\u0027,"},{"line_number":293,"context_line":"             \u0027MEMORY_MB:allocation_ratio\u003d6.0\u0027,"},{"line_number":294,"context_line":"             \u0027MEMORY_MB:total\u003d1024\u0027,"},{"line_number":295,"context_line":"             \u0027DISK_GB:allocation_ratio\u003d7.0\u0027,"},{"line_number":296,"context_line":"             \u0027DISK_GB:total\u003d16\u0027,"},{"line_number":297,"context_line":"             # Placement will default the following internally"},{"line_number":298,"context_line":"             \u0027VCPU:max_unit\u003d2147483647\u0027,"},{"line_number":299,"context_line":"             \u0027VCPU:min_unit\u003d1\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_c5e9f819","line":296,"range":{"start_line":291,"start_character":13,"end_line":296,"end_character":32},"in_reply_to":"7faddb67_e9a41809","updated":"2019-08-20 23:17:32.000000000","message":"OK","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"525003ecf8bb295e1a2299e72f4cf7d6c1f35d66","unresolved":false,"context_lines":[{"line_number":10,"context_line":"# License for the specific language governing permissions and limitations"},{"line_number":11,"context_line":"# under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"from collections import defaultdict"},{"line_number":14,"context_line":"import copy"},{"line_number":15,"context_line":"import uuid"},{"line_number":16,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_e49fa8b0","line":13,"range":{"start_line":13,"start_character":0,"end_line":13,"end_character":35},"updated":"2019-08-30 15:38:16.000000000","message":"nit: import only modules [1]\n\n[1] https://docs.openstack.org/hacking/latest/user/hacking.html#imports","commit_id":"f08cb3981315d60ddc611f0229fbdca495641660"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a058872d80614e56067965804fbfdd0c0da7e281","unresolved":false,"context_lines":[{"line_number":307,"context_line":"        self.assertIn(\u0027Failed to set inventory for 1 of 2 resource providers.\u0027,"},{"line_number":308,"context_line":"                      six.text_type(exc))"},{"line_number":309,"context_line":"        output \u003d self.output.getvalue() + self.error.getvalue()"},{"line_number":310,"context_line":"        err_txt \u003d (\"update conflict: Inventory for \u0027CUSTOM_FOO\u0027 on resource \""},{"line_number":311,"context_line":"                   \"provider \u0027%s\u0027 in use. (HTTP 409).\" % rp1_uuid)"},{"line_number":312,"context_line":"        self.assertIn(\u0027Failed to set inventory for resource provider %s: %s\u0027 %"},{"line_number":313,"context_line":"                      (rp1_uuid, err_txt), output)"}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_95f2bc9a","line":310,"range":{"start_line":310,"start_character":20,"end_line":310,"end_character":37},"updated":"2019-08-29 13:33:21.000000000","message":"This...","commit_id":"f08cb3981315d60ddc611f0229fbdca495641660"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a058872d80614e56067965804fbfdd0c0da7e281","unresolved":false,"context_lines":[{"line_number":308,"context_line":"                      six.text_type(exc))"},{"line_number":309,"context_line":"        output \u003d self.output.getvalue() + self.error.getvalue()"},{"line_number":310,"context_line":"        err_txt \u003d (\"update conflict: Inventory for \u0027CUSTOM_FOO\u0027 on resource \""},{"line_number":311,"context_line":"                   \"provider \u0027%s\u0027 in use. (HTTP 409).\" % rp1_uuid)"},{"line_number":312,"context_line":"        self.assertIn(\u0027Failed to set inventory for resource provider %s: %s\u0027 %"},{"line_number":313,"context_line":"                      (rp1_uuid, err_txt), output)"},{"line_number":314,"context_line":"        # Placement will default the following internally"}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_152c8cff","line":311,"range":{"start_line":311,"start_character":41,"end_line":311,"end_character":53},"updated":"2019-08-29 13:33:21.000000000","message":"...and this worry me about being too fragile in case the server side error response message ever changes, but we can cross that bridge if it breaks later.\n\nReminds me that we might want to think about running osc-placement tests against placement changes since osc-placement uses the PlacementFixture from the placement repo.","commit_id":"f08cb3981315d60ddc611f0229fbdca495641660"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"68b2a9a2da94b7b3e25107e2cb4e5c4ac021c4ee","unresolved":false,"context_lines":[{"line_number":308,"context_line":"                      six.text_type(exc))"},{"line_number":309,"context_line":"        output \u003d self.output.getvalue() + self.error.getvalue()"},{"line_number":310,"context_line":"        err_txt \u003d (\"update conflict: Inventory for \u0027CUSTOM_FOO\u0027 on resource \""},{"line_number":311,"context_line":"                   \"provider \u0027%s\u0027 in use. (HTTP 409).\" % rp1_uuid)"},{"line_number":312,"context_line":"        self.assertIn(\u0027Failed to set inventory for resource provider %s: %s\u0027 %"},{"line_number":313,"context_line":"                      (rp1_uuid, err_txt), output)"},{"line_number":314,"context_line":"        # Placement will default the following internally"}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_34f4e3ce","line":311,"range":{"start_line":311,"start_character":41,"end_line":311,"end_character":53},"in_reply_to":"7faddb67_152c8cff","updated":"2019-09-04 02:00:46.000000000","message":"That\u0027s fair enough. Are you saying you think it\u0027s safe enough to use the non-highlighted parts of the message though?","commit_id":"f08cb3981315d60ddc611f0229fbdca495641660"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"836842f98c83a047493b39d837eb103e95525bd0","unresolved":false,"context_lines":[{"line_number":308,"context_line":"                      six.text_type(exc))"},{"line_number":309,"context_line":"        output \u003d self.output.getvalue() + self.error.getvalue()"},{"line_number":310,"context_line":"        err_txt \u003d (\"update conflict: Inventory for \u0027CUSTOM_FOO\u0027 on resource \""},{"line_number":311,"context_line":"                   \"provider \u0027%s\u0027 in use. (HTTP 409).\" % rp1_uuid)"},{"line_number":312,"context_line":"        self.assertIn(\u0027Failed to set inventory for resource provider %s: %s\u0027 %"},{"line_number":313,"context_line":"                      (rp1_uuid, err_txt), output)"},{"line_number":314,"context_line":"        # Placement will default the following internally"}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_68bb54c3","line":311,"range":{"start_line":311,"start_character":41,"end_line":311,"end_character":53},"in_reply_to":"7faddb67_34f4e3ce","updated":"2019-09-04 13:08:07.000000000","message":"Yeah I would think those are safe but don\u0027t worry about updating now.","commit_id":"f08cb3981315d60ddc611f0229fbdca495641660"}],"releasenotes/notes/resource-provider-inventory-set-aggregate-5f2239dd2685b636.yaml":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7282c8ae45902d0d29bb3f7630500a913d029bdb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"7faddb67_d3246d62","line":7,"updated":"2019-08-20 16:51:11.000000000","message":"Might want to hint at what this is immediately useful for, something like, \"For example, CPU, MEMORY_MB and/or DISK_GB allocation ratios can be managed in aggregate similar to the compute service AggregateCoreFilter, AggregateRamFilter and AggregateDiskFilter.\"\n\nIf mentioning the nova scheduler filters is too gross, then we could say something like this instead:\n\n\"For example, CPU, MEMORY_MB and/or DISK_GB allocation ratios can be managed in aggregate to resolve `bug 1804125`_.\"\n\n.. _bug 1804125: https://docs.openstack.org/nova/latest/admin/configuration/schedulers.html#bug-1804125\n\nOr just copy the paragraph from the commit message?","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0278ca0a0c2fa3c7b6492c4ea39d0455292b97e6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"7faddb67_e5ec342a","line":7,"in_reply_to":"7faddb67_d3246d62","updated":"2019-08-20 23:17:32.000000000","message":"OK","commit_id":"3e0793f7930f7c43040e622d4d1eee490fd93386"}],"requirements.txt":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a058872d80614e56067965804fbfdd0c0da7e281","unresolved":false,"context_lines":[{"line_number":7,"context_line":"keystoneauth1\u003e\u003d3.3.0 # Apache-2.0"},{"line_number":8,"context_line":"simplejson\u003e\u003d3.16.0 # MIT"},{"line_number":9,"context_line":"osc-lib\u003e\u003d1.2.0  # Apache-2.0"},{"line_number":10,"context_line":"oslo.utils\u003e\u003d3.37.0 # Apache-2.0"}],"source_content_type":"text/plain","patch_set":15,"id":"7faddb67_15f1ec6a","line":10,"range":{"start_line":10,"start_character":14,"end_line":10,"end_character":16},"updated":"2019-08-29 13:33:21.000000000","message":"I\u0027m curious how you came up with this version to use. Since osc-placement already implicitly requires oslo.utils because of the transitive dependency via osc-lib, one could say we could just require oslo.utils\u003e\u003d3.16.0 since that\u0027s what osc-lib 1.2.0 requires:\n\nhttps://github.com/openstack/osc-lib/blob/1.2.0/requirements.txt#L12\n\nOr you could require \u003e\u003d3.41.0 since that\u0027s what is currently in upper-constraints:\n\nhttps://github.com/openstack/requirements/blob/master/upper-constraints.txt#L660\n\nDid you just copy the 3.37 from placement?\n\nhttps://github.com/openstack/placement/blob/master/requirements.txt#L19","commit_id":"f08cb3981315d60ddc611f0229fbdca495641660"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"68b2a9a2da94b7b3e25107e2cb4e5c4ac021c4ee","unresolved":false,"context_lines":[{"line_number":7,"context_line":"keystoneauth1\u003e\u003d3.3.0 # Apache-2.0"},{"line_number":8,"context_line":"simplejson\u003e\u003d3.16.0 # MIT"},{"line_number":9,"context_line":"osc-lib\u003e\u003d1.2.0  # Apache-2.0"},{"line_number":10,"context_line":"oslo.utils\u003e\u003d3.37.0 # Apache-2.0"}],"source_content_type":"text/plain","patch_set":15,"id":"7faddb67_740edbe4","line":10,"range":{"start_line":10,"start_character":14,"end_line":10,"end_character":16},"in_reply_to":"7faddb67_15f1ec6a","updated":"2019-09-04 02:00:46.000000000","message":"I copied 3.37 from placement... wasn\u0027t sure which way to go on that.","commit_id":"f08cb3981315d60ddc611f0229fbdca495641660"}],"setup.cfg":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1f3bed93b1849271d6fdbbfb5cb2efb836618a47","unresolved":false,"context_lines":[{"line_number":57,"context_line":"    resource_provider_trait_set \u003d osc_placement.resources.trait:SetResourceProviderTrait"},{"line_number":58,"context_line":"    resource_provider_trait_delete \u003d osc_placement.resources.trait:DeleteResourceProviderTrait"},{"line_number":59,"context_line":"    allocation_candidate_list \u003d osc_placement.resources.allocation_candidate:ListAllocationCandidate"},{"line_number":60,"context_line":"    aggregate_allocation_ratio_set \u003d osc_placement.resources.aggregate:AllocationRatioSet"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"[build_sphinx]"},{"line_number":63,"context_line":"source-dir \u003d doc/source"}],"source_content_type":"text/x-ttcn-cfg","patch_set":5,"id":"5fc1f717_4624bdaf","line":60,"updated":"2019-03-22 14:48:34.000000000","message":"There is already an aggregate top level key in the openstack CLI provided by nova to manipulate host aggregates. This new command now added to the same top level key but it manipulates placement aggregates if I understand correctly. Mixing these two entities under the same name could be confusing to the CLI user. The placement aggregate handling commands are under \u0027resource provider aggregate\u0027 path.","commit_id":"c350cd19156be0f2207ba5c73c5faa4517da2505"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e053d2e83eeb9de5c847c4db03deb4ced4e569c2","unresolved":false,"context_lines":[{"line_number":57,"context_line":"    resource_provider_trait_set \u003d osc_placement.resources.trait:SetResourceProviderTrait"},{"line_number":58,"context_line":"    resource_provider_trait_delete \u003d osc_placement.resources.trait:DeleteResourceProviderTrait"},{"line_number":59,"context_line":"    allocation_candidate_list \u003d osc_placement.resources.allocation_candidate:ListAllocationCandidate"},{"line_number":60,"context_line":"    aggregate_allocation_ratio_set \u003d osc_placement.resources.aggregate:AllocationRatioSet"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"[build_sphinx]"},{"line_number":63,"context_line":"source-dir \u003d doc/source"}],"source_content_type":"text/x-ttcn-cfg","patch_set":5,"id":"5fc1f717_a9945145","line":60,"in_reply_to":"5fc1f717_4624bdaf","updated":"2019-03-22 15:08:44.000000000","message":"Yeah, I did it this way because this is a batch command that operates upon multiple resource providers in a single aggregate, whereas the \u0027resource provider aggregate\u0027 commands operate upon a single resource provider UUID. I thought it might be confusing to have a batch command under \u0027resource provider aggregate\u0027. But I\u0027m fine to change it to whatever the consensus wants.","commit_id":"c350cd19156be0f2207ba5c73c5faa4517da2505"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"f1ff9531d796e8163e616796cafcb9e6a3c53cf9","unresolved":false,"context_lines":[{"line_number":38,"context_line":"    resource_provider_usage_show  \u003d osc_placement.resources.usage:ShowUsage"},{"line_number":39,"context_line":"    resource_usage_show  \u003d osc_placement.resources.usage:ResourceShowUsage"},{"line_number":40,"context_line":"    resource_provider_inventory_set       \u003d osc_placement.resources.inventory:SetInventory"},{"line_number":41,"context_line":"    resource_provider_inventory_update    \u003d osc_placement.resources.inventory:UpdateInventory"},{"line_number":42,"context_line":"    resource_provider_inventory_class_set \u003d osc_placement.resources.inventory:SetClassInventory"},{"line_number":43,"context_line":"    resource_provider_inventory_list      \u003d osc_placement.resources.inventory:ListInventory"},{"line_number":44,"context_line":"    resource_provider_inventory_show      \u003d osc_placement.resources.inventory:ShowInventory"}],"source_content_type":"text/x-ttcn-cfg","patch_set":6,"id":"5fc1f717_3f44eab7","line":41,"updated":"2019-04-06 01:44:22.000000000","message":"Note to reviewers: I chose to make a new command instead of re-using \u0027set\u0027 because \u0027set\u0027 requires the user to supply an entire replacement of inventory when called, which didn\u0027t seem friendly for a batch command (all resource providers assumed to have identical existing inventory?).\n\n\u0027update\u0027 is also useful in general I think, to let users be able to request updates without having to do full replacements, and let the CLI handle the GET, update, PUT for them.","commit_id":"35d3301b78084d57a61ed808f6b10d14e17cb422"}]}
