)]}'
{"nova/compute/api.py":[{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"9e049e97044654f570293de0085fbdcb3d20085f","unresolved":false,"context_lines":[{"line_number":1821,"context_line":"                    # the cell database. So we temporarily untarget the"},{"line_number":1822,"context_line":"                    # context while we do the quota stuff and re-target after"},{"line_number":1823,"context_line":"                    # we\u0027re done."},{"line_number":1824,"context_line":"                    original_db_connection \u003d getattr(context, \u0027db_connection\u0027,"},{"line_number":1825,"context_line":"                                                     None)"},{"line_number":1826,"context_line":"                    context.db_connection \u003d None"},{"line_number":1827,"context_line":"                    try:"},{"line_number":1828,"context_line":"                        # This is confusing but actually decrements quota."},{"line_number":1829,"context_line":"                        quotas \u003d self._create_reservations(context,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a30ddce_f2e6e8b2","line":1826,"range":{"start_line":1824,"start_character":0,"end_line":1826,"end_character":48},"updated":"2017-03-09 08:01:06.000000000","message":"seems incorrect, I tested locally and if we change to None here, instance.flavor cannot be correctly loaded latter in:\nhttps://github.com/openstack/nova/blob/master/nova/compute/api.py#L1997\n\nDue to InstanceNotFound; Seems we still need to check instance in DB to get flaovr object","commit_id":"4bddcb475eef83193ba12f20d7a6cc22c09488b1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0dec9982e020d0ec0408a27fc297a94d6ec27704","unresolved":false,"context_lines":[{"line_number":1821,"context_line":"                    # the cell database. So we temporarily untarget the"},{"line_number":1822,"context_line":"                    # context while we do the quota stuff and re-target after"},{"line_number":1823,"context_line":"                    # we\u0027re done."},{"line_number":1824,"context_line":"                    original_db_connection \u003d getattr(context, \u0027db_connection\u0027,"},{"line_number":1825,"context_line":"                                                     None)"},{"line_number":1826,"context_line":"                    context.db_connection \u003d None"},{"line_number":1827,"context_line":"                    try:"},{"line_number":1828,"context_line":"                        # This is confusing but actually decrements quota."},{"line_number":1829,"context_line":"                        quotas \u003d self._create_reservations(context,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a30ddce_e0e368a2","line":1826,"range":{"start_line":1824,"start_character":0,"end_line":1826,"end_character":48},"in_reply_to":"9a30ddce_f2e6e8b2","updated":"2017-03-09 13:43:41.000000000","message":"Hmm, that\u0027s odd because I\u0027d think this would make it read the main (nova) DB:\n\nhttps://github.com/openstack/nova/blob/f1cf1de/nova/db/sqlalchemy/api.py#L139\n\nBut the instance isn\u0027t there either, it\u0027s in cell0. Do you have the stacktrace? This probably gets even more ugly because then I have to handle both cell contexts in _create_reservations because the instance has to load the flavor that\u0027s stashed inside of it from the cell0 database, and we have to create the quota records (reservations) in the main (nova) cell database. I either have to do that within _create_reservations or pass in the needed information like the flavor.","commit_id":"4bddcb475eef83193ba12f20d7a6cc22c09488b1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d2f9cc4c70cef63c79ef22f9de5e745026e1d8ec","unresolved":false,"context_lines":[{"line_number":1830,"context_line":"                                                           instance,"},{"line_number":1831,"context_line":"                                                           instance.task_state,"},{"line_number":1832,"context_line":"                                                           project_id, user_id)"},{"line_number":1833,"context_line":"                        quotas.commit()"},{"line_number":1834,"context_line":"                        # Now the context will be temporarily mutated back to"},{"line_number":1835,"context_line":"                        # the cell from which we want to delete the instance."},{"line_number":1836,"context_line":"                        with nova_context.target_cell(context, cell):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a30ddce_a6727aab","line":1833,"updated":"2017-03-09 04:04:54.000000000","message":"Can we just do something like create an empty CellMapping and use target_cell instead? For example:\n\n empty_cell \u003d objects.CellMapping(database_connection\u003d\u0027\u0027)\n try:\n     with nova_context.target_cell(context, empty_cell):\n         # create reservations\n         # commit\n ...\n\nI think it will work similarly because of the way the db conf is read [1].\n\n[1] https://github.com/openstack/nova/blob/f1cf1de/nova/db/sqlalchemy/api.py#L85","commit_id":"4bddcb475eef83193ba12f20d7a6cc22c09488b1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0dec9982e020d0ec0408a27fc297a94d6ec27704","unresolved":false,"context_lines":[{"line_number":1830,"context_line":"                                                           instance,"},{"line_number":1831,"context_line":"                                                           instance.task_state,"},{"line_number":1832,"context_line":"                                                           project_id, user_id)"},{"line_number":1833,"context_line":"                        quotas.commit()"},{"line_number":1834,"context_line":"                        # Now the context will be temporarily mutated back to"},{"line_number":1835,"context_line":"                        # the cell from which we want to delete the instance."},{"line_number":1836,"context_line":"                        with nova_context.target_cell(context, cell):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a30ddce_40c354ed","line":1833,"in_reply_to":"9a30ddce_a6727aab","updated":"2017-03-09 13:43:41.000000000","message":"Yeah I thought about something like that, I knew this was hacky but it was late and I wanted to go to bed.","commit_id":"4bddcb475eef83193ba12f20d7a6cc22c09488b1"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"29b96b7038ef00103bdbeb58c15739ba339c2178","unresolved":false,"context_lines":[{"line_number":1821,"context_line":"                    # the cell database. So we temporarily untarget the"},{"line_number":1822,"context_line":"                    # context while we do the quota stuff and re-target after"},{"line_number":1823,"context_line":"                    # we\u0027re done."},{"line_number":1824,"context_line":"                    empty_cell \u003d objects.CellMapping(database_connection\u003dNone)"},{"line_number":1825,"context_line":"                    # We have to get the flavor from the instance while the"},{"line_number":1826,"context_line":"                    # context is still targeted to where the instance lives."},{"line_number":1827,"context_line":"                    with nova_context.target_cell(context, cell):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a30ddce_ec60bfd1","line":1824,"range":{"start_line":1824,"start_character":41,"end_line":1824,"end_character":52},"updated":"2017-03-09 14:38:40.000000000","message":"*** ValueError: Field `database_connection\u0027 cannot be None","commit_id":"44b25f11287ee621a86dfa7a1f0a908f64a45bf3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b6dd25961f4ecc5cf5b1dc5f11f10d0165343170","unresolved":false,"context_lines":[{"line_number":1821,"context_line":"                    # the cell database. So we temporarily untarget the"},{"line_number":1822,"context_line":"                    # context while we do the quota stuff and re-target after"},{"line_number":1823,"context_line":"                    # we\u0027re done."},{"line_number":1824,"context_line":"                    empty_cell \u003d objects.CellMapping(database_connection\u003dNone)"},{"line_number":1825,"context_line":"                    # We have to get the flavor from the instance while the"},{"line_number":1826,"context_line":"                    # context is still targeted to where the instance lives."},{"line_number":1827,"context_line":"                    with nova_context.target_cell(context, cell):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a30ddce_e0e60314","line":1824,"range":{"start_line":1824,"start_character":41,"end_line":1824,"end_character":52},"in_reply_to":"9a30ddce_0c68b386","updated":"2017-03-09 15:08:24.000000000","message":"Oh right the field doesn\u0027t have nullable\u003dTrue.","commit_id":"44b25f11287ee621a86dfa7a1f0a908f64a45bf3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7e18a9d3795db21a5785cbaeb79dc97e2331ddc4","unresolved":false,"context_lines":[{"line_number":1821,"context_line":"                    # the cell database. So we temporarily untarget the"},{"line_number":1822,"context_line":"                    # context while we do the quota stuff and re-target after"},{"line_number":1823,"context_line":"                    # we\u0027re done."},{"line_number":1824,"context_line":"                    empty_cell \u003d objects.CellMapping(database_connection\u003dNone)"},{"line_number":1825,"context_line":"                    # We have to get the flavor from the instance while the"},{"line_number":1826,"context_line":"                    # context is still targeted to where the instance lives."},{"line_number":1827,"context_line":"                    with nova_context.target_cell(context, cell):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a30ddce_4c670b4d","line":1824,"range":{"start_line":1824,"start_character":41,"end_line":1824,"end_character":52},"in_reply_to":"9a30ddce_0c68b386","updated":"2017-03-09 14:53:17.000000000","message":"Using the empty string seems kinda gross.\n\nWhy don\u0027t we just make target_cell() untarget the context if you pass None for the cell_mapping? I\u0027ve wanted that a couple times in tests and it would be trivial to do I think.","commit_id":"44b25f11287ee621a86dfa7a1f0a908f64a45bf3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b6dd25961f4ecc5cf5b1dc5f11f10d0165343170","unresolved":false,"context_lines":[{"line_number":1821,"context_line":"                    # the cell database. So we temporarily untarget the"},{"line_number":1822,"context_line":"                    # context while we do the quota stuff and re-target after"},{"line_number":1823,"context_line":"                    # we\u0027re done."},{"line_number":1824,"context_line":"                    empty_cell \u003d objects.CellMapping(database_connection\u003dNone)"},{"line_number":1825,"context_line":"                    # We have to get the flavor from the instance while the"},{"line_number":1826,"context_line":"                    # context is still targeted to where the instance lives."},{"line_number":1827,"context_line":"                    with nova_context.target_cell(context, cell):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a30ddce_60da134e","line":1824,"range":{"start_line":1824,"start_character":41,"end_line":1824,"end_character":52},"in_reply_to":"9a30ddce_4c670b4d","updated":"2017-03-09 15:08:24.000000000","message":"I could try that.","commit_id":"44b25f11287ee621a86dfa7a1f0a908f64a45bf3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"446c7e6d01e8c3ba752f5aef108dda7e53e47885","unresolved":false,"context_lines":[{"line_number":1821,"context_line":"                    # the cell database. So we temporarily untarget the"},{"line_number":1822,"context_line":"                    # context while we do the quota stuff and re-target after"},{"line_number":1823,"context_line":"                    # we\u0027re done."},{"line_number":1824,"context_line":"                    empty_cell \u003d objects.CellMapping(database_connection\u003dNone)"},{"line_number":1825,"context_line":"                    # We have to get the flavor from the instance while the"},{"line_number":1826,"context_line":"                    # context is still targeted to where the instance lives."},{"line_number":1827,"context_line":"                    with nova_context.target_cell(context, cell):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a30ddce_20ee5bb4","line":1824,"range":{"start_line":1824,"start_character":41,"end_line":1824,"end_character":52},"in_reply_to":"9a30ddce_e0e60314","updated":"2017-03-09 15:10:49.000000000","message":"I just pushed a revision, check it out.","commit_id":"44b25f11287ee621a86dfa7a1f0a908f64a45bf3"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"6154fb7dfe9b6b52c3784eb49e5cd99754d9416b","unresolved":false,"context_lines":[{"line_number":1821,"context_line":"                    # the cell database. So we temporarily untarget the"},{"line_number":1822,"context_line":"                    # context while we do the quota stuff and re-target after"},{"line_number":1823,"context_line":"                    # we\u0027re done."},{"line_number":1824,"context_line":"                    empty_cell \u003d objects.CellMapping(database_connection\u003dNone)"},{"line_number":1825,"context_line":"                    # We have to get the flavor from the instance while the"},{"line_number":1826,"context_line":"                    # context is still targeted to where the instance lives."},{"line_number":1827,"context_line":"                    with nova_context.target_cell(context, cell):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a30ddce_0c68b386","line":1824,"range":{"start_line":1824,"start_character":41,"end_line":1824,"end_character":52},"in_reply_to":"9a30ddce_ec60bfd1","updated":"2017-03-09 14:43:11.000000000","message":"Tested, database_connection\u003d\u0027\u0027 will work :)","commit_id":"44b25f11287ee621a86dfa7a1f0a908f64a45bf3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7e18a9d3795db21a5785cbaeb79dc97e2331ddc4","unresolved":false,"context_lines":[{"line_number":1830,"context_line":"                        quota_flavor \u003d self._get_flavor_for_reservation("},{"line_number":1831,"context_line":"                            instance)"},{"line_number":1832,"context_line":"                    try:"},{"line_number":1833,"context_line":"                        with nova_context.target_cell(context, empty_cell):"},{"line_number":1834,"context_line":"                            # This is confusing but actually decrements quota."},{"line_number":1835,"context_line":"                            quotas \u003d self._create_reservations("},{"line_number":1836,"context_line":"                                context, instance, instance.task_state,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a30ddce_ecc6ff66","line":1833,"updated":"2017-03-09 14:53:17.000000000","message":"Didn\u0027t you and Jay give me the business for not merging two subsequent target contexts? Why not just put the flavor lookup in this try block?","commit_id":"44b25f11287ee621a86dfa7a1f0a908f64a45bf3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"446c7e6d01e8c3ba752f5aef108dda7e53e47885","unresolved":false,"context_lines":[{"line_number":1830,"context_line":"                        quota_flavor \u003d self._get_flavor_for_reservation("},{"line_number":1831,"context_line":"                            instance)"},{"line_number":1832,"context_line":"                    try:"},{"line_number":1833,"context_line":"                        with nova_context.target_cell(context, empty_cell):"},{"line_number":1834,"context_line":"                            # This is confusing but actually decrements quota."},{"line_number":1835,"context_line":"                            quotas \u003d self._create_reservations("},{"line_number":1836,"context_line":"                                context, instance, instance.task_state,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a30ddce_60321361","line":1833,"in_reply_to":"9a30ddce_a085eb07","updated":"2017-03-09 15:10:49.000000000","message":"Heh, duh, pre-coffee.","commit_id":"44b25f11287ee621a86dfa7a1f0a908f64a45bf3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b6dd25961f4ecc5cf5b1dc5f11f10d0165343170","unresolved":false,"context_lines":[{"line_number":1830,"context_line":"                        quota_flavor \u003d self._get_flavor_for_reservation("},{"line_number":1831,"context_line":"                            instance)"},{"line_number":1832,"context_line":"                    try:"},{"line_number":1833,"context_line":"                        with nova_context.target_cell(context, empty_cell):"},{"line_number":1834,"context_line":"                            # This is confusing but actually decrements quota."},{"line_number":1835,"context_line":"                            quotas \u003d self._create_reservations("},{"line_number":1836,"context_line":"                                context, instance, instance.task_state,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a30ddce_a085eb07","line":1833,"in_reply_to":"9a30ddce_ecc6ff66","updated":"2017-03-09 15:08:24.000000000","message":"Because the instance is in cell0 and we\u0027re looking the flavor up via the instance in cell0 (the stashed flavor on the instance). We\u0027re doing the quota stuff in the cell database (nova), so they are different contexts.","commit_id":"44b25f11287ee621a86dfa7a1f0a908f64a45bf3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d4a04f91cfb0be0642afe0b0a8a064e487d7cd1a","unresolved":false,"context_lines":[{"line_number":1844,"context_line":"                            instance.destroy()"},{"line_number":1845,"context_line":"                            return"},{"line_number":1846,"context_line":"                    except exception.InstanceNotFound:"},{"line_number":1847,"context_line":"                        with nova_context.target_cell(context, empty_cell):"},{"line_number":1848,"context_line":"                            quotas.rollback()"},{"line_number":1849,"context_line":"                if not instance:"},{"line_number":1850,"context_line":"                    # Instance is already deleted."}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_20e1fb9a","line":1847,"range":{"start_line":1847,"start_character":63,"end_line":1847,"end_character":73},"updated":"2017-03-09 15:13:52.000000000","message":"kablammo","commit_id":"48f2f25093204932df9123771456e1337b94e1a5"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"872e501f3cca13b76be794654563f943147cbe5a","unresolved":false,"context_lines":[{"line_number":1833,"context_line":"                    try:"},{"line_number":1834,"context_line":"                        with nova_context.target_cell(context, None):"},{"line_number":1835,"context_line":"                            # This is confusing but actually decrements quota."},{"line_number":1836,"context_line":"                            quotas \u003d self._create_reservations("},{"line_number":1837,"context_line":"                                context, instance, instance.task_state,"},{"line_number":1838,"context_line":"                                project_id, user_id, flavor\u003dquota_flavor)"},{"line_number":1839,"context_line":"                            quotas.commit()"},{"line_number":1840,"context_line":"                        # Now destroy the instance from the cell it lives in."},{"line_number":1841,"context_line":"                        with nova_context.target_cell(context, cell):"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a30ddce_35f8497b","line":1838,"range":{"start_line":1836,"start_character":28,"end_line":1838,"end_character":73},"updated":"2017-03-09 21:18:24.000000000","message":"Ditto.\n\nCan the commit raise?","commit_id":"93fd927b1a7f77d2390bed9a637a36aec0c2feb5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a8cf718020b9f2ce2fe5eb94b726225da3aaf710","unresolved":false,"context_lines":[{"line_number":1833,"context_line":"                    try:"},{"line_number":1834,"context_line":"                        with nova_context.target_cell(context, None):"},{"line_number":1835,"context_line":"                            # This is confusing but actually decrements quota."},{"line_number":1836,"context_line":"                            quotas \u003d self._create_reservations("},{"line_number":1837,"context_line":"                                context, instance, instance.task_state,"},{"line_number":1838,"context_line":"                                project_id, user_id, flavor\u003dquota_flavor)"},{"line_number":1839,"context_line":"                            quotas.commit()"},{"line_number":1840,"context_line":"                        # Now destroy the instance from the cell it lives in."},{"line_number":1841,"context_line":"                        with nova_context.target_cell(context, cell):"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a30ddce_5ca9627a","line":1838,"range":{"start_line":1836,"start_character":28,"end_line":1838,"end_character":73},"in_reply_to":"9a30ddce_35f8497b","updated":"2017-03-09 22:38:57.000000000","message":"Not any more than they would from _delete_while_booting.","commit_id":"93fd927b1a7f77d2390bed9a637a36aec0c2feb5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a8cf718020b9f2ce2fe5eb94b726225da3aaf710","unresolved":false,"context_lines":[{"line_number":1971,"context_line":"                # isn\u0027t up, delete instance from db and clean bdms info and"},{"line_number":1972,"context_line":"                # network info"},{"line_number":1973,"context_line":"                self._local_delete(context, instance, bdms, delete_type, cb)"},{"line_number":1974,"context_line":"                quotas.commit()"},{"line_number":1975,"context_line":""},{"line_number":1976,"context_line":"        except exception.InstanceNotFound:"},{"line_number":1977,"context_line":"            # NOTE(comstud): Race condition. Instance already gone."}],"source_content_type":"text/x-python","patch_set":8,"id":"9a30ddce_fcccd66b","line":1974,"updated":"2017-03-09 22:38:57.000000000","message":"Or here.","commit_id":"93fd927b1a7f77d2390bed9a637a36aec0c2feb5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5b41216bbe226ac84bcb4ff13ba185fef789dcfd","unresolved":false,"context_lines":[{"line_number":1832,"context_line":"                        # If the instance has the targeted context in it then"},{"line_number":1833,"context_line":"                        # we don\u0027t need the context manager."},{"line_number":1834,"context_line":"                        quota_flavor \u003d self._get_flavor_for_reservation("},{"line_number":1835,"context_line":"                            instance)"},{"line_number":1836,"context_line":""},{"line_number":1837,"context_line":"                    with nova_context.target_cell(context, None):"},{"line_number":1838,"context_line":"                        # This is confusing but actually decrements quota usage"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a30ddce_9600bf03","line":1835,"updated":"2017-03-13 15:42:02.000000000","message":"I don\u0027t think you assert that this is called anywhere in the tests, so we don\u0027t know that it\u0027s going to revert the old_flavor in the case of the logic in this helper. Perhaps just assert that it is calling it, and then do a quick unit test of the helper to validate the logic?","commit_id":"1d0984a8fe60b50a1ac9d687bb71deac1dee4b20"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d91b8677c785c0c98fefa248422341ace5bd4884","unresolved":false,"context_lines":[{"line_number":1832,"context_line":"                        # If the instance has the targeted context in it then"},{"line_number":1833,"context_line":"                        # we don\u0027t need the context manager."},{"line_number":1834,"context_line":"                        quota_flavor \u003d self._get_flavor_for_reservation("},{"line_number":1835,"context_line":"                            instance)"},{"line_number":1836,"context_line":""},{"line_number":1837,"context_line":"                    with nova_context.target_cell(context, None):"},{"line_number":1838,"context_line":"                        # This is confusing but actually decrements quota usage"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a30ddce_79c7bc63","line":1835,"in_reply_to":"9a30ddce_59491830","updated":"2017-03-13 15:52:04.000000000","message":"Right, but if i change this line to:\n\n quota_flavor \u003d instance.flavor\n\nit will still pass all the tests, but it will do the wrong thing in the case that this is deleted during resize right?","commit_id":"1d0984a8fe60b50a1ac9d687bb71deac1dee4b20"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cae58926181d36516f121b2bb48c1223c142c760","unresolved":false,"context_lines":[{"line_number":1832,"context_line":"                        # If the instance has the targeted context in it then"},{"line_number":1833,"context_line":"                        # we don\u0027t need the context manager."},{"line_number":1834,"context_line":"                        quota_flavor \u003d self._get_flavor_for_reservation("},{"line_number":1835,"context_line":"                            instance)"},{"line_number":1836,"context_line":""},{"line_number":1837,"context_line":"                    with nova_context.target_cell(context, None):"},{"line_number":1838,"context_line":"                        # This is confusing but actually decrements quota usage"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a30ddce_6f57ee25","line":1835,"in_reply_to":"9a30ddce_79c7bc63","updated":"2017-03-13 16:40:42.000000000","message":"OK, I can mock this out in the _delete tests and add a couple of unit tests for specifically testing the logic in _get_flavor_for_reservation.","commit_id":"1d0984a8fe60b50a1ac9d687bb71deac1dee4b20"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a9616593b5ed9f6ce972cedc4bfa35ed27e3146e","unresolved":false,"context_lines":[{"line_number":1832,"context_line":"                        # If the instance has the targeted context in it then"},{"line_number":1833,"context_line":"                        # we don\u0027t need the context manager."},{"line_number":1834,"context_line":"                        quota_flavor \u003d self._get_flavor_for_reservation("},{"line_number":1835,"context_line":"                            instance)"},{"line_number":1836,"context_line":""},{"line_number":1837,"context_line":"                    with nova_context.target_cell(context, None):"},{"line_number":1838,"context_line":"                        # This is confusing but actually decrements quota usage"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a30ddce_59491830","line":1835,"in_reply_to":"9a30ddce_9600bf03","updated":"2017-03-13 15:48:32.000000000","message":"I don\u0027t assert that it\u0027s called directly because it\u0027s not mocked out, and I don\u0027t think it needs to be mocked out. We know it\u0027s being used because of the assertion on passing instance.flavor to _create_reservations:\n\nhttps://review.openstack.org/#/c/443403/11/nova/tests/unit/compute/test_compute_api.py@1518","commit_id":"1d0984a8fe60b50a1ac9d687bb71deac1dee4b20"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5b41216bbe226ac84bcb4ff13ba185fef789dcfd","unresolved":false,"context_lines":[{"line_number":2054,"context_line":"        #                are updated to new instance type, we should use"},{"line_number":2055,"context_line":"        #                the old instance type to create reservation."},{"line_number":2056,"context_line":"        # see https://bugs.launchpad.net/nova/+bug/1099729 for more details"},{"line_number":2057,"context_line":"        if original_task_state in (task_states.RESIZE_MIGRATED,"},{"line_number":2058,"context_line":"                                   task_states.RESIZE_FINISH):"},{"line_number":2059,"context_line":"            old_flavor \u003d flavor or instance.old_flavor"},{"line_number":2060,"context_line":"            instance_vcpus \u003d old_flavor.vcpus"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a30ddce_393d44c4","line":2057,"updated":"2017-03-13 15:42:02.000000000","message":"Couldn\u0027t you re-use the helper in the case that flavor wasn\u0027t passed in and avoid this duplicated logic?","commit_id":"1d0984a8fe60b50a1ac9d687bb71deac1dee4b20"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a9616593b5ed9f6ce972cedc4bfa35ed27e3146e","unresolved":false,"context_lines":[{"line_number":2054,"context_line":"        #                are updated to new instance type, we should use"},{"line_number":2055,"context_line":"        #                the old instance type to create reservation."},{"line_number":2056,"context_line":"        # see https://bugs.launchpad.net/nova/+bug/1099729 for more details"},{"line_number":2057,"context_line":"        if original_task_state in (task_states.RESIZE_MIGRATED,"},{"line_number":2058,"context_line":"                                   task_states.RESIZE_FINISH):"},{"line_number":2059,"context_line":"            old_flavor \u003d flavor or instance.old_flavor"},{"line_number":2060,"context_line":"            instance_vcpus \u003d old_flavor.vcpus"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a30ddce_59c2f885","line":2057,"in_reply_to":"9a30ddce_393d44c4","updated":"2017-03-13 15:48:32.000000000","message":"I could, but we still need the task_state logic for calculating instance_memory_mb since it\u0027s different for a migration. Suggestions there?","commit_id":"1d0984a8fe60b50a1ac9d687bb71deac1dee4b20"}],"nova/context.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0715d09f32f5baa1322cdace61dbb453637a7776","unresolved":false,"context_lines":[{"line_number":358,"context_line":"            raise exception.Forbidden()"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"def set_target_cell(context, cell_mapping):"},{"line_number":362,"context_line":"    \"\"\"Adds database connection information to the context"},{"line_number":363,"context_line":"    for communicating with the given target_cell."},{"line_number":364,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9a30ddce_c3974946","line":361,"range":{"start_line":361,"start_character":4,"end_line":361,"end_character":19},"updated":"2017-03-09 15:27:49.000000000","message":"This is the wrong method, we want target_cell.","commit_id":"c71c177b3f8e2fdf6622ec3d118ab252e826eee8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1aeaa0b801406e1d9fb05fc184f00263307bdba9","unresolved":false,"context_lines":[{"line_number":358,"context_line":"            raise exception.Forbidden()"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"def set_target_cell(context, cell_mapping):"},{"line_number":362,"context_line":"    \"\"\"Adds database connection information to the context"},{"line_number":363,"context_line":"    for communicating with the given target_cell."},{"line_number":364,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9a30ddce_43b8b9c8","line":361,"range":{"start_line":361,"start_character":4,"end_line":361,"end_character":19},"in_reply_to":"9a30ddce_c3974946","updated":"2017-03-09 15:29:38.000000000","message":"Oh I see, target_cell calls set_target_cell.","commit_id":"c71c177b3f8e2fdf6622ec3d118ab252e826eee8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1aeaa0b801406e1d9fb05fc184f00263307bdba9","unresolved":false,"context_lines":[{"line_number":365,"context_line":"    This is used for permanently targeting a cell in a context."},{"line_number":366,"context_line":"    Use this when you want all subsequent code to target a cell."},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"    Passing None for cell_mapping will untarget the context."},{"line_number":369,"context_line":""},{"line_number":370,"context_line":"    :param context: The RequestContext to add connection information"},{"line_number":371,"context_line":"    :param cell_mapping: An objects.CellMapping object or None"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a30ddce_83ae317f","line":368,"updated":"2017-03-09 15:29:38.000000000","message":"We should put this in the docstring for target_cell too.","commit_id":"c71c177b3f8e2fdf6622ec3d118ab252e826eee8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5c6e2a04ff7e90b4a6e44868c580aa20c02fd683","unresolved":false,"context_lines":[{"line_number":388,"context_line":"    This context manager makes a temporary change to the context"},{"line_number":389,"context_line":"    and restores it when complete."},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"    Passing None for cell_mapping will untarget the context."},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"    :param context: The RequestContext to add connection information"},{"line_number":394,"context_line":"    :param cell_mapping: An objects.CellMapping object or None"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a30ddce_c90f6386","line":391,"updated":"2017-03-09 17:41:26.000000000","message":"This might be more correct as:\n\n Passing a none mapping will untarget the context temporarily\n\nJust to make it distinct from above, which implies \"forever\"","commit_id":"efe6b3171a90a3855e9af4554922524c90cf0af2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9e0ca74c2b116b615635bd01d9ecf3d6e76b8b57","unresolved":false,"context_lines":[{"line_number":388,"context_line":"    This context manager makes a temporary change to the context"},{"line_number":389,"context_line":"    and restores it when complete."},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"    Passing None for cell_mapping will untarget the context."},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"    :param context: The RequestContext to add connection information"},{"line_number":394,"context_line":"    :param cell_mapping: An objects.CellMapping object or None"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a30ddce_5ceec2b4","line":391,"in_reply_to":"9a30ddce_c90f6386","updated":"2017-03-09 22:38:53.000000000","message":"Done","commit_id":"efe6b3171a90a3855e9af4554922524c90cf0af2"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"872e501f3cca13b76be794654563f943147cbe5a","unresolved":false,"context_lines":[{"line_number":371,"context_line":"    :param cell_mapping: An objects.CellMapping object or None"},{"line_number":372,"context_line":"    \"\"\""},{"line_number":373,"context_line":"    # avoid circular import"},{"line_number":374,"context_line":"    from nova import db"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"    if cell_mapping is not None:"},{"line_number":377,"context_line":"        db_connection_string \u003d cell_mapping.database_connection"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a30ddce_356da931","line":374,"range":{"start_line":374,"start_character":0,"end_line":374,"end_character":23},"updated":"2017-03-09 21:18:24.000000000","message":"This can go inside the first branch of the conditional.  Save a little churn.","commit_id":"93fd927b1a7f77d2390bed9a637a36aec0c2feb5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a8cf718020b9f2ce2fe5eb94b726225da3aaf710","unresolved":false,"context_lines":[{"line_number":371,"context_line":"    :param cell_mapping: An objects.CellMapping object or None"},{"line_number":372,"context_line":"    \"\"\""},{"line_number":373,"context_line":"    # avoid circular import"},{"line_number":374,"context_line":"    from nova import db"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"    if cell_mapping is not None:"},{"line_number":377,"context_line":"        db_connection_string \u003d cell_mapping.database_connection"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a30ddce_bce5fedc","line":374,"range":{"start_line":374,"start_character":0,"end_line":374,"end_character":23},"in_reply_to":"9a30ddce_356da931","updated":"2017-03-09 22:38:57.000000000","message":"Done","commit_id":"93fd927b1a7f77d2390bed9a637a36aec0c2feb5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5b41216bbe226ac84bcb4ff13ba185fef789dcfd","unresolved":false,"context_lines":[{"line_number":381,"context_line":"                cell_mapping.transport_url)"},{"line_number":382,"context_line":"    else:"},{"line_number":383,"context_line":"        context.db_connection \u003d None"},{"line_number":384,"context_line":"        context.mq_connection \u003d None"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"@contextmanager"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a30ddce_36b39399","line":384,"updated":"2017-03-13 15:42:02.000000000","message":"Don\u0027t we need a unit test for this new behavior? Like this:\n\nhttps://github.com/openstack/nova/blob/master/nova/tests/unit/test_context.py#L307-L310","commit_id":"1d0984a8fe60b50a1ac9d687bb71deac1dee4b20"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a9616593b5ed9f6ce972cedc4bfa35ed27e3146e","unresolved":false,"context_lines":[{"line_number":381,"context_line":"                cell_mapping.transport_url)"},{"line_number":382,"context_line":"    else:"},{"line_number":383,"context_line":"        context.db_connection \u003d None"},{"line_number":384,"context_line":"        context.mq_connection \u003d None"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"@contextmanager"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a30ddce_796b3c85","line":384,"in_reply_to":"9a30ddce_36b39399","updated":"2017-03-13 15:48:32.000000000","message":"I can, although the functional test:\n\nnova/tests/functional/regressions/test_bug_1670627.py\n\nWill cover that this works, so a unit test is a bit redundant. But I can add one if you want.","commit_id":"1d0984a8fe60b50a1ac9d687bb71deac1dee4b20"}],"nova/tests/functional/regressions/test_bug_1670627.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"872e501f3cca13b76be794654563f943147cbe5a","unresolved":false,"context_lines":[{"line_number":126,"context_line":"        self._delete_server(server[\u0027id\u0027])"},{"line_number":127,"context_line":"        self._wait_for_instance_delete(server[\u0027id\u0027])"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        # Now check the quota again. When the bug is fixed, ending usage should"},{"line_number":130,"context_line":"        # be current usage - 1."},{"line_number":131,"context_line":"        ending_usage \u003d self.api.get_limits()"},{"line_number":132,"context_line":"        self.assertEqual(current_usage[\u0027absolute\u0027][\u0027totalInstancesUsed\u0027] - 1,"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a30ddce_d5e60daf","line":129,"range":{"start_line":129,"start_character":37,"end_line":129,"end_character":41},"updated":"2017-03-09 21:18:24.000000000","message":"\u0027Since\u0027?  (Though I\u0027m not sure you need to reference the bug that\u0027s now fixed.  Or if you do, shouldn\u0027t you say which one it was?)","commit_id":"93fd927b1a7f77d2390bed9a637a36aec0c2feb5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a8cf718020b9f2ce2fe5eb94b726225da3aaf710","unresolved":false,"context_lines":[{"line_number":126,"context_line":"        self._delete_server(server[\u0027id\u0027])"},{"line_number":127,"context_line":"        self._wait_for_instance_delete(server[\u0027id\u0027])"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        # Now check the quota again. When the bug is fixed, ending usage should"},{"line_number":130,"context_line":"        # be current usage - 1."},{"line_number":131,"context_line":"        ending_usage \u003d self.api.get_limits()"},{"line_number":132,"context_line":"        self.assertEqual(current_usage[\u0027absolute\u0027][\u0027totalInstancesUsed\u0027] - 1,"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a30ddce_9c0bda8d","line":129,"range":{"start_line":129,"start_character":37,"end_line":129,"end_character":41},"in_reply_to":"9a30ddce_d5e60daf","updated":"2017-03-09 22:38:57.000000000","message":"Yeah \u0027Since\u0027 is better here.","commit_id":"93fd927b1a7f77d2390bed9a637a36aec0c2feb5"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"23a36395a65a3cbf0466722cf75498fe694e9143","unresolved":false,"context_lines":[{"line_number":132,"context_line":"        # Now check the quota again. Since the bug is fixed, ending usage"},{"line_number":133,"context_line":"        # should be current usage - 1."},{"line_number":134,"context_line":"        ending_usage \u003d self.api.get_limits()"},{"line_number":135,"context_line":"        self.assertEqual(current_usage[\u0027absolute\u0027][\u0027totalInstancesUsed\u0027] - 1,"},{"line_number":136,"context_line":"                         ending_usage[\u0027absolute\u0027][\u0027totalInstancesUsed\u0027])"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a30ddce_df7a83bc","line":135,"range":{"start_line":135,"start_character":25,"end_line":135,"end_character":76},"updated":"2017-03-10 16:04:48.000000000","message":"or starting_usage, might be clearer.","commit_id":"1141e23ab6271de93207db6a2e3701265a37c3bc"}]}