)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dd896bfaca4d4100e0d3482ee8f5b5a2ab2f8d6a","unresolved":false,"context_lines":[{"line_number":10,"context_line":"beginning with \"resources:\", and if found, use them to update the"},{"line_number":11,"context_line":"resources dict sent to placement."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"The entry for a custom resource class will look like"},{"line_number":14,"context_line":"\"resources:CUSTOM_FOO\u003d1\". Additionally, standard classes in a flavor can"},{"line_number":15,"context_line":"be overridden with an entry that looks like: \"resources:VCPU\u003d0\". If a"},{"line_number":16,"context_line":"standard class is found in extra_specs, it will be used instead of the"},{"line_number":17,"context_line":"amount in the flavor. This is useful for things like Ironic, where an"},{"line_number":18,"context_line":"operator may want to list the amount of RAM, disk, etc. in the flavor,"},{"line_number":19,"context_line":"but use a custom Ironic resource class for doing the actual selection."},{"line_number":20,"context_line":"An amount for a standard class that is zero will result in that class"},{"line_number":21,"context_line":"being removed from the requested resources dict sent to placement."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Blueprint: custom-resource-classes-in-flavors"},{"line_number":24,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"7f231b9d_0565ee45","line":21,"range":{"start_line":13,"start_character":0,"end_line":21,"end_character":66},"updated":"2017-06-16 19:38:56.000000000","message":"We should put a DocImpact tag on this so it generates a bug, because we\u0027re going to want to document this in:\n\nhttps://docs.openstack.org/admin-guide/compute-flavors.html#extra-specs\n\nSince it\u0027s complicated.","commit_id":"7b7df64a37911682e71b67d79470f6c09c801ec4"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"c3b9d6d79b81a0da18755529a6ac9c58fd13ea7c","unresolved":false,"context_lines":[{"line_number":10,"context_line":"beginning with \"resources:\", and if found, use them to update the"},{"line_number":11,"context_line":"resources dict sent to placement."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"The entry for a custom resource class will look like"},{"line_number":14,"context_line":"\"resources:CUSTOM_FOO\u003d1\". Additionally, standard classes in a flavor can"},{"line_number":15,"context_line":"be overridden with an entry that looks like: \"resources:VCPU\u003d0\". If a"},{"line_number":16,"context_line":"standard class is found in extra_specs, it will be used instead of the"},{"line_number":17,"context_line":"amount in the flavor. This is useful for things like Ironic, where an"},{"line_number":18,"context_line":"operator may want to list the amount of RAM, disk, etc. in the flavor,"},{"line_number":19,"context_line":"but use a custom Ironic resource class for doing the actual selection."},{"line_number":20,"context_line":"An amount for a standard class that is zero will result in that class"},{"line_number":21,"context_line":"being removed from the requested resources dict sent to placement."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Blueprint: custom-resource-classes-in-flavors"},{"line_number":24,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"7f231b9d_b0555afc","line":21,"range":{"start_line":13,"start_character":0,"end_line":21,"end_character":66},"in_reply_to":"7f231b9d_0565ee45","updated":"2017-06-16 20:16:28.000000000","message":"Done","commit_id":"7b7df64a37911682e71b67d79470f6c09c801ec4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"efa18614396f4987ae78a098a5af14d4190eb799","unresolved":false,"context_lines":[{"line_number":20,"context_line":"An amount for a standard class that is zero will result in that class"},{"line_number":21,"context_line":"being removed from the requested resources dict sent to placement."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"DocImpact"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Blueprint: custom-resource-classes-in-flavors"},{"line_number":26,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"5f201791_1e38ff9d","line":23,"updated":"2017-06-23 16:37:09.000000000","message":"nit: provide a hint about what the doc impact is for when this generates a nova bug. The hint is that we should document the capability and rules in https://docs.openstack.org/admin-guide/compute-flavors.html#extra-specs.","commit_id":"e5605c57a5cb6e5768b3f940ff80218ad95c55b7"}],"nova/exception.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dd896bfaca4d4100e0d3482ee8f5b5a2ab2f8d6a","unresolved":false,"context_lines":[{"line_number":2072,"context_line":""},{"line_number":2073,"context_line":""},{"line_number":2074,"context_line":"class InvalidResourceAmount(Invalid):"},{"line_number":2075,"context_line":"    msg_fmt \u003d _(\"Resource amounts must be integers. Received \u0027%(amount)s.\u0027\")"},{"line_number":2076,"context_line":""},{"line_number":2077,"context_line":""},{"line_number":2078,"context_line":"class InvalidInventory(Invalid):"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f231b9d_e5cc3236","line":2075,"range":{"start_line":2075,"start_character":72,"end_line":2075,"end_character":73},"updated":"2017-06-16 19:38:56.000000000","message":"You don\u0027t want the period in the quotes for the amount.\n\nI\u0027d just do:\n\n\"Received: %(amount)s\"","commit_id":"7b7df64a37911682e71b67d79470f6c09c801ec4"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"c3b9d6d79b81a0da18755529a6ac9c58fd13ea7c","unresolved":false,"context_lines":[{"line_number":2072,"context_line":""},{"line_number":2073,"context_line":""},{"line_number":2074,"context_line":"class InvalidResourceAmount(Invalid):"},{"line_number":2075,"context_line":"    msg_fmt \u003d _(\"Resource amounts must be integers. Received \u0027%(amount)s.\u0027\")"},{"line_number":2076,"context_line":""},{"line_number":2077,"context_line":""},{"line_number":2078,"context_line":"class InvalidInventory(Invalid):"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f231b9d_90a6de0e","line":2075,"range":{"start_line":2075,"start_character":72,"end_line":2075,"end_character":73},"in_reply_to":"7f231b9d_e5cc3236","updated":"2017-06-16 20:16:28.000000000","message":"urgh - the period should go after the single quote. I could lose the period, but all of the other messages end in periods, so gotta be consistent.","commit_id":"7b7df64a37911682e71b67d79470f6c09c801ec4"}],"nova/scheduler/filter_scheduler.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"651f015cf7256f1c15c270e315fc024e12ed2c44","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        #"},{"line_number":172,"context_line":"        #     resources:$STANDARD_RESOURCE_CLASS\u003d0"},{"line_number":173,"context_line":"        # to remove that resource class from the request."},{"line_number":174,"context_line":"        if hasattr(spec_obj.flavor, \"extra_specs\"):"},{"line_number":175,"context_line":"            extra_specs \u003d spec_obj.flavor.extra_specs"},{"line_number":176,"context_line":"            resource_specs \u003d {key.lstrip(\"resources:\"): val"},{"line_number":177,"context_line":"                    for key, val in extra_specs.items()"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_2081784b","line":174,"updated":"2017-06-13 11:35:54.000000000","message":"nit: this block can be moved to a helper function, and the comment above to its docstring","commit_id":"ef0d52b4614936ed45accd7b232098f45a832f97"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"04f49ff2c86b28d242565c3a10d311fc73d25908","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        #"},{"line_number":172,"context_line":"        #     resources:$STANDARD_RESOURCE_CLASS\u003d0"},{"line_number":173,"context_line":"        # to remove that resource class from the request."},{"line_number":174,"context_line":"        if hasattr(spec_obj.flavor, \"extra_specs\"):"},{"line_number":175,"context_line":"            extra_specs \u003d spec_obj.flavor.extra_specs"},{"line_number":176,"context_line":"            resource_specs \u003d {key.lstrip(\"resources:\"): val"},{"line_number":177,"context_line":"                    for key, val in extra_specs.items()"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_663ae898","line":174,"in_reply_to":"7f231b9d_2081784b","updated":"2017-06-13 12:45:42.000000000","message":"Done","commit_id":"ef0d52b4614936ed45accd7b232098f45a832f97"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"651f015cf7256f1c15c270e315fc024e12ed2c44","unresolved":false,"context_lines":[{"line_number":173,"context_line":"        # to remove that resource class from the request."},{"line_number":174,"context_line":"        if hasattr(spec_obj.flavor, \"extra_specs\"):"},{"line_number":175,"context_line":"            extra_specs \u003d spec_obj.flavor.extra_specs"},{"line_number":176,"context_line":"            resource_specs \u003d {key.lstrip(\"resources:\"): val"},{"line_number":177,"context_line":"                    for key, val in extra_specs.items()"},{"line_number":178,"context_line":"                    if key.startswith(\"resources:\")}"},{"line_number":179,"context_line":"            resource_keys \u003d set([key for key in resource_specs.keys()])"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_407eb46b","line":176,"range":{"start_line":176,"start_character":30,"end_line":176,"end_character":54},"updated":"2017-06-13 11:35:54.000000000","message":"this is not how lstrip works, it will treat \u0027resources:\u0027 as an array of chars. I think you want key.split(\u0027resources:1\u0027, 1)[0] or something like that","commit_id":"ef0d52b4614936ed45accd7b232098f45a832f97"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"04f49ff2c86b28d242565c3a10d311fc73d25908","unresolved":false,"context_lines":[{"line_number":173,"context_line":"        # to remove that resource class from the request."},{"line_number":174,"context_line":"        if hasattr(spec_obj.flavor, \"extra_specs\"):"},{"line_number":175,"context_line":"            extra_specs \u003d spec_obj.flavor.extra_specs"},{"line_number":176,"context_line":"            resource_specs \u003d {key.lstrip(\"resources:\"): val"},{"line_number":177,"context_line":"                    for key, val in extra_specs.items()"},{"line_number":178,"context_line":"                    if key.startswith(\"resources:\")}"},{"line_number":179,"context_line":"            resource_keys \u003d set([key for key in resource_specs.keys()])"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_863744b1","line":176,"range":{"start_line":176,"start_character":30,"end_line":176,"end_character":54},"in_reply_to":"7f231b9d_407eb46b","updated":"2017-06-13 12:45:42.000000000","message":"Doh! That\u0027s what I get for coding under medication. :)","commit_id":"ef0d52b4614936ed45accd7b232098f45a832f97"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"651f015cf7256f1c15c270e315fc024e12ed2c44","unresolved":false,"context_lines":[{"line_number":176,"context_line":"            resource_specs \u003d {key.lstrip(\"resources:\"): val"},{"line_number":177,"context_line":"                    for key, val in extra_specs.items()"},{"line_number":178,"context_line":"                    if key.startswith(\"resources:\")}"},{"line_number":179,"context_line":"            resource_keys \u003d set([key for key in resource_specs.keys()])"},{"line_number":180,"context_line":"            custom_keys \u003d set([key for key in resource_specs.keys()"},{"line_number":181,"context_line":"                    if key.startswith(ResourceClass.CUSTOM_NAMESPACE)])"},{"line_number":182,"context_line":"            std_keys \u003d resource_keys - custom_keys"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_40f3540b","line":179,"updated":"2017-06-13 11:35:54.000000000","message":"nit:\n\n resource_keys \u003d set(resource_specs)\n custom_keys \u003d {key for key in resource_specs\n                if key.startswith(ResourceClass.CUSTOM_NAMESPACE)}","commit_id":"ef0d52b4614936ed45accd7b232098f45a832f97"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"04f49ff2c86b28d242565c3a10d311fc73d25908","unresolved":false,"context_lines":[{"line_number":176,"context_line":"            resource_specs \u003d {key.lstrip(\"resources:\"): val"},{"line_number":177,"context_line":"                    for key, val in extra_specs.items()"},{"line_number":178,"context_line":"                    if key.startswith(\"resources:\")}"},{"line_number":179,"context_line":"            resource_keys \u003d set([key for key in resource_specs.keys()])"},{"line_number":180,"context_line":"            custom_keys \u003d set([key for key in resource_specs.keys()"},{"line_number":181,"context_line":"                    if key.startswith(ResourceClass.CUSTOM_NAMESPACE)])"},{"line_number":182,"context_line":"            std_keys \u003d resource_keys - custom_keys"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_66654845","line":179,"in_reply_to":"7f231b9d_40f3540b","updated":"2017-06-13 12:45:42.000000000","message":"Done","commit_id":"ef0d52b4614936ed45accd7b232098f45a832f97"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"651f015cf7256f1c15c270e315fc024e12ed2c44","unresolved":false,"context_lines":[{"line_number":181,"context_line":"                    if key.startswith(ResourceClass.CUSTOM_NAMESPACE)])"},{"line_number":182,"context_line":"            std_keys \u003d resource_keys - custom_keys"},{"line_number":183,"context_line":"            for custom_key in custom_keys:"},{"line_number":184,"context_line":"                resources[custom_key] \u003d int(resource_specs.get(custom_key))"},{"line_number":185,"context_line":"            for std_key in std_keys:"},{"line_number":186,"context_line":"                if std_key not in resources:"},{"line_number":187,"context_line":"                    # Raise an exception? Or just log it?"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_20cf38b1","line":184,"updated":"2017-06-13 11:35:54.000000000","message":"let\u0027s maybe catch TypeError and ValueError, in case of get() returning something not convertable to an integer.","commit_id":"ef0d52b4614936ed45accd7b232098f45a832f97"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"04f49ff2c86b28d242565c3a10d311fc73d25908","unresolved":false,"context_lines":[{"line_number":181,"context_line":"                    if key.startswith(ResourceClass.CUSTOM_NAMESPACE)])"},{"line_number":182,"context_line":"            std_keys \u003d resource_keys - custom_keys"},{"line_number":183,"context_line":"            for custom_key in custom_keys:"},{"line_number":184,"context_line":"                resources[custom_key] \u003d int(resource_specs.get(custom_key))"},{"line_number":185,"context_line":"            for std_key in std_keys:"},{"line_number":186,"context_line":"                if std_key not in resources:"},{"line_number":187,"context_line":"                    # Raise an exception? Or just log it?"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_26d4f0bc","line":184,"in_reply_to":"7f231b9d_20cf38b1","updated":"2017-06-13 12:45:42.000000000","message":"It seems a bit excessive, since by definition resources can only have integer values. What do you suggest we do if we catch that? I don\u0027t want to fail silently, so the thing to do would be to re-raise it. The other option is to create a new special exception class, and raise that.","commit_id":"ef0d52b4614936ed45accd7b232098f45a832f97"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"651f015cf7256f1c15c270e315fc024e12ed2c44","unresolved":false,"context_lines":[{"line_number":184,"context_line":"                resources[custom_key] \u003d int(resource_specs.get(custom_key))"},{"line_number":185,"context_line":"            for std_key in std_keys:"},{"line_number":186,"context_line":"                if std_key not in resources:"},{"line_number":187,"context_line":"                    # Raise an exception? Or just log it?"},{"line_number":188,"context_line":"                    raise exception.InvalidResourceClass("},{"line_number":189,"context_line":"                            resource_class\u003dstd_key)"},{"line_number":190,"context_line":"                val \u003d int(resource_specs.get(std_key))"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_405a34f3","line":187,"updated":"2017-06-13 11:35:54.000000000","message":"I guess raising is safer, and we can always make it possible later. At least for Ironic we force the CUSTOM_ prefix when exposing the inventory.","commit_id":"ef0d52b4614936ed45accd7b232098f45a832f97"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"04f49ff2c86b28d242565c3a10d311fc73d25908","unresolved":false,"context_lines":[{"line_number":184,"context_line":"                resources[custom_key] \u003d int(resource_specs.get(custom_key))"},{"line_number":185,"context_line":"            for std_key in std_keys:"},{"line_number":186,"context_line":"                if std_key not in resources:"},{"line_number":187,"context_line":"                    # Raise an exception? Or just log it?"},{"line_number":188,"context_line":"                    raise exception.InvalidResourceClass("},{"line_number":189,"context_line":"                            resource_class\u003dstd_key)"},{"line_number":190,"context_line":"                val \u003d int(resource_specs.get(std_key))"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_86f06469","line":187,"in_reply_to":"7f231b9d_405a34f3","updated":"2017-06-13 12:45:42.000000000","message":"I was more concerned with a typo, like \"DISK_BG\".","commit_id":"ef0d52b4614936ed45accd7b232098f45a832f97"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"651f015cf7256f1c15c270e315fc024e12ed2c44","unresolved":false,"context_lines":[{"line_number":187,"context_line":"                    # Raise an exception? Or just log it?"},{"line_number":188,"context_line":"                    raise exception.InvalidResourceClass("},{"line_number":189,"context_line":"                            resource_class\u003dstd_key)"},{"line_number":190,"context_line":"                val \u003d int(resource_specs.get(std_key))"},{"line_number":191,"context_line":"                if val \u003d\u003d 0:"},{"line_number":192,"context_line":"                    resources.pop(std_key, None)"},{"line_number":193,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_a068e886","line":190,"range":{"start_line":190,"start_character":22,"end_line":190,"end_character":25},"updated":"2017-06-13 11:35:54.000000000","message":"ditto about TypeError and ValueError","commit_id":"ef0d52b4614936ed45accd7b232098f45a832f97"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"f9b16781300f4923d629c604354006f89008a570","unresolved":false,"context_lines":[{"line_number":28,"context_line":"from nova import exception"},{"line_number":29,"context_line":"from nova.i18n import _"},{"line_number":30,"context_line":"from nova.objects import fields"},{"line_number":31,"context_line":"from nova.objects.resource_provider import ResourceClass"},{"line_number":32,"context_line":"from nova import rpc"},{"line_number":33,"context_line":"from nova.scheduler import client as scheduler_client"},{"line_number":34,"context_line":"from nova.scheduler import driver"}],"source_content_type":"text/x-python","patch_set":3,"id":"7f231b9d_730f49e1","line":31,"updated":"2017-06-13 13:40:38.000000000","message":"omg, an unqualified import, you\u0027re violating THE RULES (according someone somewhere)","commit_id":"7232deecd0eec142dc2976f2692ce26beadccfd2"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"f9b16781300f4923d629c604354006f89008a570","unresolved":false,"context_lines":[{"line_number":163,"context_line":"            resources[fields.ResourceClass.DISK_GB] \u003d requested_disk_gb"},{"line_number":164,"context_line":"        try:"},{"line_number":165,"context_line":"            # This method will update the resources dict in place. It will"},{"line_number":166,"context_line":"            # raise an .ObjectActionError if extra_specs is not present. Note"},{"line_number":167,"context_line":"            # that testing for extra_specs via hasattr() will fail in Python 3,"},{"line_number":168,"context_line":"            # so the only option is to catch the exception."},{"line_number":169,"context_line":"            self._process_extra_specs(spec_obj, resources)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7f231b9d_53e0cdfa","line":166,"updated":"2017-06-13 13:40:38.000000000","message":"did you mean to leave \u0027.\u0027 in front the exception?","commit_id":"7232deecd0eec142dc2976f2692ce26beadccfd2"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"c5c6af6a114ecd6ac3eb46792787482e0ce4cf76","unresolved":false,"context_lines":[{"line_number":163,"context_line":"            resources[fields.ResourceClass.DISK_GB] \u003d requested_disk_gb"},{"line_number":164,"context_line":"        try:"},{"line_number":165,"context_line":"            # This method will update the resources dict in place. It will"},{"line_number":166,"context_line":"            # raise an .ObjectActionError if extra_specs is not present. Note"},{"line_number":167,"context_line":"            # that testing for extra_specs via hasattr() will fail in Python 3,"},{"line_number":168,"context_line":"            # so the only option is to catch the exception."},{"line_number":169,"context_line":"            self._process_extra_specs(spec_obj, resources)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7f231b9d_769f3703","line":166,"in_reply_to":"7f231b9d_53e0cdfa","updated":"2017-06-13 13:55:46.000000000","message":"Nope, just an oversight.","commit_id":"7232deecd0eec142dc2976f2692ce26beadccfd2"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"f9b16781300f4923d629c604354006f89008a570","unresolved":false,"context_lines":[{"line_number":176,"context_line":"        \"\"\"Check the flavor\u0027s extra_specs for custom resource information."},{"line_number":177,"context_line":"        These will be in the format of either:"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"            resources:$CUSTOM_RESOURCE_CLASS\u003d$N"},{"line_number":180,"context_line":"        ...to add a custom resource class to the request, with a positive"},{"line_number":181,"context_line":"        integer amount of $N"},{"line_number":182,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7f231b9d_767057a6","line":179,"updated":"2017-06-13 13:40:38.000000000","message":"This format sort of implies strings, not key value pairs?","commit_id":"7232deecd0eec142dc2976f2692ce26beadccfd2"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"c5c6af6a114ecd6ac3eb46792787482e0ce4cf76","unresolved":false,"context_lines":[{"line_number":176,"context_line":"        \"\"\"Check the flavor\u0027s extra_specs for custom resource information."},{"line_number":177,"context_line":"        These will be in the format of either:"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"            resources:$CUSTOM_RESOURCE_CLASS\u003d$N"},{"line_number":180,"context_line":"        ...to add a custom resource class to the request, with a positive"},{"line_number":181,"context_line":"        integer amount of $N"},{"line_number":182,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7f231b9d_f659075a","line":179,"in_reply_to":"7f231b9d_767057a6","updated":"2017-06-13 13:55:46.000000000","message":"Well, that\u0027s the form it is in the flavor. It gets automagically converted to a dict during the request process. I can remove it if it\u0027s confusing. I just copied this from the spec.","commit_id":"7232deecd0eec142dc2976f2692ce26beadccfd2"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"f9b16781300f4923d629c604354006f89008a570","unresolved":false,"context_lines":[{"line_number":181,"context_line":"        integer amount of $N"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"            resources:$STANDARD_RESOURCE_CLASS\u003d0"},{"line_number":184,"context_line":"        ...to remove that resource class from the request."},{"line_number":185,"context_line":"        \"\"\""},{"line_number":186,"context_line":"        extra_specs \u003d spec_obj.flavor.extra_specs"},{"line_number":187,"context_line":"        resource_specs \u003d {key.split(\"resources:\", 1)[-1]: val"}],"source_content_type":"text/x-python","patch_set":3,"id":"7f231b9d_d31cddc2","line":184,"updated":"2017-06-13 13:40:38.000000000","message":"The implication here is that \u0027resources:$STANDARD_RESOURCE_CLASS\u003d5\u0027 is undefined, but it is in fact defined down below as overriding or setting.","commit_id":"7232deecd0eec142dc2976f2692ce26beadccfd2"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"c5c6af6a114ecd6ac3eb46792787482e0ce4cf76","unresolved":false,"context_lines":[{"line_number":181,"context_line":"        integer amount of $N"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"            resources:$STANDARD_RESOURCE_CLASS\u003d0"},{"line_number":184,"context_line":"        ...to remove that resource class from the request."},{"line_number":185,"context_line":"        \"\"\""},{"line_number":186,"context_line":"        extra_specs \u003d spec_obj.flavor.extra_specs"},{"line_number":187,"context_line":"        resource_specs \u003d {key.split(\"resources:\", 1)[-1]: val"}],"source_content_type":"text/x-python","patch_set":3,"id":"7f231b9d_963413fd","line":184,"in_reply_to":"7f231b9d_d31cddc2","updated":"2017-06-13 13:55:46.000000000","message":"Done","commit_id":"7232deecd0eec142dc2976f2692ce26beadccfd2"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"f9b16781300f4923d629c604354006f89008a570","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        std_keys \u003d resource_keys - custom_keys"},{"line_number":194,"context_line":"        for custom_key in custom_keys:"},{"line_number":195,"context_line":"            # An invalid value will cause an exception here. We don\u0027t want it"},{"line_number":196,"context_line":"            # to fail silently."},{"line_number":197,"context_line":"            resources[custom_key] \u003d int(resource_specs.get(custom_key))"},{"line_number":198,"context_line":"        for std_key in std_keys:"},{"line_number":199,"context_line":"            if std_key not in resources:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7f231b9d_d3d5fde6","line":196,"updated":"2017-06-13 13:40:38.000000000","message":"Who/what catches it?\n\nWhat expectation can we have here that the extra specs have already been validated in some fashion? I suppose the the \"extra\" nature of the specs means that validation is a bit wack.","commit_id":"7232deecd0eec142dc2976f2692ce26beadccfd2"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"c9de10de56ef57d8b656e0c5365a0dc2f9f3a7e1","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        std_keys \u003d resource_keys - custom_keys"},{"line_number":194,"context_line":"        for custom_key in custom_keys:"},{"line_number":195,"context_line":"            # An invalid value will cause an exception here. We don\u0027t want it"},{"line_number":196,"context_line":"            # to fail silently."},{"line_number":197,"context_line":"            resources[custom_key] \u003d int(resource_specs.get(custom_key))"},{"line_number":198,"context_line":"        for std_key in std_keys:"},{"line_number":199,"context_line":"            if std_key not in resources:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7f231b9d_91a89277","line":196,"in_reply_to":"7f231b9d_892bd480","updated":"2017-06-13 19:38:41.000000000","message":"The only extra_specs validation I could find is here:\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/flavors.py#L286-L291\n\n...and that just validates the key, not the value.","commit_id":"7232deecd0eec142dc2976f2692ce26beadccfd2"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"c5c6af6a114ecd6ac3eb46792787482e0ce4cf76","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        std_keys \u003d resource_keys - custom_keys"},{"line_number":194,"context_line":"        for custom_key in custom_keys:"},{"line_number":195,"context_line":"            # An invalid value will cause an exception here. We don\u0027t want it"},{"line_number":196,"context_line":"            # to fail silently."},{"line_number":197,"context_line":"            resources[custom_key] \u003d int(resource_specs.get(custom_key))"},{"line_number":198,"context_line":"        for std_key in std_keys:"},{"line_number":199,"context_line":"            if std_key not in resources:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7f231b9d_892bd480","line":196,"in_reply_to":"7f231b9d_d3d5fde6","updated":"2017-06-13 13:55:46.000000000","message":"I would assume that this is the realm of operators, since they create the flavors and have access to the logs. I don\u0027t see it as being \"caught\" in order to fail gracefully, or continue with the process, as an exception here means that they have defined an invalid flavor.\n\nThat said, I\u0027d love to hear better ideas for how to handle errors in extra_specs.","commit_id":"7232deecd0eec142dc2976f2692ce26beadccfd2"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"f9b16781300f4923d629c604354006f89008a570","unresolved":false,"context_lines":[{"line_number":199,"context_line":"            if std_key not in resources:"},{"line_number":200,"context_line":"                # Raise an exception? Or just log it?"},{"line_number":201,"context_line":"                raise exception.InvalidResourceClass("},{"line_number":202,"context_line":"                        resource_class\u003dstd_key)"},{"line_number":203,"context_line":"            # Same comment as above about invalid values."},{"line_number":204,"context_line":"            val \u003d int(resource_specs.get(std_key))"},{"line_number":205,"context_line":"            if val \u003d\u003d 0:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7f231b9d_b3c72113","line":202,"updated":"2017-06-13 13:40:38.000000000","message":"seems defensive (in the good way) to raise the exception\n\nBut again, who/what catches it?","commit_id":"7232deecd0eec142dc2976f2692ce26beadccfd2"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"91f7c38710cb76a3e9472268d01693296cf139c6","unresolved":false,"context_lines":[{"line_number":186,"context_line":"                if key.startswith(\"resources:\")}"},{"line_number":187,"context_line":"        resource_keys \u003d set(resource_specs)"},{"line_number":188,"context_line":"        custom_keys \u003d set([key for key in resource_keys"},{"line_number":189,"context_line":"                if key.startswith(ResourceClass.CUSTOM_NAMESPACE)])"},{"line_number":190,"context_line":"        std_keys \u003d resource_keys - custom_keys"},{"line_number":191,"context_line":"        for custom_key in custom_keys:"},{"line_number":192,"context_line":"            # An invalid value will cause an exception here. We don\u0027t want it"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f231b9d_00d52a1c","line":189,"updated":"2017-06-14 16:15:57.000000000","message":"nit: set([]) can be replaced with {} syntax","commit_id":"3883dceee4bf8ad4aa91f169e53da36a95bb50dd"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ae7cd783fbfa2adc737eda22a00413abbdd72734","unresolved":false,"context_lines":[{"line_number":186,"context_line":"                if key.startswith(\"resources:\")}"},{"line_number":187,"context_line":"        resource_keys \u003d set(resource_specs)"},{"line_number":188,"context_line":"        custom_keys \u003d set([key for key in resource_keys"},{"line_number":189,"context_line":"                if key.startswith(ResourceClass.CUSTOM_NAMESPACE)])"},{"line_number":190,"context_line":"        std_keys \u003d resource_keys - custom_keys"},{"line_number":191,"context_line":"        for custom_key in custom_keys:"},{"line_number":192,"context_line":"            # An invalid value will cause an exception here. We don\u0027t want it"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f231b9d_9b852fd2","line":189,"in_reply_to":"7f231b9d_00d52a1c","updated":"2017-06-14 16:45:09.000000000","message":"Please, no... it\u0027s easier to tell it\u0027s a set and not a dict when we use the set() syntax.","commit_id":"3883dceee4bf8ad4aa91f169e53da36a95bb50dd"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"91f7c38710cb76a3e9472268d01693296cf139c6","unresolved":false,"context_lines":[{"line_number":191,"context_line":"        for custom_key in custom_keys:"},{"line_number":192,"context_line":"            # An invalid value will cause an exception here. We don\u0027t want it"},{"line_number":193,"context_line":"            # to fail silently."},{"line_number":194,"context_line":"            resources[custom_key] \u003d int(resource_specs.get(custom_key))"},{"line_number":195,"context_line":"        for std_key in std_keys:"},{"line_number":196,"context_line":"            if std_key not in resources:"},{"line_number":197,"context_line":"                # Raise an exception? Or just log it?"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f231b9d_60e5fe08","line":194,"updated":"2017-06-14 16:15:57.000000000","message":"can\u0027t we raise InvalidResourceClass on Value or TypeError","commit_id":"3883dceee4bf8ad4aa91f169e53da36a95bb50dd"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"91f7c38710cb76a3e9472268d01693296cf139c6","unresolved":false,"context_lines":[{"line_number":198,"context_line":"                raise exception.InvalidResourceClass("},{"line_number":199,"context_line":"                        resource_class\u003dstd_key)"},{"line_number":200,"context_line":"            # Same comment as above about invalid values."},{"line_number":201,"context_line":"            val \u003d int(resource_specs.get(std_key))"},{"line_number":202,"context_line":"            if val \u003d\u003d 0:"},{"line_number":203,"context_line":"                resources.pop(std_key, None)"},{"line_number":204,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f231b9d_20eb86dc","line":201,"updated":"2017-06-14 16:15:57.000000000","message":"ditto","commit_id":"3883dceee4bf8ad4aa91f169e53da36a95bb50dd"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"042def9597ff791b477ba772102ab6b0d471e65a","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                # Amounts must be integers"},{"line_number":195,"context_line":"                resources[custom_key] \u003d int(val)"},{"line_number":196,"context_line":"            except ValueError as e:"},{"line_number":197,"context_line":"                raise exception.InvalidResourceAmount(amount\u003dval)"},{"line_number":198,"context_line":"        for std_key in std_keys:"},{"line_number":199,"context_line":"            if std_key not in resources:"},{"line_number":200,"context_line":"                # Raise an exception? Or just log it?"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f231b9d_290e8ee4","line":197,"updated":"2017-06-16 15:01:53.000000000","message":"I\u0027m still curious what happens in the caller that receives this exception?\n\nI agree that this is a reasonable thing to raise, but something needs to handle it.\n\nFrom browsing the code it looks like this will rise to the select_destinations method in manager.py which right now is only expecting to raise NoValidHost\n\nWill InvalidResourceAmount cause an earth shattering boom? A log message? What?","commit_id":"8dafffa2a6363bcf5cb643bdfdbe4c6e59f3c3c9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"11d7f81cdd077ab31d980e8fe2bbd8cd0456773d","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                # Amounts must be integers"},{"line_number":195,"context_line":"                resources[custom_key] \u003d int(val)"},{"line_number":196,"context_line":"            except ValueError as e:"},{"line_number":197,"context_line":"                raise exception.InvalidResourceAmount(amount\u003dval)"},{"line_number":198,"context_line":"        for std_key in std_keys:"},{"line_number":199,"context_line":"            if std_key not in resources:"},{"line_number":200,"context_line":"                # Raise an exception? Or just log it?"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f231b9d_b06e7ac6","line":197,"in_reply_to":"7f231b9d_290e8ee4","updated":"2017-06-16 19:43:35.000000000","message":"It would result in a NoValidHost and the instance going into ERROR state.","commit_id":"8dafffa2a6363bcf5cb643bdfdbe4c6e59f3c3c9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dd896bfaca4d4100e0d3482ee8f5b5a2ab2f8d6a","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        # to avoid asking for disk usage."},{"line_number":162,"context_line":"        if requested_disk_gb !\u003d 0:"},{"line_number":163,"context_line":"            resources[fields.ResourceClass.DISK_GB] \u003d requested_disk_gb"},{"line_number":164,"context_line":"        if spec_obj.flavor.obj_attr_is_set(\"extra_specs\"):"},{"line_number":165,"context_line":"            self._process_extra_specs(spec_obj.flavor.extra_specs, resources)"},{"line_number":166,"context_line":"        return resources"},{"line_number":167,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7f231b9d_05f3ae75","line":164,"range":{"start_line":164,"start_character":11,"end_line":164,"end_character":57},"updated":"2017-06-16 19:38:56.000000000","message":"nit: You can do this instead:\n\nif \u0027extra_specs\u0027 in spec_obj.flavor:\n   ...\n\nThat\u0027s cleaner looking than using obj_attr_is_set.","commit_id":"7b7df64a37911682e71b67d79470f6c09c801ec4"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"c3b9d6d79b81a0da18755529a6ac9c58fd13ea7c","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        # to avoid asking for disk usage."},{"line_number":162,"context_line":"        if requested_disk_gb !\u003d 0:"},{"line_number":163,"context_line":"            resources[fields.ResourceClass.DISK_GB] \u003d requested_disk_gb"},{"line_number":164,"context_line":"        if spec_obj.flavor.obj_attr_is_set(\"extra_specs\"):"},{"line_number":165,"context_line":"            self._process_extra_specs(spec_obj.flavor.extra_specs, resources)"},{"line_number":166,"context_line":"        return resources"},{"line_number":167,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7f231b9d_50716675","line":164,"range":{"start_line":164,"start_character":11,"end_line":164,"end_character":57},"in_reply_to":"7f231b9d_05f3ae75","updated":"2017-06-16 20:16:28.000000000","message":"Huh, didn\u0027t know that would work without tripping the field loading error. Much nicer.","commit_id":"7b7df64a37911682e71b67d79470f6c09c801ec4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dd896bfaca4d4100e0d3482ee8f5b5a2ab2f8d6a","unresolved":false,"context_lines":[{"line_number":193,"context_line":"            try:"},{"line_number":194,"context_line":"                # Amounts must be integers"},{"line_number":195,"context_line":"                return int(val)"},{"line_number":196,"context_line":"            except ValueError:"},{"line_number":197,"context_line":"                raise exception.InvalidResourceAmount(amount\u003dval)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        for custom_key in custom_keys:"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f231b9d_e5367203","line":196,"range":{"start_line":196,"start_character":19,"end_line":196,"end_character":29},"updated":"2017-06-16 19:38:56.000000000","message":"Check for TypeError in case val is None? I don\u0027t know why someone would do None, but they might screw up thinking that None and 0 are handled the same.","commit_id":"7b7df64a37911682e71b67d79470f6c09c801ec4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"20d3a2bcd8752c7cef37838e951fceb086d42da1","unresolved":false,"context_lines":[{"line_number":193,"context_line":"            try:"},{"line_number":194,"context_line":"                # Amounts must be integers"},{"line_number":195,"context_line":"                return int(val)"},{"line_number":196,"context_line":"            except ValueError:"},{"line_number":197,"context_line":"                raise exception.InvalidResourceAmount(amount\u003dval)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        for custom_key in custom_keys:"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f201791_83095832","line":196,"range":{"start_line":196,"start_character":19,"end_line":196,"end_character":29},"in_reply_to":"7f231b9d_309dca65","updated":"2017-06-23 16:25:36.000000000","message":"The string values can\u0027t be None? I don\u0027t think DictOfStringsField is validating that the values are None or not, is it? It would be good to at least have a test for this.","commit_id":"7b7df64a37911682e71b67d79470f6c09c801ec4"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"c3b9d6d79b81a0da18755529a6ac9c58fd13ea7c","unresolved":false,"context_lines":[{"line_number":193,"context_line":"            try:"},{"line_number":194,"context_line":"                # Amounts must be integers"},{"line_number":195,"context_line":"                return int(val)"},{"line_number":196,"context_line":"            except ValueError:"},{"line_number":197,"context_line":"                raise exception.InvalidResourceAmount(amount\u003dval)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        for custom_key in custom_keys:"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f231b9d_309dca65","line":196,"range":{"start_line":196,"start_character":19,"end_line":196,"end_character":29},"in_reply_to":"7f231b9d_e5367203","updated":"2017-06-16 20:16:28.000000000","message":"I don\u0027t think that\u0027s possible, since extra_specs is a DictOfStringsField in the Flavor object. So the only case would be a string that can\u0027t be cast to an int.","commit_id":"7b7df64a37911682e71b67d79470f6c09c801ec4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dd896bfaca4d4100e0d3482ee8f5b5a2ab2f8d6a","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                # Amounts must be integers"},{"line_number":195,"context_line":"                return int(val)"},{"line_number":196,"context_line":"            except ValueError:"},{"line_number":197,"context_line":"                raise exception.InvalidResourceAmount(amount\u003dval)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        for custom_key in custom_keys:"},{"line_number":200,"context_line":"            resources[custom_key] \u003d valid_int(resource_specs.get(custom_key))"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f231b9d_452d26a6","line":197,"range":{"start_line":197,"start_character":32,"end_line":197,"end_character":53},"updated":"2017-06-16 19:38:56.000000000","message":"We should probably include the key for context here.","commit_id":"7b7df64a37911682e71b67d79470f6c09c801ec4"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"c3b9d6d79b81a0da18755529a6ac9c58fd13ea7c","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                # Amounts must be integers"},{"line_number":195,"context_line":"                return int(val)"},{"line_number":196,"context_line":"            except ValueError:"},{"line_number":197,"context_line":"                raise exception.InvalidResourceAmount(amount\u003dval)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        for custom_key in custom_keys:"},{"line_number":200,"context_line":"            resources[custom_key] \u003d valid_int(resource_specs.get(custom_key))"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f231b9d_1001ae06","line":197,"range":{"start_line":197,"start_character":32,"end_line":197,"end_character":53},"in_reply_to":"7f231b9d_452d26a6","updated":"2017-06-16 20:16:28.000000000","message":"Done","commit_id":"7b7df64a37911682e71b67d79470f6c09c801ec4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dd896bfaca4d4100e0d3482ee8f5b5a2ab2f8d6a","unresolved":false,"context_lines":[{"line_number":200,"context_line":"            resources[custom_key] \u003d valid_int(resource_specs.get(custom_key))"},{"line_number":201,"context_line":"        for std_key in std_keys:"},{"line_number":202,"context_line":"            if std_key not in resources:"},{"line_number":203,"context_line":"                # Raise an exception? Or just log it?"},{"line_number":204,"context_line":"                raise exception.InvalidResourceClass(resource_class\u003dstd_key)"},{"line_number":205,"context_line":"            val \u003d valid_int(resource_specs.get(std_key))"},{"line_number":206,"context_line":"            if val \u003d\u003d 0:"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f231b9d_25010a07","line":203,"range":{"start_line":203,"start_character":16,"end_line":203,"end_character":53},"updated":"2017-06-16 19:38:56.000000000","message":"I would log it as a warning - the flavor has an extra spec with a standard resource class override which we aren\u0027t actually requesting. We don\u0027t need to make things fail.","commit_id":"7b7df64a37911682e71b67d79470f6c09c801ec4"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"c3b9d6d79b81a0da18755529a6ac9c58fd13ea7c","unresolved":false,"context_lines":[{"line_number":200,"context_line":"            resources[custom_key] \u003d valid_int(resource_specs.get(custom_key))"},{"line_number":201,"context_line":"        for std_key in std_keys:"},{"line_number":202,"context_line":"            if std_key not in resources:"},{"line_number":203,"context_line":"                # Raise an exception? Or just log it?"},{"line_number":204,"context_line":"                raise exception.InvalidResourceClass(resource_class\u003dstd_key)"},{"line_number":205,"context_line":"            val \u003d valid_int(resource_specs.get(std_key))"},{"line_number":206,"context_line":"            if val \u003d\u003d 0:"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f231b9d_3006ea21","line":203,"range":{"start_line":203,"start_character":16,"end_line":203,"end_character":53},"in_reply_to":"7f231b9d_25010a07","updated":"2017-06-16 20:16:28.000000000","message":"Done","commit_id":"7b7df64a37911682e71b67d79470f6c09c801ec4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dd896bfaca4d4100e0d3482ee8f5b5a2ab2f8d6a","unresolved":false,"context_lines":[{"line_number":204,"context_line":"                raise exception.InvalidResourceClass(resource_class\u003dstd_key)"},{"line_number":205,"context_line":"            val \u003d valid_int(resource_specs.get(std_key))"},{"line_number":206,"context_line":"            if val \u003d\u003d 0:"},{"line_number":207,"context_line":"                resources.pop(std_key, None)"},{"line_number":208,"context_line":"            else:"},{"line_number":209,"context_line":"                resources[std_key] \u003d val"},{"line_number":210,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7f231b9d_d0045617","line":207,"range":{"start_line":207,"start_character":39,"end_line":207,"end_character":43},"updated":"2017-06-16 19:38:56.000000000","message":"Per above, you wouldn\u0027t even get here, but this needs to stay if you just log it.","commit_id":"7b7df64a37911682e71b67d79470f6c09c801ec4"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"e108a4fe9436ae11a71e5dbbaf7f51d13ab71600","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        for custom_key in custom_keys:"},{"line_number":204,"context_line":"            custom_val \u003d validate_int(custom_key)"},{"line_number":205,"context_line":"            if custom_val is not None:"},{"line_number":206,"context_line":"                resources[custom_key] \u003d custom_val"},{"line_number":207,"context_line":"        for std_key in std_keys:"},{"line_number":208,"context_line":"            if std_key not in resources:"},{"line_number":209,"context_line":"                LOG.warning(\"Received an invalid ResourceClass \u0027%(key)s\u0027 in \""}],"source_content_type":"text/x-python","patch_set":7,"id":"7f231b9d_653a3099","line":206,"updated":"2017-06-19 20:54:05.000000000","message":"The above lets in requests for 0 amount of a custom resource class.","commit_id":"b7b00245bccebeb79eb5140208b3c56b19a75428"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"e5d3928789bd0b4c98bf00359ca6d4667069a4b4","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        for custom_key in custom_keys:"},{"line_number":204,"context_line":"            custom_val \u003d validate_int(custom_key)"},{"line_number":205,"context_line":"            if custom_val is not None:"},{"line_number":206,"context_line":"                resources[custom_key] \u003d custom_val"},{"line_number":207,"context_line":"        for std_key in std_keys:"},{"line_number":208,"context_line":"            if std_key not in resources:"},{"line_number":209,"context_line":"                LOG.warning(\"Received an invalid ResourceClass \u0027%(key)s\u0027 in \""}],"source_content_type":"text/x-python","patch_set":7,"id":"5f201791_ff2441c8","line":206,"in_reply_to":"5f201791_eed7f54e","updated":"2017-06-22 18:50:55.000000000","message":"If they can set $STANDARD_RESOURCE_CLASS to 0, they may think they can set a custom resource class to 0.","commit_id":"b7b00245bccebeb79eb5140208b3c56b19a75428"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"dbc48526a869ff1d61459e942c4139c9a1db2954","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        for custom_key in custom_keys:"},{"line_number":204,"context_line":"            custom_val \u003d validate_int(custom_key)"},{"line_number":205,"context_line":"            if custom_val is not None:"},{"line_number":206,"context_line":"                resources[custom_key] \u003d custom_val"},{"line_number":207,"context_line":"        for std_key in std_keys:"},{"line_number":208,"context_line":"            if std_key not in resources:"},{"line_number":209,"context_line":"                LOG.warning(\"Received an invalid ResourceClass \u0027%(key)s\u0027 in \""}],"source_content_type":"text/x-python","patch_set":7,"id":"5f201791_eed7f54e","line":206,"in_reply_to":"7f231b9d_653a3099","updated":"2017-06-22 16:43:24.000000000","message":"I don\u0027t really see how we need to protect operators from themselves like this, but ok.","commit_id":"b7b00245bccebeb79eb5140208b3c56b19a75428"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"c5f5b3c08e89bfb9bd4b2e99b4bc9c4aefa4fe05","unresolved":false,"context_lines":[{"line_number":194,"context_line":"            try:"},{"line_number":195,"context_line":"                # Amounts must be integers"},{"line_number":196,"context_line":"                return int(val)"},{"line_number":197,"context_line":"            except ValueError:"},{"line_number":198,"context_line":"                # Not a valid integer"},{"line_number":199,"context_line":"                LOG.warning(\"Resource amounts must be integers. Received \""},{"line_number":200,"context_line":"                        \"\u0027%(val)s\u0027 for key %(key)s.\", {\"key\": key, \"val\": val})"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_32077485","line":197,"range":{"start_line":197,"start_character":19,"end_line":197,"end_character":29},"updated":"2017-06-23 14:30:07.000000000","message":"int(None) raises TypeError. should we catch it here too?","commit_id":"e5605c57a5cb6e5768b3f940ff80218ad95c55b7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a3d68281bcfc81b28b556ad85101df98464a8847","unresolved":false,"context_lines":[{"line_number":194,"context_line":"            try:"},{"line_number":195,"context_line":"                # Amounts must be integers"},{"line_number":196,"context_line":"                return int(val)"},{"line_number":197,"context_line":"            except ValueError:"},{"line_number":198,"context_line":"                # Not a valid integer"},{"line_number":199,"context_line":"                LOG.warning(\"Resource amounts must be integers. Received \""},{"line_number":200,"context_line":"                        \"\u0027%(val)s\u0027 for key %(key)s.\", {\"key\": key, \"val\": val})"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_febf83cd","line":197,"range":{"start_line":197,"start_character":19,"end_line":197,"end_character":29},"in_reply_to":"5f201791_1e00ff00","updated":"2017-06-23 16:39:52.000000000","message":"\u003e\u003e\u003e from nova.objects import fields\n\u003e\u003e\u003e class obj(object):\n...     test \u003d fields.DictOfStringsField()\n... \n\u003e\u003e\u003e myobj \u003d obj()\n\u003e\u003e\u003e myobj.test \u003d {\u0027test\u0027: None}\n\u003e\u003e\u003e myobj\n\u003c__main__.obj object at 0xb6c670ec\u003e\n\u003e\u003e\u003e myobj.test\n{\u0027test\u0027: None}\n\u003e\u003e\u003e","commit_id":"e5605c57a5cb6e5768b3f940ff80218ad95c55b7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe821b0792ff933664e15869cd02c97fac133c4e","unresolved":false,"context_lines":[{"line_number":194,"context_line":"            try:"},{"line_number":195,"context_line":"                # Amounts must be integers"},{"line_number":196,"context_line":"                return int(val)"},{"line_number":197,"context_line":"            except ValueError:"},{"line_number":198,"context_line":"                # Not a valid integer"},{"line_number":199,"context_line":"                LOG.warning(\"Resource amounts must be integers. Received \""},{"line_number":200,"context_line":"                        \"\u0027%(val)s\u0027 for key %(key)s.\", {\"key\": key, \"val\": val})"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_1e00ff00","line":197,"range":{"start_line":197,"start_character":19,"end_line":197,"end_character":29},"in_reply_to":"5f201791_32077485","updated":"2017-06-23 16:26:14.000000000","message":"I brought this up earlier too:\n\nhttps://review.openstack.org/#/c/473627/6/nova/scheduler/filter_scheduler.py@196\n\nI think we should at least have a test to make sure.","commit_id":"e5605c57a5cb6e5768b3f940ff80218ad95c55b7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"efa18614396f4987ae78a098a5af14d4190eb799","unresolved":false,"context_lines":[{"line_number":212,"context_line":"                resources[custom_key] \u003d custom_val"},{"line_number":213,"context_line":"        for std_key in std_keys:"},{"line_number":214,"context_line":"            if std_key not in resources:"},{"line_number":215,"context_line":"                LOG.warning(\"Received an invalid ResourceClass \u0027%(key)s\u0027 in \""},{"line_number":216,"context_line":"                        \"extra_specs.\", {\"key\": std_key})"},{"line_number":217,"context_line":"            val \u003d validate_int(std_key)"},{"line_number":218,"context_line":"            if val is None:"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_feec232a","line":215,"updated":"2017-06-23 16:37:09.000000000","message":"Why not continue here? You used to raise an exception.\n\nSo an operator could set resources:IPV4_ADDRESS\u003d1 and it would filter resource providers by that value?","commit_id":"e5605c57a5cb6e5768b3f940ff80218ad95c55b7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1e1ec9e1d4115c00ec4015698764024a38b63557","unresolved":false,"context_lines":[{"line_number":194,"context_line":"            try:"},{"line_number":195,"context_line":"                # Amounts must be integers"},{"line_number":196,"context_line":"                return int(val)"},{"line_number":197,"context_line":"            except ValueError:"},{"line_number":198,"context_line":"                # Not a valid integer"},{"line_number":199,"context_line":"                LOG.warning(\"Resource amounts must be integers. Received \""},{"line_number":200,"context_line":"                        \"\u0027%(val)s\u0027 for key %(key)s.\", {\"key\": key, \"val\": val})"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f201791_8767bfe9","line":197,"range":{"start_line":197,"start_character":19,"end_line":197,"end_character":29},"updated":"2017-06-27 23:17:03.000000000","message":"You can get a TypeError here if val is None, as noted in PS8. Should handle that and have a unit test for it.","commit_id":"0e6fc5fa4054c4fa4dd2546e7d604277ec382d9a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ca45e183cffa05ac11ca44c47ed9df297fab858e","unresolved":false,"context_lines":[{"line_number":194,"context_line":"            try:"},{"line_number":195,"context_line":"                # Amounts must be integers"},{"line_number":196,"context_line":"                return int(val)"},{"line_number":197,"context_line":"            except ValueError:"},{"line_number":198,"context_line":"                # Not a valid integer"},{"line_number":199,"context_line":"                LOG.warning(\"Resource amounts must be integers. Received \""},{"line_number":200,"context_line":"                        \"\u0027%(val)s\u0027 for key %(key)s.\", {\"key\": key, \"val\": val})"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f1d235d_e52d19a9","line":197,"range":{"start_line":197,"start_character":19,"end_line":197,"end_character":29},"in_reply_to":"5f201791_29866cba","updated":"2017-06-29 12:43:28.000000000","message":"Yeah I guess you\u0027re right, and there is a DictOfNullableStringsField field which isn\u0027t used for extra_specs, and nullable defaults to False. I don\u0027t know why this fails but what I tested in https://review.openstack.org/#/c/473627/8/nova/scheduler/filter_scheduler.py@197 didn\u0027t fail, but I guess we don\u0027t need to stress over it.","commit_id":"0e6fc5fa4054c4fa4dd2546e7d604277ec382d9a"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"976f8cc2556687b67c47044e66bc9d7b26205225","unresolved":false,"context_lines":[{"line_number":194,"context_line":"            try:"},{"line_number":195,"context_line":"                # Amounts must be integers"},{"line_number":196,"context_line":"                return int(val)"},{"line_number":197,"context_line":"            except ValueError:"},{"line_number":198,"context_line":"                # Not a valid integer"},{"line_number":199,"context_line":"                LOG.warning(\"Resource amounts must be integers. Received \""},{"line_number":200,"context_line":"                        \"\u0027%(val)s\u0027 for key %(key)s.\", {\"key\": key, \"val\": val})"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f201791_29866cba","line":197,"range":{"start_line":197,"start_character":19,"end_line":197,"end_character":29},"in_reply_to":"5f201791_8767bfe9","updated":"2017-06-28 11:56:11.000000000","message":"I tried to add a test that started with this:\n        flavor \u003d objects.Flavor(vcpus\u003d1,                                                                                                                                             \n                                memory_mb\u003d1024,                                                                                                                                      \n                                root_gb\u003d10,                                                                                                                                          \n                                ephemeral_gb\u003d5,                                                                                                                                      \n                                swap\u003d0,                                                                                                                                              \n                                extra_specs\u003d{                                                                                                                                        \n                                    \"resources:MEMORY_MB\": None})\nWhen that executed, I got this exception:\n  Traceback (most recent call last):\n    File \"nova/tests/unit/scheduler/test_filter_scheduler.py\", line 483, in test_get_resources_per_request_spec_null_value\n      \"resources:MEMORY_MB\": None})\n    File \"nova/objects/flavor.py\", line 234, in __init__\n      super(Flavor, self).__init__(*args, **kwargs)\n    File \"/home/ed/projects/nova/.tox/py27/local/lib/python2.7/site-packages/oslo_versionedobjects/base.py\", line 307, in __init__\n      setattr(self, key, kwargs[key])\n    File \"/home/ed/projects/nova/.tox/py27/local/lib/python2.7/site-packages/oslo_versionedobjects/base.py\", line 72, in setter\n      field_value \u003d field.coerce(self, name, value)\n    File \"/home/ed/projects/nova/.tox/py27/local/lib/python2.7/site-packages/oslo_versionedobjects/fields.py\", line 195, in coerce\n      return self._type.coerce(obj, attr, value)\n    File \"/home/ed/projects/nova/.tox/py27/local/lib/python2.7/site-packages/oslo_versionedobjects/fields.py\", line 665, in coerce\n      coerced_dict.update(value)\n    File \"/home/ed/projects/nova/.tox/py27/local/lib/python2.7/site-packages/oslo_versionedobjects/fields.py\", line 1292, in update\n      super(CoercedDict, self).update(self._coerce_dict(other),\n    File \"/home/ed/projects/nova/.tox/py27/local/lib/python2.7/site-packages/oslo_versionedobjects/fields.py\", line 1270, in _coerce_dict\n      res[key] \u003d self._coerce_item(key, element)\n    File \"/home/ed/projects/nova/.tox/py27/local/lib/python2.7/site-packages/oslo_versionedobjects/fields.py\", line 1282, in _coerce_item\n      return self._element_type.coerce(self._obj, att_name, item)\n    File \"/home/ed/projects/nova/.tox/py27/local/lib/python2.7/site-packages/oslo_versionedobjects/fields.py\", line 193, in coerce\n      return self._null(obj, attr)\n    File \"/home/ed/projects/nova/.tox/py27/local/lib/python2.7/site-packages/oslo_versionedobjects/fields.py\", line 171, in _null\n      raise ValueError(_(\"Field `%s\u0027 cannot be None\") % attr)\n  ValueError: Field `extra_specs[resources:MEMORY_MB]\u0027 cannot be None\nSo a) I still don\u0027t think that this can be None, and b) I don\u0027t think it\u0027s possible to test for it.","commit_id":"0e6fc5fa4054c4fa4dd2546e7d604277ec382d9a"}],"nova/scheduler/utils.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2972a6714b72f0bdf5fd153f2d9af3aee4cd4d25","unresolved":false,"context_lines":[{"line_number":175,"context_line":"        _process_extra_specs(spec_obj.flavor.extra_specs, resources)"},{"line_number":176,"context_line":"    return resources"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    return resources"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"def set_vm_state_and_notify(context, instance_uuid, service, method, updates,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f1d235d_8f21dfa2","line":178,"range":{"start_line":178,"start_character":0,"end_line":178,"end_character":20},"updated":"2017-06-29 14:39:18.000000000","message":"This ain\u0027t gonna work.","commit_id":"3e7e517ae4a3de62a09cb5387c8df2c8c3ba400c"}],"nova/tests/unit/scheduler/test_filter_scheduler.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"91f7c38710cb76a3e9472268d01693296cf139c6","unresolved":false,"context_lines":[{"line_number":461,"context_line":"                                swap\u003d0,"},{"line_number":462,"context_line":"                                extra_specs\u003d{\"resources:CUSTOM_TEST_CLASS\": 1})"},{"line_number":463,"context_line":"        fake_spec \u003d objects.RequestSpec(flavor\u003dflavor)"},{"line_number":464,"context_line":"        resources \u003d self.driver._get_resources_per_request_spec(fake_spec)"},{"line_number":465,"context_line":"        mock_proc.assert_called_once()"},{"line_number":466,"context_line":""},{"line_number":467,"context_line":"    @mock.patch(\"nova.scheduler.filter_scheduler.FilterScheduler.\""}],"source_content_type":"text/x-python","patch_set":4,"id":"7f231b9d_40c7a265","line":464,"range":{"start_line":464,"start_character":8,"end_line":464,"end_character":20},"updated":"2017-06-14 16:15:57.000000000","message":"unused variable causing pep8 failure","commit_id":"3883dceee4bf8ad4aa91f169e53da36a95bb50dd"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"91f7c38710cb76a3e9472268d01693296cf139c6","unresolved":false,"context_lines":[{"line_number":473,"context_line":"                                ephemeral_gb\u003d5,"},{"line_number":474,"context_line":"                                swap\u003d0)"},{"line_number":475,"context_line":"        fake_spec \u003d objects.RequestSpec(flavor\u003dflavor)"},{"line_number":476,"context_line":"        resources \u003d self.driver._get_resources_per_request_spec(fake_spec)"},{"line_number":477,"context_line":"        mock_proc.assert_not_called()"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f231b9d_e0a7cebc","line":476,"range":{"start_line":476,"start_character":8,"end_line":476,"end_character":20},"updated":"2017-06-14 16:15:57.000000000","message":"ditto","commit_id":"3883dceee4bf8ad4aa91f169e53da36a95bb50dd"}]}