)]}'
{"nova/scheduler/utils.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"989c2d3642f0a27e7bc264a23a829ba0d07fce75","unresolved":false,"context_lines":[{"line_number":246,"context_line":"            # val represents the amount.  Convert to int, or warn and skip it."},{"line_number":247,"context_line":"            try:"},{"line_number":248,"context_line":"                amt \u003d int(val)"},{"line_number":249,"context_line":"                if amt \u003c 0:"},{"line_number":250,"context_line":"                    raise ValueError()"},{"line_number":251,"context_line":"            except ValueError:"},{"line_number":252,"context_line":"                LOG.warning("}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4b6375_306608c3","line":249,"range":{"start_line":249,"start_character":23,"end_line":249,"end_character":24},"updated":"2017-10-25 18:28:21.000000000","message":"This needs to allow zero so it\u0027s usable for the code path in _process_extra_specs where we delete a standard resource class.","commit_id":"b39ae3701dc08775c41b99f39176546053867018"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"dff6649d4f615efe92faeb9be91495248ad3368c","unresolved":false,"context_lines":[{"line_number":246,"context_line":"            # val represents the amount.  Convert to int, or warn and skip it."},{"line_number":247,"context_line":"            try:"},{"line_number":248,"context_line":"                amt \u003d int(val)"},{"line_number":249,"context_line":"                if amt \u003c 0:"},{"line_number":250,"context_line":"                    raise ValueError()"},{"line_number":251,"context_line":"            except ValueError:"},{"line_number":252,"context_line":"                LOG.warning("}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4b6375_d0245460","line":249,"range":{"start_line":249,"start_character":23,"end_line":249,"end_character":24},"in_reply_to":"3f4b6375_306608c3","updated":"2017-10-25 18:30:38.000000000","message":"Dude, this allows zero.  Get a grip.","commit_id":"b39ae3701dc08775c41b99f39176546053867018"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"dc52b433b65927282b3614305ace0230d37d15ee","unresolved":false,"context_lines":[{"line_number":192,"context_line":"              \u0027SRIOV_NET_VF\u0027: 1,"},{"line_number":193,"context_line":"              \u0027NET_EGRESS_BYTES_SEC\u0027: 10000,"},{"line_number":194,"context_line":"          },"},{"line_number":195,"context_line":"          \u0027required1\u0027: {"},{"line_number":196,"context_line":"              \u0027CUSTOM_PHYSNET_NET1\u0027,"},{"line_number":197,"context_line":"          },"},{"line_number":198,"context_line":"          \u0027resources2\u0027: {"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f4b6375_ab627808","line":195,"updated":"2017-10-26 11:50:27.000000000","message":"this representation of a set alongside dicts in this context is fairly non-obvious. For a moment I was all \"shouldn\u0027t this be a list!!!!\" and then I went and read the code and re-read the earlier commentary which makes it clear. Might be easier for future readers to use python\u0027s repr of a set: set([1, 2, 3]) instead?","commit_id":"aa440fe957b69a1a173051e7348f708cee7134af"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6db1100e118f62f6bd92ed389be1d2472f0324ec","unresolved":false,"context_lines":[{"line_number":192,"context_line":"              \u0027SRIOV_NET_VF\u0027: 1,"},{"line_number":193,"context_line":"              \u0027NET_EGRESS_BYTES_SEC\u0027: 10000,"},{"line_number":194,"context_line":"          },"},{"line_number":195,"context_line":"          \u0027required1\u0027: {"},{"line_number":196,"context_line":"              \u0027CUSTOM_PHYSNET_NET1\u0027,"},{"line_number":197,"context_line":"          },"},{"line_number":198,"context_line":"          \u0027resources2\u0027: {"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f4b6375_ff2dc694","line":195,"in_reply_to":"3f4b6375_ab627808","updated":"2017-10-26 13:38:20.000000000","message":"Done","commit_id":"aa440fe957b69a1a173051e7348f708cee7134af"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"dc52b433b65927282b3614305ace0230d37d15ee","unresolved":false,"context_lines":[{"line_number":209,"context_line":"        }"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    Compare and contrast with _process_extra_specs:"},{"line_number":212,"context_line":"    - This method *returns* a value rather than updating a parameter."},{"line_number":213,"context_line":"    - This method does not attempt to deal with existing flavor-provided"},{"line_number":214,"context_line":"      resources."},{"line_number":215,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3f4b6375_2b3448fb","line":212,"updated":"2017-10-26 11:50:27.000000000","message":"+10 million","commit_id":"aa440fe957b69a1a173051e7348f708cee7134af"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6db1100e118f62f6bd92ed389be1d2472f0324ec","unresolved":false,"context_lines":[{"line_number":209,"context_line":"        }"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    Compare and contrast with _process_extra_specs:"},{"line_number":212,"context_line":"    - This method *returns* a value rather than updating a parameter."},{"line_number":213,"context_line":"    - This method does not attempt to deal with existing flavor-provided"},{"line_number":214,"context_line":"      resources."},{"line_number":215,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3f4b6375_3f70fe07","line":212,"in_reply_to":"3f4b6375_2b3448fb","updated":"2017-10-26 13:38:20.000000000","message":"Though actually I should reword this in the next patch where I get rid of _process_extra_specs entirely.","commit_id":"aa440fe957b69a1a173051e7348f708cee7134af"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"dc52b433b65927282b3614305ace0230d37d15ee","unresolved":false,"context_lines":[{"line_number":223,"context_line":"    # Currently the only valid value for a trait entry is \u0027required\u0027."},{"line_number":224,"context_line":"    trait_vals \u003d (\u0027required\u0027,)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"    # Regex patterns for numbered or un-numbered resources/trait keys"},{"line_number":227,"context_line":"    rpat, tpat \u003d (re.compile(r\"^%s([1-9][0-9]*)?$\" % prefix)"},{"line_number":228,"context_line":"                  for prefix in (res_prefix, trait_prefix))"},{"line_number":229,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3f4b6375_eea3defe","line":226,"updated":"2017-10-26 11:50:27.000000000","message":"I would have been inclined to do something with split (using the prefixes) instead of regex, but this is fine.","commit_id":"aa440fe957b69a1a173051e7348f708cee7134af"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6db1100e118f62f6bd92ed389be1d2472f0324ec","unresolved":false,"context_lines":[{"line_number":223,"context_line":"    # Currently the only valid value for a trait entry is \u0027required\u0027."},{"line_number":224,"context_line":"    trait_vals \u003d (\u0027required\u0027,)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"    # Regex patterns for numbered or un-numbered resources/trait keys"},{"line_number":227,"context_line":"    rpat, tpat \u003d (re.compile(r\"^%s([1-9][0-9]*)?$\" % prefix)"},{"line_number":228,"context_line":"                  for prefix in (res_prefix, trait_prefix))"},{"line_number":229,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3f4b6375_7f955686","line":226,"in_reply_to":"3f4b6375_eea3defe","updated":"2017-10-26 13:38:20.000000000","message":"Like swapping out `if re.match(rpat, group):` (which would actually be better as `if rpat.match(group):` - fixing) with:\n\n if group.startswith(\u0027resources\u0027):\n     try:\n         num \u003d int(group.split(\u0027resources\u0027, 1)[-1])\n         if num \u003c\u003d 0:\n             raise ValueError()\n     except ValueError:\n         continue\n     ...\n\n?","commit_id":"aa440fe957b69a1a173051e7348f708cee7134af"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"dc52b433b65927282b3614305ace0230d37d15ee","unresolved":false,"context_lines":[{"line_number":250,"context_line":"                    raise ValueError()"},{"line_number":251,"context_line":"            except ValueError:"},{"line_number":252,"context_line":"                LOG.warning("},{"line_number":253,"context_line":"                    \"Resource amounts must be positive integers. Received \""},{"line_number":254,"context_line":"                    \"\u0027%(val)s\u0027 for key %(key)s.\", {\"key\": key, \"val\": val})"},{"line_number":255,"context_line":"                continue"},{"line_number":256,"context_line":"            if group not in ret:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f4b6375_ce582232","line":253,"range":{"start_line":253,"start_character":55,"end_line":253,"end_character":63},"updated":"2017-10-26 11:50:27.000000000","message":"integers or zero","commit_id":"aa440fe957b69a1a173051e7348f708cee7134af"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6db1100e118f62f6bd92ed389be1d2472f0324ec","unresolved":false,"context_lines":[{"line_number":250,"context_line":"                    raise ValueError()"},{"line_number":251,"context_line":"            except ValueError:"},{"line_number":252,"context_line":"                LOG.warning("},{"line_number":253,"context_line":"                    \"Resource amounts must be positive integers. Received \""},{"line_number":254,"context_line":"                    \"\u0027%(val)s\u0027 for key %(key)s.\", {\"key\": key, \"val\": val})"},{"line_number":255,"context_line":"                continue"},{"line_number":256,"context_line":"            if group not in ret:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f4b6375_5f53bae2","line":253,"range":{"start_line":253,"start_character":55,"end_line":253,"end_character":63},"in_reply_to":"3f4b6375_ce582232","updated":"2017-10-26 13:38:20.000000000","message":"Done","commit_id":"aa440fe957b69a1a173051e7348f708cee7134af"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"dc52b433b65927282b3614305ace0230d37d15ee","unresolved":false,"context_lines":[{"line_number":276,"context_line":"            group \u003d group.replace(trait_prefix, val)"},{"line_number":277,"context_line":"            if group not in ret:"},{"line_number":278,"context_line":"                ret[group] \u003d set()"},{"line_number":279,"context_line":"            ret[group].add(name)"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"    return ret"},{"line_number":282,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3f4b6375_0eef5a1e","line":279,"updated":"2017-10-26 11:50:27.000000000","message":"Sigh, I hope not, but do we need an catchall clauses that warns when something that is \u0027resourcesFOOBAR\u0027 or \u0027traitsFOOBAR\u0027 is present? We clearly don\u0027t want to warn on everything in extra_specs, but what about things that are trying and failing to be resource and traits?\n\nMy vote is no.","commit_id":"aa440fe957b69a1a173051e7348f708cee7134af"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6db1100e118f62f6bd92ed389be1d2472f0324ec","unresolved":false,"context_lines":[{"line_number":276,"context_line":"            group \u003d group.replace(trait_prefix, val)"},{"line_number":277,"context_line":"            if group not in ret:"},{"line_number":278,"context_line":"                ret[group] \u003d set()"},{"line_number":279,"context_line":"            ret[group].add(name)"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"    return ret"},{"line_number":282,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3f4b6375_7fa0b690","line":279,"in_reply_to":"3f4b6375_0eef5a1e","updated":"2017-10-26 13:38:20.000000000","message":"Yeah, I\u0027m gonna go with no for now.  Again, extra_specs is freeform, so we\u0027d be making assumptions and potentially warning on stuff that\u0027s intentional (not that that\u0027s a show-stopper).  But also, where do you draw the line?  I can think of several formatty things we *could* check/warn for - the exact prefix; the prefix non-lowercase; etc.  Could get out of hand pretty quick.","commit_id":"aa440fe957b69a1a173051e7348f708cee7134af"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"30678dcb0a67adeb5c55f799d7afbc8fb0828590","unresolved":false,"context_lines":[{"line_number":206,"context_line":"          \u0027resources3\u0027: {"},{"line_number":207,"context_line":"              \u0027DISK_GB\u0027: 5,"},{"line_number":208,"context_line":"          },"},{"line_number":209,"context_line":"        }"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    Compare and contrast with _process_extra_specs:"},{"line_number":212,"context_line":"    - This method *returns* a value rather than updating a parameter."}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4b6375_49954586","line":209,"updated":"2017-10-26 17:27:08.000000000","message":"We have different types at different keys, resource -\u003e dict but required -\u003e set. It will cause that the user of the return value needs to parse the keys again to see if it is starts with \u0027resource\u0027 or \u0027require\u0027 to be able to process the value under the key.\n\nFor example look at occurrences of \u0027group.startswith(\u0027resources\u0027)\u0027 in the subsequent patch. That could be avoided if the return value of this function separates resources and traits a bit better. \n\nFor example:\n{\n    \u0027resources\u0027: {\n        \u0027resources\u0027: {\n              \u0027VCPU\u0027: 2,\n              \u0027MEMORY_MB\u0027: 2048,\n        },\n        \u0027resources1\u0027: {\n              \u0027SRIOV_NET_VF\u0027: 1,\n              \u0027NET_EGRESS_BYTES_SEC\u0027: 10000,\n        }\n\n    }\n    \u0027traits\u0027: { ... }\n}\n\nIn general I suggest to consider structuring the return value in a way that it is easy to use where it will be used. If there is more that one conflicting way how the return value is used then I suggest to encapsulate the data in an object with query functions for the different usages.","commit_id":"c81208643ddd1c58655d718efada0c8d9dafebed"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"cbeea7fc15fdffe854d09841a568da79d897663d","unresolved":false,"context_lines":[{"line_number":206,"context_line":"          \u0027resources3\u0027: {"},{"line_number":207,"context_line":"              \u0027DISK_GB\u0027: 5,"},{"line_number":208,"context_line":"          },"},{"line_number":209,"context_line":"        }"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    Compare and contrast with _process_extra_specs:"},{"line_number":212,"context_line":"    - This method *returns* a value rather than updating a parameter."}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4b6375_7b155c48","line":209,"in_reply_to":"3f4b6375_49954586","updated":"2017-10-26 23:20:41.000000000","message":"Hum.\n\nI don\u0027t love the idea of the split-up dict because it makes it less obvious that resourcesX and requiredX are associated with each other.\n\nI\u0027ll do it with an object and see what you think.","commit_id":"c81208643ddd1c58655d718efada0c8d9dafebed"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"30678dcb0a67adeb5c55f799d7afbc8fb0828590","unresolved":false,"context_lines":[{"line_number":229,"context_line":""},{"line_number":230,"context_line":"    ret \u003d {}"},{"line_number":231,"context_line":"    for key, val in extra_specs.items():"},{"line_number":232,"context_line":"        if \u0027:\u0027 not in key:"},{"line_number":233,"context_line":"            continue"},{"line_number":234,"context_line":"        # \u0027group\u0027 is \u0027resources[$N]\u0027/\u0027trait[$N]\u0027"},{"line_number":235,"context_line":"        # \u0027name\u0027 is either the resource class name or the trait name."},{"line_number":236,"context_line":"        group, name \u003d key.split(\u0027:\u0027, 1)"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"        # Process \"resources[$N]\""},{"line_number":239,"context_line":"        if rpat.match(group):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4b6375_c6512238","line":236,"range":{"start_line":232,"start_character":0,"end_line":236,"end_character":39},"updated":"2017-10-26 17:27:08.000000000","message":"This could be meld into the regexp match by adding the \u0027:\u0027 to the pattern and grouping the two side of the \u0027:\u0027.  But I see that you want to get rid of the regex in the future so it is OK now as is.","commit_id":"c81208643ddd1c58655d718efada0c8d9dafebed"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"cbeea7fc15fdffe854d09841a568da79d897663d","unresolved":false,"context_lines":[{"line_number":229,"context_line":""},{"line_number":230,"context_line":"    ret \u003d {}"},{"line_number":231,"context_line":"    for key, val in extra_specs.items():"},{"line_number":232,"context_line":"        if \u0027:\u0027 not in key:"},{"line_number":233,"context_line":"            continue"},{"line_number":234,"context_line":"        # \u0027group\u0027 is \u0027resources[$N]\u0027/\u0027trait[$N]\u0027"},{"line_number":235,"context_line":"        # \u0027name\u0027 is either the resource class name or the trait name."},{"line_number":236,"context_line":"        group, name \u003d key.split(\u0027:\u0027, 1)"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"        # Process \"resources[$N]\""},{"line_number":239,"context_line":"        if rpat.match(group):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4b6375_dda4484d","line":236,"range":{"start_line":232,"start_character":0,"end_line":236,"end_character":39},"in_reply_to":"3f4b6375_c6512238","updated":"2017-10-26 23:20:41.000000000","message":"I don\u0027t want to get rid of the regex.\n\nI\u0027ll extend it and use the match object.  But note that I\u0027m gonna use .* for the right-hand side of the colon - I want to let the conditions below do the validation, not duplicate the pattern, which may change.","commit_id":"c81208643ddd1c58655d718efada0c8d9dafebed"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9903293cd889ae93bc90fdc322f56e91cfee95f6","unresolved":false,"context_lines":[{"line_number":49,"context_line":"    or \"required[$N]\"."},{"line_number":50,"context_line":"    \"\"\""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    RES_PREFIX \u003d \u0027resources\u0027"},{"line_number":53,"context_line":"    TRAIT_PREFIX \u003d \u0027trait\u0027"},{"line_number":54,"context_line":"    PREFIXES \u003d (RES_PREFIX, TRAIT_PREFIX)"},{"line_number":55,"context_line":"    # Currently the only valid value for a trait entry is \u0027required\u0027."},{"line_number":56,"context_line":"    TRAIT_VALS \u003d (\u0027required\u0027,)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def __init__(self):"},{"line_number":59,"context_line":"        # { \u0027resources[$N]\u0027: { $RESOURCE_CLASS: $AMOUNT, ... }, ...,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f4b6375_99680788","line":56,"range":{"start_line":52,"start_character":0,"end_line":56,"end_character":30},"updated":"2017-10-27 22:34:26.000000000","message":"These are extra_specs-specific, so they should be moved into from_extra_specs","commit_id":"3c9eb74fd946b65259f304c5af3fc45f000f3e4e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d9ab2ebfbee9a7b6dc70acddd308450b83a2789a","unresolved":false,"context_lines":[{"line_number":71,"context_line":"            \"trait$N:$TRAIT_NAME\": \"required\""},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        :param extra_specs: The flavor extra_specs dict."},{"line_number":74,"context_line":"        :return: A GranularResourceRequest object representing the resources"},{"line_number":75,"context_line":"                 and required traits in the extra_specs."},{"line_number":76,"context_line":"        \"\"\""},{"line_number":77,"context_line":"        # Regex patterns for numbered or un-numbered resources/trait keys"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f4b6375_4df3f174","line":74,"range":{"start_line":74,"start_character":19,"end_line":74,"end_character":27},"updated":"2017-10-27 17:37:00.000000000","message":"strike","commit_id":"3c9eb74fd946b65259f304c5af3fc45f000f3e4e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d9ab2ebfbee9a7b6dc70acddd308450b83a2789a","unresolved":false,"context_lines":[{"line_number":119,"context_line":"                    LOG.warning("},{"line_number":120,"context_line":"                        \"Only (%(tvals)s) traits are supported. Received \""},{"line_number":121,"context_line":"                        \"\u0027%(val)s\u0027 for key %(key)s.\","},{"line_number":122,"context_line":"                        {\"tvals\": \u0027,\u0027.join(cls.TRAIT_VALS), \"key\": key,"},{"line_number":123,"context_line":"                         \"val\": val})"},{"line_number":124,"context_line":"                    continue"},{"line_number":125,"context_line":"                # Validate the trait name"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f4b6375_8df8c949","line":122,"updated":"2017-10-27 17:37:00.000000000","message":"a space wouldn\u0027t go amiss here","commit_id":"3c9eb74fd946b65259f304c5af3fc45f000f3e4e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d52f21f417139973b6f10c1b9d1d7ac98cab5b60","unresolved":false,"context_lines":[{"line_number":44,"context_line":"                                                       \u0027members\u0027])"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"class ResourceGroup(object):"},{"line_number":48,"context_line":"    \"\"\"Represents a grouping of resource and trait requests."},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    Group \u00270\u0027 is special: it represents a request for resources and traits in"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_d98d3832","line":47,"range":{"start_line":47,"start_character":6,"end_line":47,"end_character":19},"updated":"2017-10-30 16:36:45.000000000","message":"This should be RequestGroup, not ResourceGroup. It\u0027s not a group of resources. It\u0027s a group of related requests (for resources and traits from the same provider tree).","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"758c5ecc8d352e2efe435b174a73573a547f02dc","unresolved":false,"context_lines":[{"line_number":44,"context_line":"                                                       \u0027members\u0027])"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"class ResourceGroup(object):"},{"line_number":48,"context_line":"    \"\"\"Represents a grouping of resource and trait requests."},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    Group \u00270\u0027 is special: it represents a request for resources and traits in"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_6918af74","line":47,"range":{"start_line":47,"start_character":6,"end_line":47,"end_character":19},"in_reply_to":"3f4b6375_d98d3832","updated":"2017-10-30 23:00:57.000000000","message":"Done","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ce2d20b0524e8b906564e50cd7ca031fa8bb2f28","unresolved":false,"context_lines":[{"line_number":47,"context_line":"class ResourceGroup(object):"},{"line_number":48,"context_line":"    \"\"\"Represents a grouping of resource and trait requests."},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    Group \u00270\u0027 is special: it represents a request for resources and traits in"},{"line_number":51,"context_line":"    any resource provider in the same tree (or, eventually, aggregate)."},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    The other groups represent requests for resources and traits which must be"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_92a2fd4a","line":50,"range":{"start_line":50,"start_character":4,"end_line":50,"end_character":24},"updated":"2017-10-30 14:25:18.000000000","message":"First I thought that resources0 is a valid input, but now I see that the regex does not accept that so it is safe to use 0.","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"758c5ecc8d352e2efe435b174a73573a547f02dc","unresolved":false,"context_lines":[{"line_number":47,"context_line":"class ResourceGroup(object):"},{"line_number":48,"context_line":"    \"\"\"Represents a grouping of resource and trait requests."},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    Group \u00270\u0027 is special: it represents a request for resources and traits in"},{"line_number":51,"context_line":"    any resource provider in the same tree (or, eventually, aggregate)."},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    The other groups represent requests for resources and traits which must be"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_69466f52","line":50,"range":{"start_line":50,"start_character":4,"end_line":50,"end_character":24},"in_reply_to":"3f4b6375_7949a430","updated":"2017-10-30 23:00:57.000000000","message":"Per discussion, it is special, but we\u0027re going to represent that via an attribute called use_same_provider.","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d52f21f417139973b6f10c1b9d1d7ac98cab5b60","unresolved":false,"context_lines":[{"line_number":47,"context_line":"class ResourceGroup(object):"},{"line_number":48,"context_line":"    \"\"\"Represents a grouping of resource and trait requests."},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    Group \u00270\u0027 is special: it represents a request for resources and traits in"},{"line_number":51,"context_line":"    any resource provider in the same tree (or, eventually, aggregate)."},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    The other groups represent requests for resources and traits which must be"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_7949a430","line":50,"range":{"start_line":50,"start_character":4,"end_line":50,"end_character":24},"in_reply_to":"3f4b6375_92a2fd4a","updated":"2017-10-30 16:36:45.000000000","message":"\"group 0\" is not special, though. it\u0027s just another request group.","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d52f21f417139973b6f10c1b9d1d7ac98cab5b60","unresolved":false,"context_lines":[{"line_number":48,"context_line":"    \"\"\"Represents a grouping of resource and trait requests."},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    Group \u00270\u0027 is special: it represents a request for resources and traits in"},{"line_number":51,"context_line":"    any resource provider in the same tree (or, eventually, aggregate)."},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    The other groups represent requests for resources and traits which must be"},{"line_number":54,"context_line":"    satisfied by a single resource provider."}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_5946e823","line":51,"range":{"start_line":51,"start_character":42,"end_line":51,"end_character":70},"updated":"2017-10-30 16:36:45.000000000","message":"aggregates don\u0027t have resources nor do they have traits.","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"758c5ecc8d352e2efe435b174a73573a547f02dc","unresolved":false,"context_lines":[{"line_number":48,"context_line":"    \"\"\"Represents a grouping of resource and trait requests."},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    Group \u00270\u0027 is special: it represents a request for resources and traits in"},{"line_number":51,"context_line":"    any resource provider in the same tree (or, eventually, aggregate)."},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    The other groups represent requests for resources and traits which must be"},{"line_number":54,"context_line":"    satisfied by a single resource provider."}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_a9d8e77e","line":51,"range":{"start_line":51,"start_character":42,"end_line":51,"end_character":70},"in_reply_to":"3f4b6375_5946e823","updated":"2017-10-30 23:00:57.000000000","message":"Gah, sorry, I keep using this a shorthand for \"RPs in the same aggregate\".  Fixed.","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d52f21f417139973b6f10c1b9d1d7ac98cab5b60","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    Group \u00270\u0027 is special: it represents a request for resources and traits in"},{"line_number":51,"context_line":"    any resource provider in the same tree (or, eventually, aggregate)."},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    The other groups represent requests for resources and traits which must be"},{"line_number":54,"context_line":"    satisfied by a single resource provider."},{"line_number":55,"context_line":"    \"\"\""},{"line_number":56,"context_line":"    def __init__(self, ident):"},{"line_number":57,"context_line":"        \"\"\"ident is a string (not a number).\"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_591dc80d","line":54,"range":{"start_line":53,"start_character":4,"end_line":54,"end_character":44},"updated":"2017-10-30 16:36:45.000000000","message":"No, sorry for being a pedant, but *all* the request groups mean the same thing: requested resources and traits that should be provided by the same provider tree.\n\n\"Request group 0\" merely represents a request for resources and traits from the same provider tree. \"Request group 1\" represents a request for resources and traits from the same provider tree. They are exactly the same concept.","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"758c5ecc8d352e2efe435b174a73573a547f02dc","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    Group \u00270\u0027 is special: it represents a request for resources and traits in"},{"line_number":51,"context_line":"    any resource provider in the same tree (or, eventually, aggregate)."},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    The other groups represent requests for resources and traits which must be"},{"line_number":54,"context_line":"    satisfied by a single resource provider."},{"line_number":55,"context_line":"    \"\"\""},{"line_number":56,"context_line":"    def __init__(self, ident):"},{"line_number":57,"context_line":"        \"\"\"ident is a string (not a number).\"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_a9c10783","line":54,"range":{"start_line":53,"start_character":4,"end_line":54,"end_character":44},"in_reply_to":"3f4b6375_591dc80d","updated":"2017-10-30 23:00:57.000000000","message":"From our discussion: per the spec, they\u0027re different.","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d52f21f417139973b6f10c1b9d1d7ac98cab5b60","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    \"\"\""},{"line_number":56,"context_line":"    def __init__(self, ident):"},{"line_number":57,"context_line":"        \"\"\"ident is a string (not a number).\"\"\""},{"line_number":58,"context_line":"        self._ident \u003d ident"},{"line_number":59,"context_line":"        self.resources \u003d {}"},{"line_number":60,"context_line":"        self.required_traits \u003d set()"},{"line_number":61,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_99e0e0fc","line":58,"updated":"2017-10-30 16:36:45.000000000","message":"Technically, you don\u0027t need this attribute...","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"758c5ecc8d352e2efe435b174a73573a547f02dc","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    \"\"\""},{"line_number":56,"context_line":"    def __init__(self, ident):"},{"line_number":57,"context_line":"        \"\"\"ident is a string (not a number).\"\"\""},{"line_number":58,"context_line":"        self._ident \u003d ident"},{"line_number":59,"context_line":"        self.resources \u003d {}"},{"line_number":60,"context_line":"        self.required_traits \u003d set()"},{"line_number":61,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_89be4bfd","line":58,"in_reply_to":"3f4b6375_99e0e0fc","updated":"2017-10-30 23:00:57.000000000","message":"Done","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d52f21f417139973b6f10c1b9d1d7ac98cab5b60","unresolved":false,"context_lines":[{"line_number":59,"context_line":"        self.resources \u003d {}"},{"line_number":60,"context_line":"        self.required_traits \u003d set()"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def add_resource(self, resource_class, amount):"},{"line_number":63,"context_line":"        self.resources[resource_class] \u003d amount"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def add_trait(self, trait_name):"},{"line_number":66,"context_line":"        self.required_traits.add(trait_name)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"class ResourceRequest(object):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_599ee854","line":66,"range":{"start_line":62,"start_character":0,"end_line":66,"end_character":44},"updated":"2017-10-30 16:36:45.000000000","message":"I don\u0027t really see the need for these helper methods, but OK.","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"758c5ecc8d352e2efe435b174a73573a547f02dc","unresolved":false,"context_lines":[{"line_number":59,"context_line":"        self.resources \u003d {}"},{"line_number":60,"context_line":"        self.required_traits \u003d set()"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def add_resource(self, resource_class, amount):"},{"line_number":63,"context_line":"        self.resources[resource_class] \u003d amount"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def add_trait(self, trait_name):"},{"line_number":66,"context_line":"        self.required_traits.add(trait_name)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"class ResourceRequest(object):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_a46fe990","line":66,"range":{"start_line":62,"start_character":0,"end_line":66,"end_character":44},"in_reply_to":"3f4b6375_599ee854","updated":"2017-10-30 23:00:57.000000000","message":"Done","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ce2d20b0524e8b906564e50cd7ca031fa8bb2f28","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        # { ident: ResourceGroup }"},{"line_number":73,"context_line":"        self._rg_by_id \u003d {}"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def get_resource_group(self, ident):"},{"line_number":76,"context_line":"        ident \u003d ident or \u00270\u0027"},{"line_number":77,"context_line":"        if ident not in self._rg_by_id:"},{"line_number":78,"context_line":"            self._rg_by_id[ident] \u003d ResourceGroup(ident)"},{"line_number":79,"context_line":"        return self._rg_by_id[ident]"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_725e6146","line":76,"range":{"start_line":75,"start_character":0,"end_line":76,"end_character":28},"updated":"2017-10-30 14:25:18.000000000","message":"My first idea was to suggest using default value for ident but then I saw that it is a lot easier to pass just None from the regex match directly. So it is OK for me.","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d52f21f417139973b6f10c1b9d1d7ac98cab5b60","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        # { ident: ResourceGroup }"},{"line_number":73,"context_line":"        self._rg_by_id \u003d {}"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def get_resource_group(self, ident):"},{"line_number":76,"context_line":"        ident \u003d ident or \u00270\u0027"},{"line_number":77,"context_line":"        if ident not in self._rg_by_id:"},{"line_number":78,"context_line":"            self._rg_by_id[ident] \u003d ResourceGroup(ident)"},{"line_number":79,"context_line":"        return self._rg_by_id[ident]"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_b9b4bcf3","line":76,"range":{"start_line":75,"start_character":0,"end_line":76,"end_character":28},"in_reply_to":"3f4b6375_725e6146","updated":"2017-10-30 16:36:45.000000000","message":"I don\u0027t see a reason for the ResourceRequest object, really, but OK.","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"758c5ecc8d352e2efe435b174a73573a547f02dc","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        # { ident: ResourceGroup }"},{"line_number":73,"context_line":"        self._rg_by_id \u003d {}"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def get_resource_group(self, ident):"},{"line_number":76,"context_line":"        ident \u003d ident or \u00270\u0027"},{"line_number":77,"context_line":"        if ident not in self._rg_by_id:"},{"line_number":78,"context_line":"            self._rg_by_id[ident] \u003d ResourceGroup(ident)"},{"line_number":79,"context_line":"        return self._rg_by_id[ident]"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_a55a5580","line":76,"range":{"start_line":75,"start_character":0,"end_line":76,"end_character":28},"in_reply_to":"3f4b6375_725e6146","updated":"2017-10-30 23:00:57.000000000","message":"Yeah, I played with it both ways; this way lets me isolate the \u00270\u0027 special case best.  I need to cite it here and in the to_queryparams method later in the series, and that\u0027s all.","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"758c5ecc8d352e2efe435b174a73573a547f02dc","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        # { ident: ResourceGroup }"},{"line_number":73,"context_line":"        self._rg_by_id \u003d {}"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def get_resource_group(self, ident):"},{"line_number":76,"context_line":"        ident \u003d ident or \u00270\u0027"},{"line_number":77,"context_line":"        if ident not in self._rg_by_id:"},{"line_number":78,"context_line":"            self._rg_by_id[ident] \u003d ResourceGroup(ident)"},{"line_number":79,"context_line":"        return self._rg_by_id[ident]"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_e45de1e9","line":76,"range":{"start_line":75,"start_character":0,"end_line":76,"end_character":28},"in_reply_to":"3f4b6375_b9b4bcf3","updated":"2017-10-30 23:00:57.000000000","message":"Yeah, I\u0027d really like to keep it for the sake of having a nice place to house the dict and the helper methods.  Especially as the series progresses, there\u0027s going to be several methods that operate on RequestGroups and it feels disjointed to handle them with separate unbound util methods.","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d52f21f417139973b6f10c1b9d1d7ac98cab5b60","unresolved":false,"context_lines":[{"line_number":75,"context_line":"    def get_resource_group(self, ident):"},{"line_number":76,"context_line":"        ident \u003d ident or \u00270\u0027"},{"line_number":77,"context_line":"        if ident not in self._rg_by_id:"},{"line_number":78,"context_line":"            self._rg_by_id[ident] \u003d ResourceGroup(ident)"},{"line_number":79,"context_line":"        return self._rg_by_id[ident]"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_399d6c66","line":78,"range":{"start_line":78,"start_character":50,"end_line":78,"end_character":55},"updated":"2017-10-30 16:36:45.000000000","message":"you can leave this off... technically you don\u0027t need to store the \"ident\" of the request group.","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"758c5ecc8d352e2efe435b174a73573a547f02dc","unresolved":false,"context_lines":[{"line_number":75,"context_line":"    def get_resource_group(self, ident):"},{"line_number":76,"context_line":"        ident \u003d ident or \u00270\u0027"},{"line_number":77,"context_line":"        if ident not in self._rg_by_id:"},{"line_number":78,"context_line":"            self._rg_by_id[ident] \u003d ResourceGroup(ident)"},{"line_number":79,"context_line":"        return self._rg_by_id[ident]"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_09401b07","line":78,"range":{"start_line":78,"start_character":50,"end_line":78,"end_character":55},"in_reply_to":"3f4b6375_399d6c66","updated":"2017-10-30 23:00:57.000000000","message":"Done","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ce2d20b0524e8b906564e50cd7ca031fa8bb2f28","unresolved":false,"context_lines":[{"line_number":135,"context_line":"                ret.get_resource_group(suffix).add_resource(name, amt)"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"            # Process \"trait[$N]\""},{"line_number":138,"context_line":"            elif prefix \u003d\u003d trait_prefix:"},{"line_number":139,"context_line":"                # It\u0027s a trait.  Ensure the value is supported."},{"line_number":140,"context_line":"                if val not in trait_vals:"},{"line_number":141,"context_line":"                    LOG.warning("}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_cf6332b1","line":138,"updated":"2017-10-30 14:25:18.000000000","message":"Two smaller functions can be introduced, one for handling resources and one from handling traits.","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d52f21f417139973b6f10c1b9d1d7ac98cab5b60","unresolved":false,"context_lines":[{"line_number":135,"context_line":"                ret.get_resource_group(suffix).add_resource(name, amt)"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"            # Process \"trait[$N]\""},{"line_number":138,"context_line":"            elif prefix \u003d\u003d trait_prefix:"},{"line_number":139,"context_line":"                # It\u0027s a trait.  Ensure the value is supported."},{"line_number":140,"context_line":"                if val not in trait_vals:"},{"line_number":141,"context_line":"                    LOG.warning("}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_d909b895","line":138,"in_reply_to":"3f4b6375_cf6332b1","updated":"2017-10-30 16:36:45.000000000","message":"I actually prefer the (much simpler and straightforward IMHO) code that I proposed in a previous comment:\n\n     for k, v in ex_specs.items():\n         if k.startswith(\u0027resources\u0027):\n             rg_id \u003d k[9:] or \u00270\u0027\n             rg \u003d get_resource_group(rg_id)\n             rc \u003d k.split(\u0027:\u0027)[1]\n             rg.resources[rc] \u003d int(v)\n         elif k.startswith(\u0027trait\u0027):\n             rg_id \u003d k[5:] or \u00270\u0027\n             rg \u003d get_resource_group(rg_id)\n             trait \u003d k.split(\u0027:\u0027)[1]\n             rg.required_traits.add(trait)\n         else:\n             continue\n\n     return resource_groups.values()","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"758c5ecc8d352e2efe435b174a73573a547f02dc","unresolved":false,"context_lines":[{"line_number":135,"context_line":"                ret.get_resource_group(suffix).add_resource(name, amt)"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"            # Process \"trait[$N]\""},{"line_number":138,"context_line":"            elif prefix \u003d\u003d trait_prefix:"},{"line_number":139,"context_line":"                # It\u0027s a trait.  Ensure the value is supported."},{"line_number":140,"context_line":"                if val not in trait_vals:"},{"line_number":141,"context_line":"                    LOG.warning("}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_c5caf118","line":138,"in_reply_to":"3f4b6375_cf6332b1","updated":"2017-10-30 23:00:57.000000000","message":"Okay, will do.","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"758c5ecc8d352e2efe435b174a73573a547f02dc","unresolved":false,"context_lines":[{"line_number":135,"context_line":"                ret.get_resource_group(suffix).add_resource(name, amt)"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"            # Process \"trait[$N]\""},{"line_number":138,"context_line":"            elif prefix \u003d\u003d trait_prefix:"},{"line_number":139,"context_line":"                # It\u0027s a trait.  Ensure the value is supported."},{"line_number":140,"context_line":"                if val not in trait_vals:"},{"line_number":141,"context_line":"                    LOG.warning("}],"source_content_type":"text/x-python","patch_set":7,"id":"3f4b6375_63c68e19","line":138,"in_reply_to":"3f4b6375_d909b895","updated":"2017-10-30 23:00:57.000000000","message":"Sorry, how is it simpler?  The string parsing is sixes, mostly a matter of preference (e.g. I personally don\u0027t like hardcoding prefix lengths).  The LOC difference is just the validation.\n\nI\u0027ll refactor to consolidate these chunks with the ResourceGroup.add_* methods.","commit_id":"6b3cb609d40dfcf1304cb2720fc43653b94d73f3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"abce8cff6965014ab99fc45b2cc4559e0de2b57a","unresolved":false,"context_lines":[{"line_number":48,"context_line":"    def __init__(self, use_same_provider):"},{"line_number":49,"context_line":"        \"\"\"Create an initially empty grouping of resource and trait requests."},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"        :param use_same_provider: If True (the default), this RequestGroup"},{"line_number":52,"context_line":"                                  it represents requests for resources and"},{"line_number":53,"context_line":"                                  traits which must be satisfied by a single"},{"line_number":54,"context_line":"                                  resource provider.  If False, represents a"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f4b6375_e6ea7ad6","line":51,"range":{"start_line":51,"start_character":43,"end_line":51,"end_character":54},"updated":"2017-10-31 10:22:22.000000000","message":"use_same_provider is not defaulted to True in the signature.","commit_id":"0cedbc36859c2a8ad71dea5c4cf21f84154d5841"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"68915e0752599ad61c5acf1c34034155f8eed058","unresolved":false,"context_lines":[{"line_number":48,"context_line":"    def __init__(self, use_same_provider):"},{"line_number":49,"context_line":"        \"\"\"Create an initially empty grouping of resource and trait requests."},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"        :param use_same_provider: If True (the default), this RequestGroup"},{"line_number":52,"context_line":"                                  it represents requests for resources and"},{"line_number":53,"context_line":"                                  traits which must be satisfied by a single"},{"line_number":54,"context_line":"                                  resource provider.  If False, represents a"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f4b6375_76163558","line":51,"range":{"start_line":51,"start_character":43,"end_line":51,"end_character":54},"in_reply_to":"3f4b6375_e6ea7ad6","updated":"2017-10-31 13:47:28.000000000","message":"Whoops, interim sandbox version, will fix.","commit_id":"0cedbc36859c2a8ad71dea5c4cf21f84154d5841"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"abce8cff6965014ab99fc45b2cc4559e0de2b57a","unresolved":false,"context_lines":[{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    def get_resource_group(self, ident):"},{"line_number":71,"context_line":"        if ident not in self._rg_by_id:"},{"line_number":72,"context_line":"            self._rg_by_id[ident] \u003d RequestGroup(bool(ident))"},{"line_number":73,"context_line":"        return self._rg_by_id[ident]"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def _add_resource(self, groupid, rclass, amount):"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f4b6375_06f356a0","line":72,"range":{"start_line":72,"start_character":27,"end_line":72,"end_character":32},"updated":"2017-10-31 10:22:22.000000000","message":"As far as I see this is None now for \u0027resources\u0027 group and it was 0 before. Not that it is wrong I just realized it.","commit_id":"0cedbc36859c2a8ad71dea5c4cf21f84154d5841"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"68915e0752599ad61c5acf1c34034155f8eed058","unresolved":false,"context_lines":[{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    def get_resource_group(self, ident):"},{"line_number":71,"context_line":"        if ident not in self._rg_by_id:"},{"line_number":72,"context_line":"            self._rg_by_id[ident] \u003d RequestGroup(bool(ident))"},{"line_number":73,"context_line":"        return self._rg_by_id[ident]"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def _add_resource(self, groupid, rclass, amount):"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f4b6375_4cae0ce8","line":72,"range":{"start_line":72,"start_character":27,"end_line":72,"end_character":32},"in_reply_to":"3f4b6375_06f356a0","updated":"2017-10-31 13:47:28.000000000","message":"Yup.  None is a valid dict key, and it saves me a little bit of converting from one \"special\" value to another.","commit_id":"0cedbc36859c2a8ad71dea5c4cf21f84154d5841"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"8ff5af6c98db88a812bce997020a61245b40c29d","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                 \"val\": trait_type})"},{"line_number":106,"context_line":"            return"},{"line_number":107,"context_line":"        # Validate the trait name"},{"line_number":108,"context_line":"        if not (os_traits.is_custom(trait_name)"},{"line_number":109,"context_line":"                or os_traits.check_traits([trait_name])[0]):"},{"line_number":110,"context_line":"            LOG.warning("},{"line_number":111,"context_line":"                \"Received an invalid Trait \u0027%s\u0027 in extra_specs.\", trait_name)"},{"line_number":112,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f4b6375_bbdc9b18","line":109,"range":{"start_line":108,"start_character":8,"end_line":109,"end_character":60},"updated":"2017-10-31 09:28:37.000000000","message":"Why we only check the valid for standard trait? A custom trait doesn\u0027t mean it is valid also.\n\nActually I think we needn\u0027t to check the valid of the trait for every request.\n\nIf the admin user create a flavor with invalid trait, when booting the instance, the placement will return fault about the invalid trait, then the admin user will know that.","commit_id":"0cedbc36859c2a8ad71dea5c4cf21f84154d5841"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"68915e0752599ad61c5acf1c34034155f8eed058","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                 \"val\": trait_type})"},{"line_number":106,"context_line":"            return"},{"line_number":107,"context_line":"        # Validate the trait name"},{"line_number":108,"context_line":"        if not (os_traits.is_custom(trait_name)"},{"line_number":109,"context_line":"                or os_traits.check_traits([trait_name])[0]):"},{"line_number":110,"context_line":"            LOG.warning("},{"line_number":111,"context_line":"                \"Received an invalid Trait \u0027%s\u0027 in extra_specs.\", trait_name)"},{"line_number":112,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f4b6375_d6ef2985","line":109,"range":{"start_line":108,"start_character":8,"end_line":109,"end_character":60},"in_reply_to":"3f4b6375_266e5296","updated":"2017-10-31 13:47:28.000000000","message":"Yeah, this is a deliberate compromise.\n\nValidating on flavor creation would be possible, but not super useful, because it only checks against the state of things at that moment.  Custom traits could get added or deleted subsequently, and we\u0027re certainly not going to go around validating existing flavors any time that happens.\n\nAnd as Alex says, they\u0027ll all get validated for real against registered values in placement during the GET /allocation_candidates call this is being built for.\n\nSo here we\u0027re only doing what we can get away with for really cheap.  For standard traits, we can validate against os-traits without going across the wire.  For custom traits, all we can do is ensure they have the CUSTOM_ prefix.","commit_id":"0cedbc36859c2a8ad71dea5c4cf21f84154d5841"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"abce8cff6965014ab99fc45b2cc4559e0de2b57a","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                 \"val\": trait_type})"},{"line_number":106,"context_line":"            return"},{"line_number":107,"context_line":"        # Validate the trait name"},{"line_number":108,"context_line":"        if not (os_traits.is_custom(trait_name)"},{"line_number":109,"context_line":"                or os_traits.check_traits([trait_name])[0]):"},{"line_number":110,"context_line":"            LOG.warning("},{"line_number":111,"context_line":"                \"Received an invalid Trait \u0027%s\u0027 in extra_specs.\", trait_name)"},{"line_number":112,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f4b6375_266e5296","line":109,"range":{"start_line":108,"start_character":8,"end_line":109,"end_character":60},"in_reply_to":"3f4b6375_bbdc9b18","updated":"2017-10-31 10:22:22.000000000","message":"Or we could check the validity of the trait when it is set in the extra_spec. That way we can even provide info to the admin about the validity.","commit_id":"0cedbc36859c2a8ad71dea5c4cf21f84154d5841"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"8ff5af6c98db88a812bce997020a61245b40c29d","unresolved":false,"context_lines":[{"line_number":127,"context_line":"                 required traits in the extra_specs."},{"line_number":128,"context_line":"        \"\"\""},{"line_number":129,"context_line":"        res_prefix \u003d \u0027resources\u0027"},{"line_number":130,"context_line":"        trait_prefix \u003d \u0027trait\u0027"},{"line_number":131,"context_line":"        # Regex patterns for numbered or un-numbered resources/trait keys"},{"line_number":132,"context_line":"        keypat \u003d re.compile(r\"^(%s)([1-9][0-9]*)?:(.*)$\" %"},{"line_number":133,"context_line":"                            \u0027|\u0027.join((res_prefix, trait_prefix)))"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f4b6375_40dc9673","line":130,"range":{"start_line":130,"start_character":24,"end_line":130,"end_character":29},"updated":"2017-10-31 09:28:37.000000000","message":"required","commit_id":"0cedbc36859c2a8ad71dea5c4cf21f84154d5841"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"abce8cff6965014ab99fc45b2cc4559e0de2b57a","unresolved":false,"context_lines":[{"line_number":127,"context_line":"                 required traits in the extra_specs."},{"line_number":128,"context_line":"        \"\"\""},{"line_number":129,"context_line":"        res_prefix \u003d \u0027resources\u0027"},{"line_number":130,"context_line":"        trait_prefix \u003d \u0027trait\u0027"},{"line_number":131,"context_line":"        # Regex patterns for numbered or un-numbered resources/trait keys"},{"line_number":132,"context_line":"        keypat \u003d re.compile(r\"^(%s)([1-9][0-9]*)?:(.*)$\" %"},{"line_number":133,"context_line":"                            \u0027|\u0027.join((res_prefix, trait_prefix)))"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f4b6375_66432a15","line":130,"range":{"start_line":130,"start_character":24,"end_line":130,"end_character":29},"in_reply_to":"3f4b6375_40dc9673","updated":"2017-10-31 10:22:22.000000000","message":"In the flavor extra_spec the prefix of the traits are \u0027trait\u0027 as per the spec [1] and the value is \u0027required\u0027\n\n[1] https://specs.openstack.org/openstack/nova-specs/specs/queens/approved/request-traits-in-nova.html","commit_id":"0cedbc36859c2a8ad71dea5c4cf21f84154d5841"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"68915e0752599ad61c5acf1c34034155f8eed058","unresolved":false,"context_lines":[{"line_number":127,"context_line":"                 required traits in the extra_specs."},{"line_number":128,"context_line":"        \"\"\""},{"line_number":129,"context_line":"        res_prefix \u003d \u0027resources\u0027"},{"line_number":130,"context_line":"        trait_prefix \u003d \u0027trait\u0027"},{"line_number":131,"context_line":"        # Regex patterns for numbered or un-numbered resources/trait keys"},{"line_number":132,"context_line":"        keypat \u003d re.compile(r\"^(%s)([1-9][0-9]*)?:(.*)$\" %"},{"line_number":133,"context_line":"                            \u0027|\u0027.join((res_prefix, trait_prefix)))"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f4b6375_f628e597","line":130,"range":{"start_line":130,"start_character":24,"end_line":130,"end_character":29},"in_reply_to":"3f4b6375_66432a15","updated":"2017-10-31 13:47:28.000000000","message":"Yuh.  Confused yet?","commit_id":"0cedbc36859c2a8ad71dea5c4cf21f84154d5841"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"8ff5af6c98db88a812bce997020a61245b40c29d","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        res_prefix \u003d \u0027resources\u0027"},{"line_number":130,"context_line":"        trait_prefix \u003d \u0027trait\u0027"},{"line_number":131,"context_line":"        # Regex patterns for numbered or un-numbered resources/trait keys"},{"line_number":132,"context_line":"        keypat \u003d re.compile(r\"^(%s)([1-9][0-9]*)?:(.*)$\" %"},{"line_number":133,"context_line":"                            \u0027|\u0027.join((res_prefix, trait_prefix)))"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        ret \u003d cls()"},{"line_number":136,"context_line":"        for key, val in extra_specs.items():"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f4b6375_e08b8265","line":133,"range":{"start_line":132,"start_character":17,"end_line":133,"end_character":65},"updated":"2017-10-31 09:28:37.000000000","message":"we needn\u0027t compile it in every requests.","commit_id":"0cedbc36859c2a8ad71dea5c4cf21f84154d5841"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"68915e0752599ad61c5acf1c34034155f8eed058","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        res_prefix \u003d \u0027resources\u0027"},{"line_number":130,"context_line":"        trait_prefix \u003d \u0027trait\u0027"},{"line_number":131,"context_line":"        # Regex patterns for numbered or un-numbered resources/trait keys"},{"line_number":132,"context_line":"        keypat \u003d re.compile(r\"^(%s)([1-9][0-9]*)?:(.*)$\" %"},{"line_number":133,"context_line":"                            \u0027|\u0027.join((res_prefix, trait_prefix)))"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        ret \u003d cls()"},{"line_number":136,"context_line":"        for key, val in extra_specs.items():"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f4b6375_d6196988","line":133,"range":{"start_line":132,"start_character":17,"end_line":133,"end_character":65},"in_reply_to":"3f4b6375_867d0670","updated":"2017-10-31 13:47:28.000000000","message":"Good call, will fix.","commit_id":"0cedbc36859c2a8ad71dea5c4cf21f84154d5841"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"abce8cff6965014ab99fc45b2cc4559e0de2b57a","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        res_prefix \u003d \u0027resources\u0027"},{"line_number":130,"context_line":"        trait_prefix \u003d \u0027trait\u0027"},{"line_number":131,"context_line":"        # Regex patterns for numbered or un-numbered resources/trait keys"},{"line_number":132,"context_line":"        keypat \u003d re.compile(r\"^(%s)([1-9][0-9]*)?:(.*)$\" %"},{"line_number":133,"context_line":"                            \u0027|\u0027.join((res_prefix, trait_prefix)))"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        ret \u003d cls()"},{"line_number":136,"context_line":"        for key, val in extra_specs.items():"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f4b6375_867d0670","line":133,"range":{"start_line":132,"start_character":17,"end_line":133,"end_character":65},"in_reply_to":"3f4b6375_e08b8265","updated":"2017-10-31 10:22:22.000000000","message":"the re module already has a caching mechanism of frequently used patterns so we don\u0027t even need to compile it I guess. But if compile it then I agree with Alex to compile it once in class level.\n\n[1] https://docs.python.org/3/library/re.html#re.compile","commit_id":"0cedbc36859c2a8ad71dea5c4cf21f84154d5841"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2350d799d2aef53973fa67cbfafae2600104d965","unresolved":false,"context_lines":[{"line_number":24,"context_line":"import oslo_messaging as messaging"},{"line_number":25,"context_line":"from oslo_serialization import jsonutils"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"from nova.api.openstack.placement import lib as plib"},{"line_number":28,"context_line":"from nova.compute import flavors"},{"line_number":29,"context_line":"from nova.compute import utils as compute_utils"},{"line_number":30,"context_line":"import nova.conf"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f4b6375_c30b9b27","line":27,"range":{"start_line":27,"start_character":48,"end_line":27,"end_character":52},"updated":"2017-11-02 18:57:43.000000000","message":"how about placement_lib?","commit_id":"549ef958e9b388aae474a17b28c6588e9a918e9d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"40ba26b0b79dae3933e5b924ab8e9787d429d17c","unresolved":false,"context_lines":[{"line_number":24,"context_line":"import oslo_messaging as messaging"},{"line_number":25,"context_line":"from oslo_serialization import jsonutils"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"from nova.api.openstack.placement import lib as plib"},{"line_number":28,"context_line":"from nova.compute import flavors"},{"line_number":29,"context_line":"from nova.compute import utils as compute_utils"},{"line_number":30,"context_line":"import nova.conf"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f4b6375_b739f5d7","line":27,"range":{"start_line":27,"start_character":48,"end_line":27,"end_character":52},"in_reply_to":"3f4b6375_c30b9b27","updated":"2017-11-02 20:46:25.000000000","message":"Done","commit_id":"549ef958e9b388aae474a17b28c6588e9a918e9d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2350d799d2aef53973fa67cbfafae2600104d965","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        # { ident: RequestGroup }"},{"line_number":59,"context_line":"        self._rg_by_id \u003d {}"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def get_resource_group(self, ident):"},{"line_number":62,"context_line":"        if ident not in self._rg_by_id:"},{"line_number":63,"context_line":"            self._rg_by_id[ident] \u003d plib.RequestGroup("},{"line_number":64,"context_line":"                use_same_provider\u003dbool(ident))"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f4b6375_e30e9739","line":61,"range":{"start_line":61,"start_character":12,"end_line":61,"end_character":26},"updated":"2017-11-02 18:57:43.000000000","message":"just noticed, this should be get_request_group(), not get_resource_group()","commit_id":"549ef958e9b388aae474a17b28c6588e9a918e9d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"40ba26b0b79dae3933e5b924ab8e9787d429d17c","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        # { ident: RequestGroup }"},{"line_number":59,"context_line":"        self._rg_by_id \u003d {}"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def get_resource_group(self, ident):"},{"line_number":62,"context_line":"        if ident not in self._rg_by_id:"},{"line_number":63,"context_line":"            self._rg_by_id[ident] \u003d plib.RequestGroup("},{"line_number":64,"context_line":"                use_same_provider\u003dbool(ident))"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f4b6375_772ffd87","line":61,"range":{"start_line":61,"start_character":12,"end_line":61,"end_character":26},"in_reply_to":"3f4b6375_e30e9739","updated":"2017-11-02 20:46:25.000000000","message":"Done","commit_id":"549ef958e9b388aae474a17b28c6588e9a918e9d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2350d799d2aef53973fa67cbfafae2600104d965","unresolved":false,"context_lines":[{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def get_resource_group(self, ident):"},{"line_number":62,"context_line":"        if ident not in self._rg_by_id:"},{"line_number":63,"context_line":"            self._rg_by_id[ident] \u003d plib.RequestGroup("},{"line_number":64,"context_line":"                use_same_provider\u003dbool(ident))"},{"line_number":65,"context_line":"        return self._rg_by_id[ident]"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def _add_resource(self, groupid, rclass, amount):"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f4b6375_66299588","line":64,"range":{"start_line":63,"start_character":13,"end_line":64,"end_character":46},"updated":"2017-11-02 18:57:43.000000000","message":"how about:\n\n rg \u003d placement_lib.RequestGroup(use_same_provider\u003dbool(ident))\n self._rg_by_id[ident] \u003d rg","commit_id":"549ef958e9b388aae474a17b28c6588e9a918e9d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"40ba26b0b79dae3933e5b924ab8e9787d429d17c","unresolved":false,"context_lines":[{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def get_resource_group(self, ident):"},{"line_number":62,"context_line":"        if ident not in self._rg_by_id:"},{"line_number":63,"context_line":"            self._rg_by_id[ident] \u003d plib.RequestGroup("},{"line_number":64,"context_line":"                use_same_provider\u003dbool(ident))"},{"line_number":65,"context_line":"        return self._rg_by_id[ident]"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def _add_resource(self, groupid, rclass, amount):"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f4b6375_175449fd","line":64,"range":{"start_line":63,"start_character":13,"end_line":64,"end_character":46},"in_reply_to":"3f4b6375_66299588","updated":"2017-11-02 20:46:25.000000000","message":"Done","commit_id":"549ef958e9b388aae474a17b28c6588e9a918e9d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"65016397dd34a7cd2f909d072227ad84c85c1651","unresolved":false,"context_lines":[{"line_number":97,"context_line":"                 \"val\": trait_type})"},{"line_number":98,"context_line":"            return"},{"line_number":99,"context_line":"        # Validate the trait name"},{"line_number":100,"context_line":"        if not (os_traits.is_custom(trait_name)"},{"line_number":101,"context_line":"                or os_traits.check_traits([trait_name])[0]):"},{"line_number":102,"context_line":"            LOG.warning("},{"line_number":103,"context_line":"                \"Received an invalid Trait \u0027%s\u0027 in extra_specs.\", trait_name)"},{"line_number":104,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f4b6375_632ce701","line":101,"range":{"start_line":100,"start_character":0,"end_line":101,"end_character":60},"updated":"2017-11-02 18:50:34.000000000","message":"how about this instead?\n\n if (not os_traits.is_custom(trait_name) and\n         not os_traits.check_traits([trait_name])[1]):\n     ...\n\nos_traits.check_traits() returns a two-tuple with the second element of the tuple being *invalid* traits.","commit_id":"549ef958e9b388aae474a17b28c6588e9a918e9d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d40cd02c0c88e2b4d405fc5f91e26943d8a9a091","unresolved":false,"context_lines":[{"line_number":97,"context_line":"                 \"val\": trait_type})"},{"line_number":98,"context_line":"            return"},{"line_number":99,"context_line":"        # Validate the trait name"},{"line_number":100,"context_line":"        if not (os_traits.is_custom(trait_name)"},{"line_number":101,"context_line":"                or os_traits.check_traits([trait_name])[0]):"},{"line_number":102,"context_line":"            LOG.warning("},{"line_number":103,"context_line":"                \"Received an invalid Trait \u0027%s\u0027 in extra_specs.\", trait_name)"},{"line_number":104,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f4b6375_d0046617","line":101,"range":{"start_line":100,"start_character":0,"end_line":101,"end_character":60},"in_reply_to":"3f4b6375_1757a9cc","updated":"2017-11-03 14:38:01.000000000","message":"Because [1] is the *invalid* traits.","commit_id":"549ef958e9b388aae474a17b28c6588e9a918e9d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"40ba26b0b79dae3933e5b924ab8e9787d429d17c","unresolved":false,"context_lines":[{"line_number":97,"context_line":"                 \"val\": trait_type})"},{"line_number":98,"context_line":"            return"},{"line_number":99,"context_line":"        # Validate the trait name"},{"line_number":100,"context_line":"        if not (os_traits.is_custom(trait_name)"},{"line_number":101,"context_line":"                or os_traits.check_traits([trait_name])[0]):"},{"line_number":102,"context_line":"            LOG.warning("},{"line_number":103,"context_line":"                \"Received an invalid Trait \u0027%s\u0027 in extra_specs.\", trait_name)"},{"line_number":104,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f4b6375_1757a9cc","line":101,"range":{"start_line":100,"start_character":0,"end_line":101,"end_character":60},"in_reply_to":"3f4b6375_632ce701","updated":"2017-11-02 20:46:25.000000000","message":"Those seem the same to me.  Why is one better?","commit_id":"549ef958e9b388aae474a17b28c6588e9a918e9d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"7d8e495bff5cc7045b67f1df1031822efefe401d","unresolved":false,"context_lines":[{"line_number":97,"context_line":"                 \"val\": trait_type})"},{"line_number":98,"context_line":"            return"},{"line_number":99,"context_line":"        # Validate the trait name"},{"line_number":100,"context_line":"        if not (os_traits.is_custom(trait_name)"},{"line_number":101,"context_line":"                or os_traits.check_traits([trait_name])[0]):"},{"line_number":102,"context_line":"            LOG.warning("},{"line_number":103,"context_line":"                \"Received an invalid Trait \u0027%s\u0027 in extra_specs.\", trait_name)"},{"line_number":104,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f4b6375_b0ae8a7e","line":101,"range":{"start_line":100,"start_character":0,"end_line":101,"end_character":60},"in_reply_to":"3f4b6375_d0046617","updated":"2017-11-03 14:54:35.000000000","message":"Right; so this code (which is now gone, so moot anyway) was saying:\n\n if not (valid_this_way or valid_that_way)\n\nand you were proposing:\n\n if (not valid_this_way and not invalid_that_way)\n\nthough you probably meant\n\n if (not valid_this_way or invalid_that_way)\n\nwhich is logically the same, but (IMO) harder to read.","commit_id":"549ef958e9b388aae474a17b28c6588e9a918e9d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"65016397dd34a7cd2f909d072227ad84c85c1651","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        # Validate the trait name"},{"line_number":100,"context_line":"        if not (os_traits.is_custom(trait_name)"},{"line_number":101,"context_line":"                or os_traits.check_traits([trait_name])[0]):"},{"line_number":102,"context_line":"            LOG.warning("},{"line_number":103,"context_line":"                \"Received an invalid Trait \u0027%s\u0027 in extra_specs.\", trait_name)"},{"line_number":104,"context_line":"            return"},{"line_number":105,"context_line":"        self.get_resource_group(groupid).required_traits.add(trait_name)"},{"line_number":106,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3f4b6375_035e5375","line":103,"range":{"start_line":102,"start_character":0,"end_line":103,"end_character":77},"updated":"2017-11-02 18:50:34.000000000","message":"The issue I have with doing this check on the client side is that the Placement API could potentially have a different version of os_traits installed, which may or may not consider the trait to be valid.\n\nI\u0027d almost prefer to just remove the validation of traits here and rely on the Placement server returning an error about an unknown trait name and handling it wherever we call the GET /allocation_candidates API call (currently that\u0027s in the scheduler/client/report.py\u0027s SchedulerReportClient.get_allocation_candidates() method)","commit_id":"549ef958e9b388aae474a17b28c6588e9a918e9d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"40ba26b0b79dae3933e5b924ab8e9787d429d17c","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        # Validate the trait name"},{"line_number":100,"context_line":"        if not (os_traits.is_custom(trait_name)"},{"line_number":101,"context_line":"                or os_traits.check_traits([trait_name])[0]):"},{"line_number":102,"context_line":"            LOG.warning("},{"line_number":103,"context_line":"                \"Received an invalid Trait \u0027%s\u0027 in extra_specs.\", trait_name)"},{"line_number":104,"context_line":"            return"},{"line_number":105,"context_line":"        self.get_resource_group(groupid).required_traits.add(trait_name)"},{"line_number":106,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3f4b6375_77b25d4b","line":103,"range":{"start_line":102,"start_character":0,"end_line":103,"end_character":77},"in_reply_to":"3f4b6375_035e5375","updated":"2017-11-02 20:46:25.000000000","message":"Can the same be said for the ResourceClass check above (which I did not introduce, just ported)?  Want me to rip that out too?","commit_id":"549ef958e9b388aae474a17b28c6588e9a918e9d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e869af631a3a85bae6f7a93a1774a4294a2290e6","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        # Validate the trait name"},{"line_number":100,"context_line":"        if not (os_traits.is_custom(trait_name)"},{"line_number":101,"context_line":"                or os_traits.check_traits([trait_name])[0]):"},{"line_number":102,"context_line":"            LOG.warning("},{"line_number":103,"context_line":"                \"Received an invalid Trait \u0027%s\u0027 in extra_specs.\", trait_name)"},{"line_number":104,"context_line":"            return"},{"line_number":105,"context_line":"        self.get_resource_group(groupid).required_traits.add(trait_name)"},{"line_number":106,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3f4b6375_a48bf3bd","line":103,"range":{"start_line":102,"start_character":0,"end_line":103,"end_character":77},"in_reply_to":"3f4b6375_77b25d4b","updated":"2017-11-03 13:28:27.000000000","message":"Ripped out the traits check for now; let me know if you want me to take out the ResourceClass check as well.","commit_id":"549ef958e9b388aae474a17b28c6588e9a918e9d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d40cd02c0c88e2b4d405fc5f91e26943d8a9a091","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        # Validate the trait name"},{"line_number":100,"context_line":"        if not (os_traits.is_custom(trait_name)"},{"line_number":101,"context_line":"                or os_traits.check_traits([trait_name])[0]):"},{"line_number":102,"context_line":"            LOG.warning("},{"line_number":103,"context_line":"                \"Received an invalid Trait \u0027%s\u0027 in extra_specs.\", trait_name)"},{"line_number":104,"context_line":"            return"},{"line_number":105,"context_line":"        self.get_resource_group(groupid).required_traits.add(trait_name)"},{"line_number":106,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3f4b6375_900a6e1d","line":103,"range":{"start_line":102,"start_character":0,"end_line":103,"end_character":77},"in_reply_to":"3f4b6375_a48bf3bd","updated":"2017-11-03 14:38:01.000000000","message":"Cool, thanks","commit_id":"549ef958e9b388aae474a17b28c6588e9a918e9d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"65016397dd34a7cd2f909d072227ad84c85c1651","unresolved":false,"context_lines":[{"line_number":116,"context_line":""},{"line_number":117,"context_line":"        :param extra_specs: The flavor extra_specs dict."},{"line_number":118,"context_line":"        :return: A ResourceRequest object representing the resources and"},{"line_number":119,"context_line":"                 required traits in the extra_specs."},{"line_number":120,"context_line":"        \"\"\""},{"line_number":121,"context_line":"        ret \u003d cls()"},{"line_number":122,"context_line":"        for key, val in extra_specs.items():"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f4b6375_834243aa","line":119,"updated":"2017-11-02 18:50:34.000000000","message":"Might want to add:\n\n :raises ValueError: when resource quantities are invalid","commit_id":"549ef958e9b388aae474a17b28c6588e9a918e9d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"40ba26b0b79dae3933e5b924ab8e9787d429d17c","unresolved":false,"context_lines":[{"line_number":116,"context_line":""},{"line_number":117,"context_line":"        :param extra_specs: The flavor extra_specs dict."},{"line_number":118,"context_line":"        :return: A ResourceRequest object representing the resources and"},{"line_number":119,"context_line":"                 required traits in the extra_specs."},{"line_number":120,"context_line":"        \"\"\""},{"line_number":121,"context_line":"        ret \u003d cls()"},{"line_number":122,"context_line":"        for key, val in extra_specs.items():"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f4b6375_b7c33589","line":119,"in_reply_to":"3f4b6375_834243aa","updated":"2017-11-02 20:46:25.000000000","message":"It doesn\u0027t.","commit_id":"549ef958e9b388aae474a17b28c6588e9a918e9d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d40cd02c0c88e2b4d405fc5f91e26943d8a9a091","unresolved":false,"context_lines":[{"line_number":116,"context_line":""},{"line_number":117,"context_line":"        :param extra_specs: The flavor extra_specs dict."},{"line_number":118,"context_line":"        :return: A ResourceRequest object representing the resources and"},{"line_number":119,"context_line":"                 required traits in the extra_specs."},{"line_number":120,"context_line":"        \"\"\""},{"line_number":121,"context_line":"        ret \u003d cls()"},{"line_number":122,"context_line":"        for key, val in extra_specs.items():"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f4b6375_b01e8a5c","line":119,"in_reply_to":"3f4b6375_b7c33589","updated":"2017-11-03 14:38:01.000000000","message":"Ah, sorry, missed the internal catch of ValueError and logging that...","commit_id":"549ef958e9b388aae474a17b28c6588e9a918e9d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b8c42b7b2eae508ac304f531896ba82180ac2aeb","unresolved":false,"context_lines":[{"line_number":228,"context_line":"                continue"},{"line_number":229,"context_line":"            resources[custom_key] \u003d custom_val"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    # Accept all standard resource classes to be overrided whether they exist"},{"line_number":232,"context_line":"    # as flavor fields or not."},{"line_number":233,"context_line":"    for std_key in std_keys:"},{"line_number":234,"context_line":"        if std_key not in set(resources).union(fields.ResourceClass.STANDARD):"},{"line_number":235,"context_line":"            LOG.warning(\"Received an invalid ResourceClass \u0027%(key)s\u0027 in \""},{"line_number":236,"context_line":"                    \"extra_specs.\", {\"key\": std_key})"},{"line_number":237,"context_line":"            continue"},{"line_number":238,"context_line":"        val \u003d validate_int(std_key)"},{"line_number":239,"context_line":"        if val is None:"},{"line_number":240,"context_line":"            # Received an invalid amount. It\u0027s already logged, so move on."}],"source_content_type":"text/x-python","patch_set":11,"id":"3f4b6375_c91deabe","line":237,"range":{"start_line":231,"start_character":0,"end_line":237,"end_character":20},"updated":"2017-11-03 12:35:20.000000000","message":"Here\u0027s where I ported that check from, btw.","commit_id":"549ef958e9b388aae474a17b28c6588e9a918e9d"}],"nova/tests/unit/scheduler/test_utils.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"dc52b433b65927282b3614305ace0230d37d15ee","unresolved":false,"context_lines":[{"line_number":253,"context_line":"            \u0027resources2:IPV4_ADDRESS\u0027: \u00272\u0027,"},{"line_number":254,"context_line":"            \u0027trait2:CUSTOM_PHYSNET_NET2\u0027: \u0027required\u0027,"},{"line_number":255,"context_line":"            \u0027trait2:HW_NIC_ACCEL_SSL\u0027: \u0027required\u0027,"},{"line_number":256,"context_line":"            \u0027resources3:DISK_GB\u0027: \u00275\u0027,"},{"line_number":257,"context_line":"        }"},{"line_number":258,"context_line":"        expected \u003d {"},{"line_number":259,"context_line":"            \u0027resources\u0027: {"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f4b6375_0ea75abf","line":256,"updated":"2017-10-26 11:50:27.000000000","message":"for sake of completeness, add traitMoo and resourcesBar?","commit_id":"aa440fe957b69a1a173051e7348f708cee7134af"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6db1100e118f62f6bd92ed389be1d2472f0324ec","unresolved":false,"context_lines":[{"line_number":253,"context_line":"            \u0027resources2:IPV4_ADDRESS\u0027: \u00272\u0027,"},{"line_number":254,"context_line":"            \u0027trait2:CUSTOM_PHYSNET_NET2\u0027: \u0027required\u0027,"},{"line_number":255,"context_line":"            \u0027trait2:HW_NIC_ACCEL_SSL\u0027: \u0027required\u0027,"},{"line_number":256,"context_line":"            \u0027resources3:DISK_GB\u0027: \u00275\u0027,"},{"line_number":257,"context_line":"        }"},{"line_number":258,"context_line":"        expected \u003d {"},{"line_number":259,"context_line":"            \u0027resources\u0027: {"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f4b6375_ff1946d6","line":256,"in_reply_to":"3f4b6375_0ea75abf","updated":"2017-10-26 13:38:20.000000000","message":"Good call, done.","commit_id":"aa440fe957b69a1a173051e7348f708cee7134af"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"30678dcb0a67adeb5c55f799d7afbc8fb0828590","unresolved":false,"context_lines":[{"line_number":287,"context_line":"                \u0027DISK_GB\u0027: 5,"},{"line_number":288,"context_line":"            },"},{"line_number":289,"context_line":"        }"},{"line_number":290,"context_line":"        self.assertEqual(expected,"},{"line_number":291,"context_line":"                         utils._process_extra_specs_granular(extra_specs))"},{"line_number":292,"context_line":""},{"line_number":293,"context_line":"    def test_merge_resources(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4b6375_4901c537","line":290,"range":{"start_line":290,"start_character":13,"end_line":290,"end_character":24},"updated":"2017-10-26 17:27:08.000000000","message":"nit: assertDictEqual() gives better error message when the expected value does not equal to the actual one.","commit_id":"c81208643ddd1c58655d718efada0c8d9dafebed"}]}