)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2de6d9bc879a5ac4bac7578a07bfb5aa95be5a64","unresolved":false,"context_lines":[{"line_number":12,"context_line":"injected_files parameter ends up as None."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"An empty list as a default enabled us to inject files through the hook"},{"line_number":15,"context_line":"mechanism. So, having this behaviour changed, breaks the functionality"},{"line_number":16,"context_line":"that was available through this, since we now aren\u0027t able to use that"},{"line_number":17,"context_line":"parameter effectively."},{"line_number":18,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba8a016a_e71a0817","line":15,"updated":"2015-11-23 16:47:16.000000000","message":"In the current diff I don\u0027t see that we are changing the default value of the actual function directly. So I think the commit message needs to be updated","commit_id":"128b4999148cecd75bdf861cb25a1e5b43bc1138"}],"nova/api/openstack/compute/personality.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3e5f4db46260630d704c4505aceab999bbd4b53b","unresolved":false,"context_lines":[{"line_number":49,"context_line":"    # server_update \u0026 server_rebuild"},{"line_number":50,"context_line":"    def server_create(self, server_dict, create_kwargs,"},{"line_number":51,"context_line":"                      body_deprecated_param\u003dNone):"},{"line_number":52,"context_line":"        create_kwargs[\u0027injected_files\u0027] \u003d self._get_injected_files("},{"line_number":53,"context_line":"            server_dict.get(\u0027personality\u0027, []))"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def server_rebuild(self, server_dict, create_kwargs,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_8f98ff6b","line":52,"updated":"2015-11-23 14:40:06.000000000","message":"Does the intention here is that the injected_files is overwritten by the personality? Is there any case when there is something already in the create_kwargs[\u0027injected_files\u0027] we are loosing here?","commit_id":"128b4999148cecd75bdf861cb25a1e5b43bc1138"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2de6d9bc879a5ac4bac7578a07bfb5aa95be5a64","unresolved":false,"context_lines":[{"line_number":49,"context_line":"    # server_update \u0026 server_rebuild"},{"line_number":50,"context_line":"    def server_create(self, server_dict, create_kwargs,"},{"line_number":51,"context_line":"                      body_deprecated_param\u003dNone):"},{"line_number":52,"context_line":"        create_kwargs[\u0027injected_files\u0027] \u003d self._get_injected_files("},{"line_number":53,"context_line":"            server_dict.get(\u0027personality\u0027, []))"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def server_rebuild(self, server_dict, create_kwargs,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_c76104aa","line":52,"in_reply_to":"ba8a016a_21d6c727","updated":"2015-11-23 16:47:16.000000000","message":"Ok you convinced me. :)","commit_id":"128b4999148cecd75bdf861cb25a1e5b43bc1138"},{"author":{"_account_id":10873,"name":"Juan Antonio Osorio Robles","email":"jaosorior@redhat.com","username":"ejuaoso"},"change_message_id":"c53cba545d818e9c986d741acd1ef41d72e43b8b","unresolved":false,"context_lines":[{"line_number":49,"context_line":"    # server_update \u0026 server_rebuild"},{"line_number":50,"context_line":"    def server_create(self, server_dict, create_kwargs,"},{"line_number":51,"context_line":"                      body_deprecated_param\u003dNone):"},{"line_number":52,"context_line":"        create_kwargs[\u0027injected_files\u0027] \u003d self._get_injected_files("},{"line_number":53,"context_line":"            server_dict.get(\u0027personality\u0027, []))"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def server_rebuild(self, server_dict, create_kwargs,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_21d6c727","line":52,"in_reply_to":"ba8a016a_8f98ff6b","updated":"2015-11-23 14:50:16.000000000","message":"well, the personality parameter has been the one setting up the injected files, both as it is now here, and in the legacy controller: https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/legacy_v2/servers.py#L532\n\nAnd, the create_kwargs was an empty dict before the extensions were read an executed, which happens here: https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/servers.py#L534","commit_id":"128b4999148cecd75bdf861cb25a1e5b43bc1138"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"0046866a7da522f52a71dff94c9cabdbcba3a70e","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    def server_create(self, server_dict, create_kwargs,"},{"line_number":51,"context_line":"                      body_deprecated_param\u003dNone):"},{"line_number":52,"context_line":"        create_kwargs[\u0027injected_files\u0027] \u003d self._get_injected_files("},{"line_number":53,"context_line":"            server_dict.get(\u0027personality\u0027, []))"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def server_rebuild(self, server_dict, create_kwargs,"},{"line_number":56,"context_line":"                      body_deprecated_param\u003dNone):"}],"source_content_type":"text/x-python","patch_set":10,"id":"5aef4532_c2c29252","line":53,"updated":"2016-03-10 12:37:37.000000000","message":"I guess this makes us consistent with here:\nhttps://github.com/openstack/nova/blob/master/nova/api/openstack/compute/legacy_v2/servers.py#L534\n\nOK.","commit_id":"be4220b37df42d4f99b5f5b8c18245a315228a8a"}],"nova/compute/api.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"5156f216f92cce0129c8afd8b220378f7ef08d93","unresolved":false,"context_lines":[{"line_number":1531,"context_line":"               display_name\u003dNone, display_description\u003dNone,"},{"line_number":1532,"context_line":"               key_name\u003dNone, key_data\u003dNone, security_group\u003dNone,"},{"line_number":1533,"context_line":"               availability_zone\u003dNone, user_data\u003dNone, metadata\u003dNone,"},{"line_number":1534,"context_line":"               injected_files\u003d[], admin_password\u003dNone,"},{"line_number":1535,"context_line":"               block_device_mapping\u003dNone, access_ip_v4\u003dNone,"},{"line_number":1536,"context_line":"               access_ip_v6\u003dNone, requested_networks\u003dNone, config_drive\u003dNone,"},{"line_number":1537,"context_line":"               auto_disk_config\u003dNone, scheduler_hints\u003dNone, legacy_bdm\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba8a016a_dde80519","line":1534,"updated":"2015-11-23 13:06:16.000000000","message":"Do not use mutable default value, use an empty tuple instead.","commit_id":"f61383aaf23a155e03de14c599baa59f0480803f"},{"author":{"_account_id":10873,"name":"Juan Antonio Osorio Robles","email":"jaosorior@redhat.com","username":"ejuaoso"},"change_message_id":"c13d7d83c41b24b2d053f6f7b0a58f34753ee8da","unresolved":false,"context_lines":[{"line_number":1531,"context_line":"               display_name\u003dNone, display_description\u003dNone,"},{"line_number":1532,"context_line":"               key_name\u003dNone, key_data\u003dNone, security_group\u003dNone,"},{"line_number":1533,"context_line":"               availability_zone\u003dNone, user_data\u003dNone, metadata\u003dNone,"},{"line_number":1534,"context_line":"               injected_files\u003d[], admin_password\u003dNone,"},{"line_number":1535,"context_line":"               block_device_mapping\u003dNone, access_ip_v4\u003dNone,"},{"line_number":1536,"context_line":"               access_ip_v6\u003dNone, requested_networks\u003dNone, config_drive\u003dNone,"},{"line_number":1537,"context_line":"               auto_disk_config\u003dNone, scheduler_hints\u003dNone, legacy_bdm\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba8a016a_e311aab0","line":1534,"in_reply_to":"ba8a016a_804f74bd","updated":"2015-11-23 13:41:20.000000000","message":"Actually, mutability is the intent of this code, and is exactly the behavior that I\u0027m looking for. \n\nHere\u0027s a concrete example, which is the intent of this CR:\n\nThis function is surrounded by that add_hook decorator, which in turn wraps this function by a hook function that is set in the entry points. This enables third party hooks to do some interesting things. In this case, what we wanted to do was add extra injected_files [1]. If this was a tuple, that wouldn\u0027t be possible. So in this case, Python\u0027s default pass-by-reference is the desirable thing.\n\nOne things to be noted is that this is merely reflecting the behavior that was the default before. So this function not receiving a list is a regression to how the API used to behave.\n\n[1] https://github.com/richm/rdo-vm-factory/blob/master/rdo-ipa-nova/novahooks.py#L256","commit_id":"f61383aaf23a155e03de14c599baa59f0480803f"},{"author":{"_account_id":10873,"name":"Juan Antonio Osorio Robles","email":"jaosorior@redhat.com","username":"ejuaoso"},"change_message_id":"fc89311f125d5127199951ba952d02273481d199","unresolved":false,"context_lines":[{"line_number":1531,"context_line":"               display_name\u003dNone, display_description\u003dNone,"},{"line_number":1532,"context_line":"               key_name\u003dNone, key_data\u003dNone, security_group\u003dNone,"},{"line_number":1533,"context_line":"               availability_zone\u003dNone, user_data\u003dNone, metadata\u003dNone,"},{"line_number":1534,"context_line":"               injected_files\u003d[], admin_password\u003dNone,"},{"line_number":1535,"context_line":"               block_device_mapping\u003dNone, access_ip_v4\u003dNone,"},{"line_number":1536,"context_line":"               access_ip_v6\u003dNone, requested_networks\u003dNone, config_drive\u003dNone,"},{"line_number":1537,"context_line":"               auto_disk_config\u003dNone, scheduler_hints\u003dNone, legacy_bdm\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba8a016a_fd9709e9","line":1534,"in_reply_to":"ba8a016a_dde80519","updated":"2015-11-23 13:12:57.000000000","message":"Is there documentation or somewhere where it states that an immutable value is the preferred way for the API? The whole point of this CR is to have a list again (as it was before) so it would be mutable and usable by hooks.","commit_id":"f61383aaf23a155e03de14c599baa59f0480803f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3fa90c6ddd4b6e96dc017e8d86823c7226e23e97","unresolved":false,"context_lines":[{"line_number":1531,"context_line":"               display_name\u003dNone, display_description\u003dNone,"},{"line_number":1532,"context_line":"               key_name\u003dNone, key_data\u003dNone, security_group\u003dNone,"},{"line_number":1533,"context_line":"               availability_zone\u003dNone, user_data\u003dNone, metadata\u003dNone,"},{"line_number":1534,"context_line":"               injected_files\u003d[], admin_password\u003dNone,"},{"line_number":1535,"context_line":"               block_device_mapping\u003dNone, access_ip_v4\u003dNone,"},{"line_number":1536,"context_line":"               access_ip_v6\u003dNone, requested_networks\u003dNone, config_drive\u003dNone,"},{"line_number":1537,"context_line":"               auto_disk_config\u003dNone, scheduler_hints\u003dNone, legacy_bdm\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba8a016a_804f74bd","line":1534,"in_reply_to":"ba8a016a_fd9709e9","updated":"2015-11-23 13:26:59.000000000","message":"Mutable default arguments are dangerous in general in python see: http://docs.python-guide.org/en/latest/writing/gotchas/","commit_id":"f61383aaf23a155e03de14c599baa59f0480803f"}],"nova/compute/rpcapi.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"5156f216f92cce0129c8afd8b220378f7ef08d93","unresolved":false,"context_lines":[{"line_number":936,"context_line":"                   events\u003devents)"},{"line_number":937,"context_line":""},{"line_number":938,"context_line":"    def build_and_run_instance(self, ctxt, instance, host, image, request_spec,"},{"line_number":939,"context_line":"            filter_properties, admin_password\u003dNone, injected_files\u003d[],"},{"line_number":940,"context_line":"            requested_networks\u003dNone, security_groups\u003dNone,"},{"line_number":941,"context_line":"            block_device_mapping\u003dNone, node\u003dNone, limits\u003dNone):"},{"line_number":942,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba8a016a_ddb9e5ed","line":939,"updated":"2015-11-23 13:06:16.000000000","message":"Use immutable default value","commit_id":"f61383aaf23a155e03de14c599baa59f0480803f"}],"nova/tests/unit/api/openstack/compute/test_serversV21.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c3f45840ea84d9edad4d0d4123ffdf7ceab4264b","unresolved":false,"context_lines":[{"line_number":3298,"context_line":"            self.assertEqual([], kwargs[\u0027injected_files\u0027])"},{"line_number":3299,"context_line":"            return old_create(*args, **kwargs)"},{"line_number":3300,"context_line":""},{"line_number":3301,"context_line":"        self.stubs.Set(compute_api.API, \u0027create\u0027, create)"},{"line_number":3302,"context_line":""},{"line_number":3303,"context_line":"        self._test_create_instance()"},{"line_number":3304,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"5aef4532_da14684e","line":3301,"range":{"start_line":3301,"start_character":8,"end_line":3301,"end_character":22},"updated":"2016-03-09 18:21:57.000000000","message":"Please use self.stub_out now since it doesn\u0027t use mox (we\u0027re trying to get away from mox).","commit_id":"a81f23fa2f2bb268b975935c5a4e0dc23188c47e"}],"nova/tests/unit/test_personality.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"59772a5b1cb38cd84aeb617e9ec9d1bb3f126802","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from nova import test\r"},{"line_number":17,"context_line":"\r"},{"line_number":18,"context_line":"\r"},{"line_number":19,"context_line":"class PersonalityExtTestV21(test.TestCase):\r"},{"line_number":20,"context_line":"    def setUp(self):\r"},{"line_number":21,"context_line":"        super(PersonalityExtTestV21, self).setUp()\r"},{"line_number":22,"context_line":"        self.personality_ext \u003d personality.Personality(None)\r"}],"source_content_type":"text/x-python","patch_set":6,"id":"9aed3d3a_31393cd8","line":19,"updated":"2016-02-29 15:20:07.000000000","message":"This is testing a compute API, it should live with the other API tests. For the personality extension, those live here:\n\nhttps://github.com/openstack/nova/blob/3da31e3262ee3b07e96efa4eca381953ff1de169/nova/tests/unit/api/openstack/compute/test_serversV21.py\n\nSo for the change you\u0027re testing, it should go in that file.","commit_id":"f05ad4656e0044d808e555d331f1b1e0bfeadff8"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"cd02e0223cde444e1ce0a630eb27972bc630297f","unresolved":false,"context_lines":[{"line_number":21,"context_line":"        super(PersonalityExtTestV21, self).setUp()\r"},{"line_number":22,"context_line":"        self.personality_ext \u003d personality.Personality(None)\r"},{"line_number":23,"context_line":"\r"},{"line_number":24,"context_line":"    def _test(self, func):\r"},{"line_number":25,"context_line":"        server_dict \u003d {\u0027personality\u0027: [{\u0027path\u0027: \u0027/tmp/some_file\u0027,\r"},{"line_number":26,"context_line":"                                        \u0027contents\u0027: \u0027data\u0027}]}\r"},{"line_number":27,"context_line":"        create_kwargs \u003d {}\r"}],"source_content_type":"text/x-python","patch_set":6,"id":"bae84128_10789b29","line":24,"updated":"2016-02-22 17:07:21.000000000","message":"These helper functions don\u0027t seem all that necessary since they\u0027re both getting the same func argument from the tests, and the thing that actually needs to be different is the server_dict, which isn\u0027t parameterized.  Might be nice to rewrite _test to take server_dict instead and have both tests call one helper.","commit_id":"f05ad4656e0044d808e555d331f1b1e0bfeadff8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6c577911424f72b88024e12b84e737e134005174","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from nova import test\r"},{"line_number":17,"context_line":"\r"},{"line_number":18,"context_line":"\r"},{"line_number":19,"context_line":"class PersonalityExtTestV21(test.TestCase):\r"},{"line_number":20,"context_line":"    def setUp(self):\r"},{"line_number":21,"context_line":"        super(PersonalityExtTestV21, self).setUp()\r"},{"line_number":22,"context_line":"        self.personality_ext \u003d personality.Personality(None)\r"}],"source_content_type":"text/x-python","patch_set":7,"id":"5aef4532_58eef3b4","line":19,"updated":"2016-03-08 15:42:47.000000000","message":"Why do we need a new test class and module for this?  This should live with the other personality API tests in the proper subtree and module, see:\n\nhttps://github.com/openstack/nova/blob/3da31e3262ee3b07e96efa4eca381953ff1de169/nova/tests/unit/api/openstack/compute/test_serversV21.py#L1747","commit_id":"036ac2e0b3d52af639ae3f4b997d5e8003fdf043"},{"author":{"_account_id":10873,"name":"Juan Antonio Osorio Robles","email":"jaosorior@redhat.com","username":"ejuaoso"},"change_message_id":"4854bf9b37f3f6e46e3f95f2b6c7ea9587626f35","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from nova import test\r"},{"line_number":17,"context_line":"\r"},{"line_number":18,"context_line":"\r"},{"line_number":19,"context_line":"class PersonalityExtTestV21(test.TestCase):\r"},{"line_number":20,"context_line":"    def setUp(self):\r"},{"line_number":21,"context_line":"        super(PersonalityExtTestV21, self).setUp()\r"},{"line_number":22,"context_line":"        self.personality_ext \u003d personality.Personality(None)\r"}],"source_content_type":"text/x-python","patch_set":7,"id":"5aef4532_cc98841e","line":19,"in_reply_to":"5aef4532_58eef3b4","updated":"2016-03-08 16:00:04.000000000","message":"I can move this to that module. But the existing test classes there don\u0027t provide the means to check that the parameters are correct. Is it fine if I just move this class to that module?","commit_id":"036ac2e0b3d52af639ae3f4b997d5e8003fdf043"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5e687fe81621adb5e6f7cbde30948f2afbe3f948","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from nova import test\r"},{"line_number":17,"context_line":"\r"},{"line_number":18,"context_line":"\r"},{"line_number":19,"context_line":"class PersonalityExtTestV21(test.TestCase):\r"},{"line_number":20,"context_line":"    def setUp(self):\r"},{"line_number":21,"context_line":"        super(PersonalityExtTestV21, self).setUp()\r"},{"line_number":22,"context_line":"        self.personality_ext \u003d personality.Personality(None)\r"}],"source_content_type":"text/x-python","patch_set":7,"id":"5aef4532_8a02c56d","line":19,"in_reply_to":"5aef4532_cc98841e","updated":"2016-03-09 01:34:16.000000000","message":"You can\u0027t mock something out there to check the kwargs? At a minimum this should at least live in that same module, but I\u0027d think you could work this into the existing tests by mocking out the right method and inspecting the kwargs passed to it.","commit_id":"036ac2e0b3d52af639ae3f4b997d5e8003fdf043"}]}