)]}'
{"nova/test.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8183ac8a566f4a9b614c6333ce6a907dae627adb","unresolved":false,"context_lines":[{"line_number":370,"context_line":"                hm.create()"},{"line_number":371,"context_line":"                self.host_mappings[hm.host] \u003d hm"},{"line_number":372,"context_line":"        svc \u003d self.useFixture("},{"line_number":373,"context_line":"            nova_fixtures.ServiceFixture(name, host, cell\u003dcell, **kwargs))"},{"line_number":374,"context_line":""},{"line_number":375,"context_line":"        # Keep track of how many instances of this service are running."},{"line_number":376,"context_line":"        self._service_fixture_count[name] +\u003d 1"}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_f005407b","line":373,"range":{"start_line":373,"start_character":58,"end_line":373,"end_character":62},"updated":"2020-03-18 18:38:29.000000000","message":"I know this is kind of a nit, but it took me a sec to figure out why you thought it was best to remove cell\u003dNone above. The kwarg comes in as effectively \"a string that is the cell name\", but this fixture takes \"a cell mapping object\". Relying on the fact that None\u003d\u003dNone, or cell becomes an object of the right type before we get here is not awesome. Granted, I probably set you up for this by hiding cell-which-is-a-name in kwargs before, but in terms of static analysis, this is worse than it was, IMHO.","commit_id":"7aa09eae952194540b1f3122455bdac20753e271"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"44728c614477dfb3f80599ed432b3c0bcbbd4e0a","unresolved":false,"context_lines":[{"line_number":370,"context_line":"                hm.create()"},{"line_number":371,"context_line":"                self.host_mappings[hm.host] \u003d hm"},{"line_number":372,"context_line":"        svc \u003d self.useFixture("},{"line_number":373,"context_line":"            nova_fixtures.ServiceFixture(name, host, cell\u003dcell, **kwargs))"},{"line_number":374,"context_line":""},{"line_number":375,"context_line":"        # Keep track of how many instances of this service are running."},{"line_number":376,"context_line":"        self._service_fixture_count[name] +\u003d 1"}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_90c6359a","line":373,"range":{"start_line":373,"start_character":58,"end_line":373,"end_character":62},"in_reply_to":"1fa4df85_f005407b","updated":"2020-03-20 18:49:56.000000000","message":"The reason for this change isn\u0027t clear - it\u0027s actually caused by starting to call the new start_compute() in tests that don\u0027t use the database. start_compute() always passes a cell kwarg to start_service(). But if a test doesn\u0027t use he database, the if block starting on L347 doesn\u0027t get hit, and we end up with 2 cell kwargs to the ServiceFixture - one that we explicitly pass on L373, and the one from **kwargs that we didn\u0027t pop inside the if block.\n\nI didn\u0027t realise there were two kinds of cell here. I\u0027ll change the start_service argument to cell_name to make it clearer.","commit_id":"7aa09eae952194540b1f3122455bdac20753e271"}],"nova/tests/functional/db/test_virtual_interface.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8183ac8a566f4a9b614c6333ce6a907dae627adb","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        self.cells \u003d objects.CellMappingList.get_all(self.context)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        compute_cell0 \u003d self.start_service("},{"line_number":74,"context_line":"            \u0027compute\u0027, host\u003d\u0027compute2\u0027, cell\u003d\u0027cell0\u0027)"},{"line_number":75,"context_line":"        self.computes \u003d [compute_cell0, self.compute]"},{"line_number":76,"context_line":"        self.instances \u003d []"},{"line_number":77,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_d01044ab","side":"PARENT","line":74,"updated":"2020-03-18 18:38:29.000000000","message":"Granted I have no idea why this was starting a compute in cell0 (which should never happen), I want to point out that your replacement code is starting it in cell1.","commit_id":"b055b5094eaca3191d066749ea6aff16dd6b9867"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"683afaab0ae2d8d25113c6c10400e1b0a33382a7","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        self.cells \u003d objects.CellMappingList.get_all(self.context)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        compute_cell0 \u003d self.start_service("},{"line_number":74,"context_line":"            \u0027compute\u0027, host\u003d\u0027compute2\u0027, cell\u003d\u0027cell0\u0027)"},{"line_number":75,"context_line":"        self.computes \u003d [compute_cell0, self.compute]"},{"line_number":76,"context_line":"        self.instances \u003d []"},{"line_number":77,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"df33271e_f07c0eb7","side":"PARENT","line":74,"in_reply_to":"1fa4df85_d00b4dd2","updated":"2020-03-20 19:48:29.000000000","message":"Well, I\u0027d rather see that separated. I don\u0027t really expect that we\u0027d see a non-deterministic failure from this change, but if we did after we land this patch, we\u0027d want to separate the rest of what you\u0027re doing here from stuff like this which shouldn\u0027t really be related.","commit_id":"b055b5094eaca3191d066749ea6aff16dd6b9867"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"44728c614477dfb3f80599ed432b3c0bcbbd4e0a","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        self.cells \u003d objects.CellMappingList.get_all(self.context)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        compute_cell0 \u003d self.start_service("},{"line_number":74,"context_line":"            \u0027compute\u0027, host\u003d\u0027compute2\u0027, cell\u003d\u0027cell0\u0027)"},{"line_number":75,"context_line":"        self.computes \u003d [compute_cell0, self.compute]"},{"line_number":76,"context_line":"        self.instances \u003d []"},{"line_number":77,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_d00b4dd2","side":"PARENT","line":74,"in_reply_to":"1fa4df85_d01044ab","updated":"2020-03-20 18:49:56.000000000","message":"Doesn\u0027t seem to be breaking anything, so could this be considered a fortunate side-effect\u0027y fix?","commit_id":"b055b5094eaca3191d066749ea6aff16dd6b9867"}],"nova/tests/functional/integrated_helpers.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ce556b2bccfd1988fe03d39bd6f2080645f42a85","unresolved":false,"context_lines":[{"line_number":438,"context_line":"        \"\"\""},{"line_number":439,"context_line":""},{"line_number":440,"context_line":"        ctx \u003d context.get_admin_context()"},{"line_number":441,"context_line":"        for host, compute in self.computes.items():"},{"line_number":442,"context_line":"            LOG.info(\u0027Running periodic for compute (%s)\u0027, host)"},{"line_number":443,"context_line":"            # Make sure the context is targeted to the proper cell database"},{"line_number":444,"context_line":"            # for multi-cell tests."}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_4c6957be","line":441,"updated":"2020-02-04 09:37:20.000000000","message":"Having self.computes defined is a hard requirement of this mixing to work. So I would do a refactor where adding services to self.computes at service start is also in the same mixin. OR I would make the _run_periodics take a computes argument.","commit_id":"a7a4b271c904bae0bca5aad0a924254329839dc5"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"e45d66c83aebd134be58a36837465cac45fa0d7c","unresolved":false,"context_lines":[{"line_number":438,"context_line":"        \"\"\""},{"line_number":439,"context_line":""},{"line_number":440,"context_line":"        ctx \u003d context.get_admin_context()"},{"line_number":441,"context_line":"        for host, compute in self.computes.items():"},{"line_number":442,"context_line":"            LOG.info(\u0027Running periodic for compute (%s)\u0027, host)"},{"line_number":443,"context_line":"            # Make sure the context is targeted to the proper cell database"},{"line_number":444,"context_line":"            # for multi-cell tests."}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_57ce33b7","line":441,"in_reply_to":"3fa7e38b_4c6957be","updated":"2020-02-12 19:33:18.000000000","message":"Good point - I\u0027ve moved the _start_compute() helper into the new mixin.","commit_id":"a7a4b271c904bae0bca5aad0a924254329839dc5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8183ac8a566f4a9b614c6333ce6a907dae627adb","unresolved":false,"context_lines":[{"line_number":860,"context_line":"        \"\"\"Run the update_available_resource task on every compute manager"},{"line_number":861,"context_line":""},{"line_number":862,"context_line":"        This runs periodics on the computes in an undefined order; some child"},{"line_number":863,"context_line":"        class redefined this function to force a specific order."},{"line_number":864,"context_line":"        \"\"\""},{"line_number":865,"context_line":""},{"line_number":866,"context_line":"        ctx \u003d context.get_admin_context()"}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_708870c5","side":"PARENT","line":863,"range":{"start_line":863,"start_character":14,"end_line":863,"end_character":23},"updated":"2020-03-18 18:38:29.000000000","message":"Okay, so not your fault, but it\u0027s still confusing.","commit_id":"b055b5094eaca3191d066749ea6aff16dd6b9867"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8183ac8a566f4a9b614c6333ce6a907dae627adb","unresolved":false,"context_lines":[{"line_number":362,"context_line":"        :param host: the name of the host that will be associated to the"},{"line_number":363,"context_line":"                     compute service."},{"line_number":364,"context_line":"        :param cell_name: optional name of the cell in which to start the"},{"line_number":365,"context_line":"                          compute service (defaults to cell1)"},{"line_number":366,"context_line":"        :return: the nova compute service object"},{"line_number":367,"context_line":"        \"\"\""},{"line_number":368,"context_line":"        compute \u003d self.start_service(\u0027compute\u0027, host\u003dhost, cell\u003dcell_name)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_30e438ac","line":365,"range":{"start_line":365,"start_character":43,"end_line":365,"end_character":60},"updated":"2020-03-18 18:38:29.000000000","message":"IMHO, this is not something this method should be claiming because it doesn\u0027t do it. If the underlying code (which does do the defaulting) ever changes, it\u0027ll be very likely that this becomes incorrect.","commit_id":"7aa09eae952194540b1f3122455bdac20753e271"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"44728c614477dfb3f80599ed432b3c0bcbbd4e0a","unresolved":false,"context_lines":[{"line_number":362,"context_line":"        :param host: the name of the host that will be associated to the"},{"line_number":363,"context_line":"                     compute service."},{"line_number":364,"context_line":"        :param cell_name: optional name of the cell in which to start the"},{"line_number":365,"context_line":"                          compute service (defaults to cell1)"},{"line_number":366,"context_line":"        :return: the nova compute service object"},{"line_number":367,"context_line":"        \"\"\""},{"line_number":368,"context_line":"        compute \u003d self.start_service(\u0027compute\u0027, host\u003dhost, cell\u003dcell_name)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_9faf2629","line":365,"range":{"start_line":365,"start_character":43,"end_line":365,"end_character":60},"in_reply_to":"1fa4df85_30e438ac","updated":"2020-03-20 18:49:56.000000000","message":"This was just moved from L515 on the left, but yeah, removed.","commit_id":"7aa09eae952194540b1f3122455bdac20753e271"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8183ac8a566f4a9b614c6333ce6a907dae627adb","unresolved":false,"context_lines":[{"line_number":367,"context_line":"        \"\"\""},{"line_number":368,"context_line":"        compute \u003d self.start_service(\u0027compute\u0027, host\u003dhost, cell\u003dcell_name)"},{"line_number":369,"context_line":"        if not hasattr(self, \u0027computes\u0027):"},{"line_number":370,"context_line":"            self.computes \u003d {}"},{"line_number":371,"context_line":"        self.computes[host] \u003d compute"},{"line_number":372,"context_line":"        return compute"},{"line_number":373,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_d0d364d4","line":370,"updated":"2020-03-18 18:38:29.000000000","message":"I muchly don\u0027t like this. IIRC, we have places where people override _setup_compute_service() to delay initialization until a later time, and those may end up not having self.computes set until after it\u0027s too late.\n\nHow about we check here to see that self.computes is set (and a dict) and raise a RuntimeError or something similar if not, warning that it\u0027s not legit to mix this class into a class that doesn\u0027t have compute accounting or something?","commit_id":"7aa09eae952194540b1f3122455bdac20753e271"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"44728c614477dfb3f80599ed432b3c0bcbbd4e0a","unresolved":false,"context_lines":[{"line_number":367,"context_line":"        \"\"\""},{"line_number":368,"context_line":"        compute \u003d self.start_service(\u0027compute\u0027, host\u003dhost, cell\u003dcell_name)"},{"line_number":369,"context_line":"        if not hasattr(self, \u0027computes\u0027):"},{"line_number":370,"context_line":"            self.computes \u003d {}"},{"line_number":371,"context_line":"        self.computes[host] \u003d compute"},{"line_number":372,"context_line":"        return compute"},{"line_number":373,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"df33271e_0107aa1e","line":370,"in_reply_to":"1fa4df85_d0d364d4","updated":"2020-03-20 18:49:56.000000000","message":"If a class overrides _setup_compute_service (while inheriting this mixin), it just won\u0027t call this _start_compute() method, so I don\u0027t see what the problem is. Maybe I\u0027m not understanding your point, but have a look at the new PS, and if you still feel the same way we should probably hash it out on IRC.","commit_id":"7aa09eae952194540b1f3122455bdac20753e271"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8183ac8a566f4a9b614c6333ce6a907dae627adb","unresolved":false,"context_lines":[{"line_number":375,"context_line":"        \"\"\"Run the update_available_resource task on every compute manager"},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"        This runs periodics on the computes in an undefined order; some child"},{"line_number":378,"context_line":"        class redefined this function to force a specific order."},{"line_number":379,"context_line":"        \"\"\""},{"line_number":380,"context_line":""},{"line_number":381,"context_line":"        ctx \u003d context.get_admin_context()"}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_70039043","line":378,"range":{"start_line":378,"start_character":14,"end_line":378,"end_character":23},"updated":"2020-03-18 18:38:29.000000000","message":"is the tense correct here? That happened in the past, but no more? Or do you mean \"some classes redefine this...\" ?","commit_id":"7aa09eae952194540b1f3122455bdac20753e271"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"44728c614477dfb3f80599ed432b3c0bcbbd4e0a","unresolved":false,"context_lines":[{"line_number":375,"context_line":"        \"\"\"Run the update_available_resource task on every compute manager"},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"        This runs periodics on the computes in an undefined order; some child"},{"line_number":378,"context_line":"        class redefined this function to force a specific order."},{"line_number":379,"context_line":"        \"\"\""},{"line_number":380,"context_line":""},{"line_number":381,"context_line":"        ctx \u003d context.get_admin_context()"}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_70fcd9b4","line":378,"range":{"start_line":378,"start_character":14,"end_line":378,"end_character":23},"in_reply_to":"1fa4df85_70039043","updated":"2020-03-20 18:49:56.000000000","message":"\u0027redefine\u0027, I strongly suspect. Fixed.","commit_id":"7aa09eae952194540b1f3122455bdac20753e271"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"683afaab0ae2d8d25113c6c10400e1b0a33382a7","unresolved":false,"context_lines":[{"line_number":378,"context_line":"        # If this is the first compute service we\u0027re starting, we need to first"},{"line_number":379,"context_line":"        # create an empty self.comptes dict to keep track of the services we"},{"line_number":380,"context_line":"        # start."},{"line_number":381,"context_line":"        if not hasattr(self, \u0027computes\u0027):"},{"line_number":382,"context_line":"            self.computes \u003d {}"},{"line_number":383,"context_line":"        self.computes[host] \u003d compute"},{"line_number":384,"context_line":"        return compute"},{"line_number":385,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"df33271e_3035d6f8","line":382,"range":{"start_line":381,"start_character":0,"end_line":382,"end_character":30},"updated":"2020-03-20 19:48:29.000000000","message":"This is the thing I don\u0027t like, and it\u0027s unchanged from the previous set. So yeah we can discuss in IRC I guess.","commit_id":"19bacacec69781694f5ded5460fa3e01e2cac259"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"83d7be8a27d42e3a2a831f4acdac1934ba7201eb","unresolved":false,"context_lines":[{"line_number":378,"context_line":"        # If this is the first compute service we\u0027re starting, we need to first"},{"line_number":379,"context_line":"        # create an empty self.comptes dict to keep track of the services we"},{"line_number":380,"context_line":"        # start."},{"line_number":381,"context_line":"        if not hasattr(self, \u0027computes\u0027):"},{"line_number":382,"context_line":"            self.computes \u003d {}"},{"line_number":383,"context_line":"        self.computes[host] \u003d compute"},{"line_number":384,"context_line":"        return compute"},{"line_number":385,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"df33271e_96081a29","line":382,"range":{"start_line":381,"start_character":0,"end_line":382,"end_character":30},"in_reply_to":"df33271e_3035d6f8","updated":"2020-03-20 21:02:32.000000000","message":"As discussed on IRC - the mixin has disappeared entirely, moving the helpers into the base test class. It\u0027s made this question moot, as the base setUp() can now initialize the empty self.computes dict.","commit_id":"19bacacec69781694f5ded5460fa3e01e2cac259"}]}