)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"2855c82c40a8cfa494fc95488b3b33618d87da8b","unresolved":false,"context_lines":[{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This change starts to profile BatchNotifier threads (e.g. various"},{"line_number":15,"context_line":"callbacks to nova) when the code starting the thread was already"},{"line_number":16,"context_line":"profiled (it\u0027s not at the moment, but that will be another change)."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: Ibd08e097b6f8457c50f8ba9e4a63b96e7e3182bc"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9fb8cfa7_2743cdb1","line":16,"updated":"2019-06-20 13:09:23.000000000","message":"If this is going to be a series of patches, can you open a bug just to track those changes?","commit_id":"042045b2f8078d77fdec2ae08fe95df28c6564ef"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"ecedd4e63d3bca6c0fbbef8a2f8894edefbba35e","unresolved":false,"context_lines":[{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This change starts to profile BatchNotifier threads (e.g. various"},{"line_number":15,"context_line":"callbacks to nova) when the code starting the thread was already"},{"line_number":16,"context_line":"profiled (it\u0027s not at the moment, but that will be another change)."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: Ibd08e097b6f8457c50f8ba9e4a63b96e7e3182bc"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9fb8cfa7_0f500e5e","line":16,"in_reply_to":"9fb8cfa7_2743cdb1","updated":"2019-06-21 12:17:13.000000000","message":"Done","commit_id":"042045b2f8078d77fdec2ae08fe95df28c6564ef"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"bafb2ec0eceb80049d719e0a8f3e5fa48d9953ee","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"While working on improving the osprofiler report in neutron I\u0027m finding"},{"line_number":10,"context_line":"various places where the profiling can be extended by propagating the"},{"line_number":11,"context_line":"profiler info further then before. Expect a series of small patches like"},{"line_number":12,"context_line":"this."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This change starts to profile BatchNotifier threads (e.g. various"},{"line_number":15,"context_line":"callbacks to nova) when the code starting the thread was already"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9fb8cfa7_ca4c8840","line":12,"range":{"start_line":11,"start_character":35,"end_line":12,"end_character":5},"updated":"2019-07-02 10:51:38.000000000","message":"nitty nit: I don\u0027t think it\u0027s good to be in commit message :)","commit_id":"34eda2d0a74c98fee3092cdafb37d3f71c6986c7"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"1e2c30f395e0b3da4f32bd46d231f8868bd274b7","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"While working on improving the osprofiler report in neutron I\u0027m finding"},{"line_number":10,"context_line":"various places where the profiling can be extended by propagating the"},{"line_number":11,"context_line":"profiler info further then before. Expect a series of small patches like"},{"line_number":12,"context_line":"this."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This change starts to profile BatchNotifier threads (e.g. various"},{"line_number":15,"context_line":"callbacks to nova) when the code starting the thread was already"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9fb8cfa7_c2869a66","line":12,"range":{"start_line":11,"start_character":35,"end_line":12,"end_character":5},"in_reply_to":"9fb8cfa7_ca4c8840","updated":"2019-07-03 12:36:48.000000000","message":"Done","commit_id":"34eda2d0a74c98fee3092cdafb37d3f71c6986c7"}],"lower-constraints.txt":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"57d41e01f6f090554955ded9a79a02e084f127f2","unresolved":false,"context_lines":[{"line_number":84,"context_line":"oslo.utils\u003d\u003d3.33.0"},{"line_number":85,"context_line":"oslo.versionedobjects\u003d\u003d1.35.1"},{"line_number":86,"context_line":"oslotest\u003d\u003d3.2.0"},{"line_number":87,"context_line":"osprofiler\u003d\u003d2.3.0"},{"line_number":88,"context_line":"ovs\u003d\u003d2.8.0"},{"line_number":89,"context_line":"ovsdbapp\u003d\u003d0.9.1"},{"line_number":90,"context_line":"Paste\u003d\u003d2.0.2"}],"source_content_type":"text/plain","patch_set":8,"id":"7faddb67_5e1fcb6b","line":87,"updated":"2019-07-08 08:23:19.000000000","message":"maybe this could be mentioned in commit message also? With some short explanation why it is bumped.","commit_id":"f09c77b939fe56c5c45fc301692a53b346940540"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"c7c0a7e27ab26b35433df78d7ac2399493b4de70","unresolved":false,"context_lines":[{"line_number":84,"context_line":"oslo.utils\u003d\u003d3.33.0"},{"line_number":85,"context_line":"oslo.versionedobjects\u003d\u003d1.35.1"},{"line_number":86,"context_line":"oslotest\u003d\u003d3.2.0"},{"line_number":87,"context_line":"osprofiler\u003d\u003d2.3.0"},{"line_number":88,"context_line":"ovs\u003d\u003d2.8.0"},{"line_number":89,"context_line":"ovsdbapp\u003d\u003d0.9.1"},{"line_number":90,"context_line":"Paste\u003d\u003d2.0.2"}],"source_content_type":"text/plain","patch_set":8,"id":"7faddb67_125ce223","line":87,"in_reply_to":"7faddb67_5e1fcb6b","updated":"2019-07-08 12:09:11.000000000","message":"Done","commit_id":"f09c77b939fe56c5c45fc301692a53b346940540"}],"neutron/common/utils.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"1f546f418d532a3701f17cbba1b1a6dbe054cb86","unresolved":false,"context_lines":[{"line_number":960,"context_line":""},{"line_number":961,"context_line":""},{"line_number":962,"context_line":"def _serialize_profiler_info():"},{"line_number":963,"context_line":"    p \u003d profiler.get()"},{"line_number":964,"context_line":"    if p:"},{"line_number":965,"context_line":"        profiler_info \u003d {"},{"line_number":966,"context_line":"            \"hmac_key\": p.hmac_key,"},{"line_number":967,"context_line":"            \"base_id\": p.get_base_id(),"},{"line_number":968,"context_line":"            \"parent_id\": p.get_id(),"},{"line_number":969,"context_line":"        }"},{"line_number":970,"context_line":"    else:"},{"line_number":971,"context_line":"        profiler_info \u003d None"},{"line_number":972,"context_line":"    return profiler_info"},{"line_number":973,"context_line":""},{"line_number":974,"context_line":""},{"line_number":975,"context_line":"def spawn(func, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_faa36c41","line":972,"range":{"start_line":963,"start_character":4,"end_line":972,"end_character":24},"updated":"2019-07-01 12:38:17.000000000","message":"small nit:\n\np \u003d profiler.get()\nif p:\n   return {\"hmac_key\": p.hmac_key,\n           \"base_id\": p.get_base_id(),\n           \"parent_id\": p.get_id(),\n          }","commit_id":"351b5e95466d91a705538948d67f9b952cc4b49d"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"8a4324c1a2a8058fe4d04afcd4f9fb8eeeb6f3c9","unresolved":false,"context_lines":[{"line_number":960,"context_line":""},{"line_number":961,"context_line":""},{"line_number":962,"context_line":"def _serialize_profiler_info():"},{"line_number":963,"context_line":"    p \u003d profiler.get()"},{"line_number":964,"context_line":"    if p:"},{"line_number":965,"context_line":"        profiler_info \u003d {"},{"line_number":966,"context_line":"            \"hmac_key\": p.hmac_key,"},{"line_number":967,"context_line":"            \"base_id\": p.get_base_id(),"},{"line_number":968,"context_line":"            \"parent_id\": p.get_id(),"},{"line_number":969,"context_line":"        }"},{"line_number":970,"context_line":"    else:"},{"line_number":971,"context_line":"        profiler_info \u003d None"},{"line_number":972,"context_line":"    return profiler_info"},{"line_number":973,"context_line":""},{"line_number":974,"context_line":""},{"line_number":975,"context_line":"def spawn(func, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_cce828e0","line":972,"range":{"start_line":963,"start_character":4,"end_line":972,"end_character":24},"in_reply_to":"9fb8cfa7_faa36c41","updated":"2019-07-02 08:49:57.000000000","message":"Done","commit_id":"351b5e95466d91a705538948d67f9b952cc4b49d"}],"neutron/notifiers/batch_notifier.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"2855c82c40a8cfa494fc95488b3b33618d87da8b","unresolved":false,"context_lines":[{"line_number":47,"context_line":""},{"line_number":48,"context_line":"        self.pending_events.append(event)"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"        p \u003d profiler.get()"},{"line_number":51,"context_line":"        if p:"},{"line_number":52,"context_line":"            profiler_info \u003d {"},{"line_number":53,"context_line":"                \"hmac_key\": p.hmac_key,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_074809ca","line":50,"updated":"2019-06-20 13:09:23.000000000","message":"nit: can you do something like https://github.com/openstack/nova/blob/master/nova/utils.py#L763?","commit_id":"042045b2f8078d77fdec2ae08fe95df28c6564ef"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"ecedd4e63d3bca6c0fbbef8a2f8894edefbba35e","unresolved":false,"context_lines":[{"line_number":47,"context_line":""},{"line_number":48,"context_line":"        self.pending_events.append(event)"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"        p \u003d profiler.get()"},{"line_number":51,"context_line":"        if p:"},{"line_number":52,"context_line":"            profiler_info \u003d {"},{"line_number":53,"context_line":"                \"hmac_key\": p.hmac_key,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_71540ccb","line":50,"in_reply_to":"9fb8cfa7_074809ca","updated":"2019-06-21 12:17:13.000000000","message":"Done","commit_id":"042045b2f8078d77fdec2ae08fe95df28c6564ef"}],"neutron/tests/base.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"0f2cbca7deced64128f650fa78b97a5da57a4677","unresolved":false,"context_lines":[{"line_number":382,"context_line":"        self.addCleanup(db_api.sqla_remove_all)"},{"line_number":383,"context_line":"        self.addCleanup(rpc_consumer_reg.clear)"},{"line_number":384,"context_line":"        self.addCleanup(rpc_producer_reg.clear)"},{"line_number":385,"context_line":"        self.addCleanup(profiler.clean)"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"    def get_new_temp_dir(self):"},{"line_number":388,"context_line":"        \"\"\"Create a new temporary directory."}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_60279566","line":385,"updated":"2019-07-05 10:47:52.000000000","message":"Hmmm I don\u0027t know what to think about this. Let\u0027s see what other reviewers comment about this.","commit_id":"f9f6895fecbf7d8b4fbb3ffae502c9baa1be8d98"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"c7c0a7e27ab26b35433df78d7ac2399493b4de70","unresolved":false,"context_lines":[{"line_number":382,"context_line":"        self.addCleanup(db_api.sqla_remove_all)"},{"line_number":383,"context_line":"        self.addCleanup(rpc_consumer_reg.clear)"},{"line_number":384,"context_line":"        self.addCleanup(rpc_producer_reg.clear)"},{"line_number":385,"context_line":"        self.addCleanup(profiler.clean)"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"    def get_new_temp_dir(self):"},{"line_number":388,"context_line":"        \"\"\"Create a new temporary directory."}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_724af655","line":385,"in_reply_to":"7faddb67_60279566","updated":"2019-07-08 12:09:11.000000000","message":"Ack.","commit_id":"f9f6895fecbf7d8b4fbb3ffae502c9baa1be8d98"}],"neutron/tests/unit/common/test_utils.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"1f546f418d532a3701f17cbba1b1a6dbe054cb86","unresolved":false,"context_lines":[{"line_number":596,"context_line":"        with utils.Timer() as timer:"},{"line_number":597,"context_line":"            self.assertIsInstance(timer.delta_time_sec, float)"},{"line_number":598,"context_line":""},{"line_number":599,"context_line":""},{"line_number":600,"context_line":"class SpawnWithProfilerTestCase(base.BaseTestCase):"},{"line_number":601,"context_line":""},{"line_number":602,"context_line":"    def test_spawn_in_utils_passes_args_to_eventlet_spawn(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_1d143a43","line":599,"updated":"2019-07-01 12:38:17.000000000","message":"1) Both spawn and spawn_n tests can be merged (apart from testing the returned values, which are different).\n\n2) IMO, those tests do not provide a real test of the profile being initialized in the new thread. Those tests should actually spawn a new thread, executing a simple function. Inside this function (and using a eventlet Queue), you can get the profile context for this thread (osprofiler.profiler.get()) and put this info in the queue.\nThe main thread should:\na) Get it\u0027s own profile info (shouldn\u0027t be initialized)\nb) Check the thread profile info contains the data passed.","commit_id":"351b5e95466d91a705538948d67f9b952cc4b49d"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"8a4324c1a2a8058fe4d04afcd4f9fb8eeeb6f3c9","unresolved":false,"context_lines":[{"line_number":596,"context_line":"        with utils.Timer() as timer:"},{"line_number":597,"context_line":"            self.assertIsInstance(timer.delta_time_sec, float)"},{"line_number":598,"context_line":""},{"line_number":599,"context_line":""},{"line_number":600,"context_line":"class SpawnWithProfilerTestCase(base.BaseTestCase):"},{"line_number":601,"context_line":""},{"line_number":602,"context_line":"    def test_spawn_in_utils_passes_args_to_eventlet_spawn(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_0ccc0087","line":599,"in_reply_to":"9fb8cfa7_1d143a43","updated":"2019-07-02 08:49:57.000000000","message":"Thank you for the idea, it made the tests way simpler.","commit_id":"351b5e95466d91a705538948d67f9b952cc4b49d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8412a18deee425368998b0738a0c4fa387746341","unresolved":false,"context_lines":[{"line_number":598,"context_line":"            self.assertEqual(init_profiler, p_outside)"},{"line_number":599,"context_line":"            self.assertEqual(init_profiler, p_inside)"},{"line_number":600,"context_line":""},{"line_number":601,"context_line":"        # Make sure outside we start with an uninitialized profiler by"},{"line_number":602,"context_line":"        # spawn()-ing a new thread. Otherwise the unit test runner thread may"},{"line_number":603,"context_line":"        # leak an initialized profiler from one test to another."},{"line_number":604,"context_line":"        eventlet.spawn(outer)"},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"    def test_spawn_with_profiler(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fb8cfa7_4859698c","line":603,"range":{"start_line":601,"start_character":10,"end_line":603,"end_character":64},"updated":"2019-07-03 14:12:00.000000000","message":"+10 to the idea of using new threads to clean the profiler context.\n\nThere are several problems in those tests:\n1) The execution of the asserts is done inside the threads. Because the main program exits before the thread reaches this point, the check is not going to be executed.\n\n2) You should clean the profile at the beginning of outer.\n\n3) You should collect the profile in three different points (see code provided)\n\n4) You should initialize a different string inside the util.spawn-ed thread (see code provided).\n\n\n    def _compare_profilers_outside_and_inside(\n            self, spawn_variant, init_profiler, init_inner_profiler):\n\n        def outer():\n            q \u003d queue.Queue()\n\n            profiler.clean()  # Clean the profiler at the beginning\n            if init_profiler:\n                profiler.init(hmac_key\u003d\u0027fake secret outer\u0027)\n            q.put(profiler.get())\n\n            def is_profiler_initialized(init_inner_profiler):\n                q.put(profiler.get())\n                if init_inner_profiler:\n                    # If the outer profile is not initialized, check\n                    # the inner one inserts other text.\n                    profiler.init(hmac_key\u003d\u0027fake secret inside\u0027)\n                q.put(profiler.get())\n\n            spawn_variant(is_profiler_initialized, init_inner_profiler)\n\n            p_outside \u003d q.get()\n            p_inside_initial \u003d q.get()\n            p_inside_final \u003d q.get()\n            return p_outside, p_inside_initial, p_inside_final\n\n        # Make sure outside we start with an uninitialized profiler by\n        # spawn()-ing a new thread. Otherwise the unit test runner thread may\n        # leak an initialized profiler from one test to another.\n        gt \u003d eventlet.spawn(outer)\n        # Wait until the main thread finish\n        p_out, p_in_ini, p_in_final \u003d gt.wait()\n\n        # Now implement logic here to check p_out, p_in_ini, p_in_final\n\n\n\n\nThe tests should be:\n    def test_spawn_with_*(self):\n        self._compare_profilers_outside_and_inside(\n            spawn_variant\u003dutils.spawn, init_profiler\u003dTrue, \n                                       init_inner_profiler\u003dTrue)\n\n    def test_spawn_with_*(self):\n        self._compare_profilers_outside_and_inside(\n            spawn_variant\u003dutils.spawn, init_profiler\u003dFalse, \n                                       init_inner_profiler\u003dTrue)\n\n    def test_spawn_with_*(self):\n        self._compare_profilers_outside_and_inside(\n            spawn_variant\u003dutils.spawn, init_profiler\u003dTrue, \n                                       init_inner_profiler\u003dFalse)\n\n    def test_spawn_with_*(self):\n        self._compare_profilers_outside_and_inside(\n            spawn_variant\u003dutils.spawn, init_profiler\u003dFalse, \n                                       init_inner_profiler\u003dFalse)\n\n\n\n\nIn order to check both \"utils.spawn\" and \"utils.spawn_n\", please use testscenarios.WithScenarios\n\n\nclass SpawnWithOrWithoutProfilerTestCase(\n        testscenarios.testcase.WithScenarios,\n        base.BaseTestCase):\n\n    scenarios \u003d [\n        (\u0027spawn\u0027, {\u0027spawn_func\u0027: utils.spawn}),\n        (\u0027spawn_n\u0027, {\u0027spawn_func\u0027: utils.spawn_n})]\n\n\n\nThen you can do:\n    def test_spawn_with_*(self):\n        self._compare_profilers_outside_and_inside(\n            spawn_variant\u003dself.spawn_func,\n            init_profiler\u003dFalse, \n            init_inner_profiler\u003dFalse)\n\n\nAnd you\u0027ll have (4 tests) * (2 scenarios)","commit_id":"e222d695893b2677340c32c69a1bb8bc83c5cf55"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"c7c0a7e27ab26b35433df78d7ac2399493b4de70","unresolved":false,"context_lines":[{"line_number":598,"context_line":"            self.assertEqual(init_profiler, p_outside)"},{"line_number":599,"context_line":"            self.assertEqual(init_profiler, p_inside)"},{"line_number":600,"context_line":""},{"line_number":601,"context_line":"        # Make sure outside we start with an uninitialized profiler by"},{"line_number":602,"context_line":"        # spawn()-ing a new thread. Otherwise the unit test runner thread may"},{"line_number":603,"context_line":"        # leak an initialized profiler from one test to another."},{"line_number":604,"context_line":"        eventlet.spawn(outer)"},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"    def test_spawn_with_profiler(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_e79e2606","line":603,"range":{"start_line":601,"start_character":10,"end_line":603,"end_character":64},"in_reply_to":"7faddb67_20e67d18","updated":"2019-07-08 12:09:11.000000000","message":"But isn\u0027t that testing the implementation instead of the interface? As a tester of how osprofiler interacts with threads all I need to know is that the initialized profiler lives on in a new thread. If I test the how, then I\u0027m testing the internal implementation details of osprofiler.","commit_id":"e222d695893b2677340c32c69a1bb8bc83c5cf55"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"0f2cbca7deced64128f650fa78b97a5da57a4677","unresolved":false,"context_lines":[{"line_number":598,"context_line":"            self.assertEqual(init_profiler, p_outside)"},{"line_number":599,"context_line":"            self.assertEqual(init_profiler, p_inside)"},{"line_number":600,"context_line":""},{"line_number":601,"context_line":"        # Make sure outside we start with an uninitialized profiler by"},{"line_number":602,"context_line":"        # spawn()-ing a new thread. Otherwise the unit test runner thread may"},{"line_number":603,"context_line":"        # leak an initialized profiler from one test to another."},{"line_number":604,"context_line":"        eventlet.spawn(outer)"},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"    def test_spawn_with_profiler(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_20e67d18","line":603,"range":{"start_line":601,"start_character":10,"end_line":603,"end_character":64},"in_reply_to":"7faddb67_7cdc3618","updated":"2019-07-05 10:47:52.000000000","message":"3-4) I doesn\u0027t make sense but the point is to test how the profiler is initialized outside and inside the spawned thread. That\u0027s why I introduced those three checking points and two separate methods to initialize the profiler (regardless of the common operation), to know if the profiler has been initialized inside or outside.","commit_id":"e222d695893b2677340c32c69a1bb8bc83c5cf55"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"5cc196b97fcda274ee302befaf5bf8fda6da19e8","unresolved":false,"context_lines":[{"line_number":598,"context_line":"            self.assertEqual(init_profiler, p_outside)"},{"line_number":599,"context_line":"            self.assertEqual(init_profiler, p_inside)"},{"line_number":600,"context_line":""},{"line_number":601,"context_line":"        # Make sure outside we start with an uninitialized profiler by"},{"line_number":602,"context_line":"        # spawn()-ing a new thread. Otherwise the unit test runner thread may"},{"line_number":603,"context_line":"        # leak an initialized profiler from one test to another."},{"line_number":604,"context_line":"        eventlet.spawn(outer)"},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"    def test_spawn_with_profiler(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_93d0d1f9","line":603,"range":{"start_line":601,"start_character":10,"end_line":603,"end_character":64},"in_reply_to":"7faddb67_e79e2606","updated":"2019-07-08 16:57:30.000000000","message":"OK, let\u0027s test only the functionality we need.","commit_id":"e222d695893b2677340c32c69a1bb8bc83c5cf55"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"0754d7d4b5564dabc00be8607fbfd138367321e4","unresolved":false,"context_lines":[{"line_number":598,"context_line":"            self.assertEqual(init_profiler, p_outside)"},{"line_number":599,"context_line":"            self.assertEqual(init_profiler, p_inside)"},{"line_number":600,"context_line":""},{"line_number":601,"context_line":"        # Make sure outside we start with an uninitialized profiler by"},{"line_number":602,"context_line":"        # spawn()-ing a new thread. Otherwise the unit test runner thread may"},{"line_number":603,"context_line":"        # leak an initialized profiler from one test to another."},{"line_number":604,"context_line":"        eventlet.spawn(outer)"},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"    def test_spawn_with_profiler(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_7cdc3618","line":603,"range":{"start_line":601,"start_character":10,"end_line":603,"end_character":64},"in_reply_to":"9fb8cfa7_4859698c","updated":"2019-07-04 09:15:33.000000000","message":"1) Done, thanks for catching my did-not-stop-to-think-I-am-using-threads errors. :-)\n\n2) Done, but I added it as a base class cleanup to have better protection against leakage like this in the future.\n\n3-4) If we had been testing some generic data stored in thread-local storage I would see the point of testing it at 3 points and differentiating between the instances initialized outside and inside. However for a profiler it never makes sense to re-initialize with other data other than the data coming from outside. A user of profiler.init can never make up new arguments to pass to profiler.init because the trace points reported by that profiler could never be queried from the trace point db. The input to profiler.init must come from the user.\n\n5: testscenarios.WithScenarios) Done.","commit_id":"e222d695893b2677340c32c69a1bb8bc83c5cf55"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"7f22b34501b1fd2f651e0c0df6f5771b13bb5482","unresolved":false,"context_lines":[{"line_number":586,"context_line":""},{"line_number":587,"context_line":"        q \u003d queue.Queue()"},{"line_number":588,"context_line":""},{"line_number":589,"context_line":"        def is_profiler_initialized():"},{"line_number":590,"context_line":"            q.put(bool(profiler.get()))"},{"line_number":591,"context_line":""},{"line_number":592,"context_line":"        def outer():"}],"source_content_type":"text/x-python","patch_set":9,"id":"7faddb67_d41b8c18","line":589,"updated":"2019-07-08 22:33:22.000000000","message":"This is really initializing the queue, right?  You\u0027re overloading the queue as an elegant way to both convey the value of profiler.get() and to initialize the queue, but by convention functions that start with \"is_\" usually directly return a boolean.","commit_id":"c4d646b249500c99a4e74080c2b877b6b9b56037"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"8f59996586252af0c667cb6be426bd57a01d5dc7","unresolved":false,"context_lines":[{"line_number":586,"context_line":""},{"line_number":587,"context_line":"        q \u003d queue.Queue()"},{"line_number":588,"context_line":""},{"line_number":589,"context_line":"        def is_profiler_initialized():"},{"line_number":590,"context_line":"            q.put(bool(profiler.get()))"},{"line_number":591,"context_line":""},{"line_number":592,"context_line":"        def outer():"}],"source_content_type":"text/x-python","patch_set":9,"id":"7faddb67_a3dd7015","line":589,"in_reply_to":"7faddb67_d41b8c18","updated":"2019-07-09 11:29:58.000000000","message":"profiler.get() returns None when the profiler is uninitialized.\nIt returns an object when the profiler is initialized.\n\nis_profiler_initialized() originally did what you would expect (producing a boolean whether the profiler is initialized or not) but putting the result in the queue instead of returning it.\n\nI changed the function to return first and put the result into the queue later where we call is_profiler_initialized.","commit_id":"c4d646b249500c99a4e74080c2b877b6b9b56037"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"7f22b34501b1fd2f651e0c0df6f5771b13bb5482","unresolved":false,"context_lines":[{"line_number":589,"context_line":"        def is_profiler_initialized():"},{"line_number":590,"context_line":"            q.put(bool(profiler.get()))"},{"line_number":591,"context_line":""},{"line_number":592,"context_line":"        def outer():"},{"line_number":593,"context_line":"            if init_profiler:"},{"line_number":594,"context_line":"                profiler.init(hmac_key\u003d\u0027fake secret\u0027)"},{"line_number":595,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"7faddb67_f416c8f1","line":592,"updated":"2019-07-08 22:33:22.000000000","message":"Perhaps a name like \"profiler_test_thread\" would be better here, for those that have dissimilar visual metaphors for how code works.","commit_id":"c4d646b249500c99a4e74080c2b877b6b9b56037"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"8f59996586252af0c667cb6be426bd57a01d5dc7","unresolved":false,"context_lines":[{"line_number":589,"context_line":"        def is_profiler_initialized():"},{"line_number":590,"context_line":"            q.put(bool(profiler.get()))"},{"line_number":591,"context_line":""},{"line_number":592,"context_line":"        def outer():"},{"line_number":593,"context_line":"            if init_profiler:"},{"line_number":594,"context_line":"                profiler.init(hmac_key\u003d\u0027fake secret\u0027)"},{"line_number":595,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"7faddb67_f8f3169d","line":592,"in_reply_to":"7faddb67_f416c8f1","updated":"2019-07-09 11:29:58.000000000","message":"Done","commit_id":"c4d646b249500c99a4e74080c2b877b6b9b56037"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"7f22b34501b1fd2f651e0c0df6f5771b13bb5482","unresolved":false,"context_lines":[{"line_number":594,"context_line":"                profiler.init(hmac_key\u003d\u0027fake secret\u0027)"},{"line_number":595,"context_line":""},{"line_number":596,"context_line":"            is_profiler_initialized()"},{"line_number":597,"context_line":"            self.spawn_variant(is_profiler_initialized)"},{"line_number":598,"context_line":""},{"line_number":599,"context_line":"        # Make sure outside we start with an uninitialized profiler by"},{"line_number":600,"context_line":"        # eventlet.spawn()-ing a new thread. Otherwise the unit test runner"}],"source_content_type":"text/x-python","patch_set":9,"id":"7faddb67_94d3d438","line":597,"updated":"2019-07-08 22:33:22.000000000","message":"Please reverse the order of lines 596 and 597; I think that it would be better to see if the profiler is initialized first by spawn to avoid the possibility that spawn is inheriting an already initialized profiler from 596.  I think that is a more rigorous test of the logic.","commit_id":"c4d646b249500c99a4e74080c2b877b6b9b56037"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"8f59996586252af0c667cb6be426bd57a01d5dc7","unresolved":false,"context_lines":[{"line_number":594,"context_line":"                profiler.init(hmac_key\u003d\u0027fake secret\u0027)"},{"line_number":595,"context_line":""},{"line_number":596,"context_line":"            is_profiler_initialized()"},{"line_number":597,"context_line":"            self.spawn_variant(is_profiler_initialized)"},{"line_number":598,"context_line":""},{"line_number":599,"context_line":"        # Make sure outside we start with an uninitialized profiler by"},{"line_number":600,"context_line":"        # eventlet.spawn()-ing a new thread. Otherwise the unit test runner"}],"source_content_type":"text/x-python","patch_set":9,"id":"7faddb67_785c067e","line":597,"in_reply_to":"7faddb67_94d3d438","updated":"2019-07-09 11:29:58.000000000","message":"Done, but this way the queue order is no longer predictable, so I had to merge the results independently of queue order.","commit_id":"c4d646b249500c99a4e74080c2b877b6b9b56037"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"7f22b34501b1fd2f651e0c0df6f5771b13bb5482","unresolved":false,"context_lines":[{"line_number":606,"context_line":"        # addCleanup(profiler.clean)"},{"line_number":607,"context_line":""},{"line_number":608,"context_line":"        p_outside \u003d q.get()"},{"line_number":609,"context_line":"        p_inside \u003d q.get()"},{"line_number":610,"context_line":""},{"line_number":611,"context_line":"        self.assertEqual(init_profiler, p_outside)"},{"line_number":612,"context_line":"        self.assertEqual(init_profiler, p_inside)"}],"source_content_type":"text/x-python","patch_set":9,"id":"7faddb67_b4d61027","line":609,"updated":"2019-07-08 22:33:22.000000000","message":"These variable names confused me initially.  I think something like \u0027uninitialized_profile\u0027 and \u0027initialized_profile\u0027 might work better, since not everyone thinks with the same visual metaphors for code.","commit_id":"c4d646b249500c99a4e74080c2b877b6b9b56037"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"8f59996586252af0c667cb6be426bd57a01d5dc7","unresolved":false,"context_lines":[{"line_number":606,"context_line":"        # addCleanup(profiler.clean)"},{"line_number":607,"context_line":""},{"line_number":608,"context_line":"        p_outside \u003d q.get()"},{"line_number":609,"context_line":"        p_inside \u003d q.get()"},{"line_number":610,"context_line":""},{"line_number":611,"context_line":"        self.assertEqual(init_profiler, p_outside)"},{"line_number":612,"context_line":"        self.assertEqual(init_profiler, p_inside)"}],"source_content_type":"text/x-python","patch_set":9,"id":"7faddb67_b8635e1a","line":609,"in_reply_to":"7faddb67_b4d61027","updated":"2019-07-09 11:29:58.000000000","message":"We cannot call them uninitialized/initialized, because that depends on which test calls this helper method.\n\nChanged outside/inside to parent/child.\n\nAs a side note my mental model was: main spawns outer which in turn spawns inner. main may or may not have leaked profilers because of the test runner, so we cannot assert any state of the profiler there. We eventlet.spawn outer to start with a predictable state of the profiler. Then we utils.spawn inner to test if utils.spawn makes inner \"inherit\" the profiler state from outer.","commit_id":"c4d646b249500c99a4e74080c2b877b6b9b56037"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"7f22b34501b1fd2f651e0c0df6f5771b13bb5482","unresolved":false,"context_lines":[{"line_number":608,"context_line":"        p_outside \u003d q.get()"},{"line_number":609,"context_line":"        p_inside \u003d q.get()"},{"line_number":610,"context_line":""},{"line_number":611,"context_line":"        self.assertEqual(init_profiler, p_outside)"},{"line_number":612,"context_line":"        self.assertEqual(init_profiler, p_inside)"},{"line_number":613,"context_line":""},{"line_number":614,"context_line":"    def test_spawn_with_profiler(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"7faddb67_54dd5c40","line":611,"updated":"2019-07-08 22:33:22.000000000","message":"But I see, really this is just testing whether the profiler is initialized the same if called from an eventlet thread or if called from the spawn function launched from that same greenlet thread.","commit_id":"c4d646b249500c99a4e74080c2b877b6b9b56037"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"5cc196b97fcda274ee302befaf5bf8fda6da19e8","unresolved":false,"context_lines":[{"line_number":608,"context_line":"        p_outside \u003d q.get()"},{"line_number":609,"context_line":"        p_inside \u003d q.get()"},{"line_number":610,"context_line":""},{"line_number":611,"context_line":"        self.assertEqual(init_profiler, p_outside)"},{"line_number":612,"context_line":"        self.assertEqual(init_profiler, p_inside)"},{"line_number":613,"context_line":""},{"line_number":614,"context_line":"    def test_spawn_with_profiler(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"7faddb67_f3eb45ad","line":611,"range":{"start_line":611,"start_character":25,"end_line":611,"end_character":49},"updated":"2019-07-08 16:57:30.000000000","message":"This check is vague, especially because we can actually test the content of the profile (e.g.: hmac_key\u003d\u0027fake secret\u0027)","commit_id":"c4d646b249500c99a4e74080c2b877b6b9b56037"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"8f59996586252af0c667cb6be426bd57a01d5dc7","unresolved":false,"context_lines":[{"line_number":608,"context_line":"        p_outside \u003d q.get()"},{"line_number":609,"context_line":"        p_inside \u003d q.get()"},{"line_number":610,"context_line":""},{"line_number":611,"context_line":"        self.assertEqual(init_profiler, p_outside)"},{"line_number":612,"context_line":"        self.assertEqual(init_profiler, p_inside)"},{"line_number":613,"context_line":""},{"line_number":614,"context_line":"    def test_spawn_with_profiler(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"7faddb67_187cf2b3","line":611,"in_reply_to":"7faddb67_54dd5c40","updated":"2019-07-09 11:29:58.000000000","message":"Nate: Almost, but please see my answer to line 609.","commit_id":"c4d646b249500c99a4e74080c2b877b6b9b56037"}],"requirements.txt":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"57d41e01f6f090554955ded9a79a02e084f127f2","unresolved":false,"context_lines":[{"line_number":43,"context_line":"oslo.upgradecheck\u003e\u003d0.1.0 # Apache-2.0"},{"line_number":44,"context_line":"oslo.utils\u003e\u003d3.33.0 # Apache-2.0"},{"line_number":45,"context_line":"oslo.versionedobjects\u003e\u003d1.35.1 # Apache-2.0"},{"line_number":46,"context_line":"osprofiler\u003e\u003d2.3.0 # Apache-2.0"},{"line_number":47,"context_line":"os-ken \u003e\u003d 0.3.0 # Apache-2.0"},{"line_number":48,"context_line":"ovs\u003e\u003d2.8.0 # Apache-2.0"},{"line_number":49,"context_line":"ovsdbapp\u003e\u003d0.9.1 # Apache-2.0"}],"source_content_type":"text/plain","patch_set":8,"id":"7faddb67_811c026b","line":46,"updated":"2019-07-08 08:23:19.000000000","message":"same here","commit_id":"f09c77b939fe56c5c45fc301692a53b346940540"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"c7c0a7e27ab26b35433df78d7ac2399493b4de70","unresolved":false,"context_lines":[{"line_number":43,"context_line":"oslo.upgradecheck\u003e\u003d0.1.0 # Apache-2.0"},{"line_number":44,"context_line":"oslo.utils\u003e\u003d3.33.0 # Apache-2.0"},{"line_number":45,"context_line":"oslo.versionedobjects\u003e\u003d1.35.1 # Apache-2.0"},{"line_number":46,"context_line":"osprofiler\u003e\u003d2.3.0 # Apache-2.0"},{"line_number":47,"context_line":"os-ken \u003e\u003d 0.3.0 # Apache-2.0"},{"line_number":48,"context_line":"ovs\u003e\u003d2.8.0 # Apache-2.0"},{"line_number":49,"context_line":"ovsdbapp\u003e\u003d0.9.1 # Apache-2.0"}],"source_content_type":"text/plain","patch_set":8,"id":"7faddb67_324d5e72","line":46,"in_reply_to":"7faddb67_811c026b","updated":"2019-07-08 12:09:11.000000000","message":"Done","commit_id":"f09c77b939fe56c5c45fc301692a53b346940540"}]}