)]}'
{"doc/source/cli/placement-manage.rst":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"84fb227bdc9daf9137d168435918964a08138707","unresolved":false,"context_lines":[{"line_number":74,"context_line":"``placement-manage db online_data_migrations [--max-count]``"},{"line_number":75,"context_line":"   Perform data migration to update all live data."},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"   ``--max-count`` controls the maximum number of objects to migrate in a given"},{"line_number":78,"context_line":"   call. If not specified, migration will occur in batches of 50 until fully"},{"line_number":79,"context_line":"   complete."},{"line_number":80,"context_line":""}],"source_content_type":"text/x-rst","patch_set":7,"id":"bfdaf3ff_f2876bb4","line":77,"range":{"start_line":77,"start_character":50,"end_line":77,"end_character":57},"updated":"2019-01-18 17:14:58.000000000","message":"Does objects make sense here, or should it be \"rows\" or \"records\"?\n\nOf course I guess rows doesn\u0027t quite make sense in situations where there are complex joins?","commit_id":"80fa50187add026578ea204578289ea077ebad32"}],"placement/cmd/manage.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"da01dd278ce9f93f33e946a8693568c0a2504763","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        migration.stamp(self.config.command.version)"},{"line_number":44,"context_line":"        return 0"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def db_online_data_migrations(self):"},{"line_number":47,"context_line":"        max_count \u003d self.config.command.max_count"},{"line_number":48,"context_line":"        if max_count is not None:"},{"line_number":49,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_eeb9fce8","line":46,"updated":"2019-01-16 16:25:21.000000000","message":"Can we have a function doc about the return codes meaning?","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"9701dec828cf8c76954779e96bb0588b601683df","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        migration.stamp(self.config.command.version)"},{"line_number":44,"context_line":"        return 0"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def db_online_data_migrations(self):"},{"line_number":47,"context_line":"        max_count \u003d self.config.command.max_count"},{"line_number":48,"context_line":"        if max_count is not None:"},{"line_number":49,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_12031a75","line":46,"in_reply_to":"bfdaf3ff_554af6ab","updated":"2019-01-18 07:53:41.000000000","message":"Thanks a lot. LGTM.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"47420a743a0037f59584f4d586f2cae94453caeb","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        migration.stamp(self.config.command.version)"},{"line_number":44,"context_line":"        return 0"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def db_online_data_migrations(self):"},{"line_number":47,"context_line":"        max_count \u003d self.config.command.max_count"},{"line_number":48,"context_line":"        if max_count is not None:"},{"line_number":49,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_554af6ab","line":46,"in_reply_to":"bfdaf3ff_a083f1cb","updated":"2019-01-17 18:57:41.000000000","message":"I would also try to mirror the CLI docs from nova for the command:\n\nhttps://github.com/openstack/nova/blob/master/doc/source/cli/nova-manage.rst\n\nThose would go somewhere in:\n\nhttps://github.com/openstack/placement/blob/master/doc/source/cli/placement-manage.rst#placement-database","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"161052ef2fa71bde91bee8deeea3777a456b652e","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        migration.stamp(self.config.command.version)"},{"line_number":44,"context_line":"        return 0"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def db_online_data_migrations(self):"},{"line_number":47,"context_line":"        max_count \u003d self.config.command.max_count"},{"line_number":48,"context_line":"        if max_count is not None:"},{"line_number":49,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_a083f1cb","line":46,"in_reply_to":"bfdaf3ff_eeb9fce8","updated":"2019-01-17 12:44:54.000000000","message":"Done","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"da01dd278ce9f93f33e946a8693568c0a2504763","unresolved":false,"context_lines":[{"line_number":50,"context_line":"                max_count \u003d int(max_count)"},{"line_number":51,"context_line":"            except ValueError:"},{"line_number":52,"context_line":"                max_count \u003d -1"},{"line_number":53,"context_line":"            unlimited \u003d False"},{"line_number":54,"context_line":"            if max_count \u003c 1:"},{"line_number":55,"context_line":"                print(_(\u0027Must supply a positive value for max_number\u0027))"},{"line_number":56,"context_line":"                return 127"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_8e22b051","line":53,"updated":"2019-01-16 16:25:21.000000000","message":"hm, unlimited \u003d False logically belongs under L50 where we know that we have a limit or go under the if at L54 where we know that we have a _good_ limit.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"161052ef2fa71bde91bee8deeea3777a456b652e","unresolved":false,"context_lines":[{"line_number":50,"context_line":"                max_count \u003d int(max_count)"},{"line_number":51,"context_line":"            except ValueError:"},{"line_number":52,"context_line":"                max_count \u003d -1"},{"line_number":53,"context_line":"            unlimited \u003d False"},{"line_number":54,"context_line":"            if max_count \u003c 1:"},{"line_number":55,"context_line":"                print(_(\u0027Must supply a positive value for max_number\u0027))"},{"line_number":56,"context_line":"                return 127"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_bd49da8b","line":53,"in_reply_to":"bfdaf3ff_8e22b051","updated":"2019-01-17 12:44:54.000000000","message":"Done. And I turned it to \"limited\" to avoid double negative. \ni.e.) \"Not unlimited\" is too difficult for me.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"47420a743a0037f59584f4d586f2cae94453caeb","unresolved":false,"context_lines":[{"line_number":50,"context_line":"                max_count \u003d int(max_count)"},{"line_number":51,"context_line":"            except ValueError:"},{"line_number":52,"context_line":"                max_count \u003d -1"},{"line_number":53,"context_line":"            unlimited \u003d False"},{"line_number":54,"context_line":"            if max_count \u003c 1:"},{"line_number":55,"context_line":"                print(_(\u0027Must supply a positive value for max_number\u0027))"},{"line_number":56,"context_line":"                return 127"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_d5a10648","line":53,"in_reply_to":"bfdaf3ff_bd49da8b","updated":"2019-01-17 18:57:41.000000000","message":"If we\u0027re just copying stuff from nova, it would be easier as a reviewer IMO to copy what\u0027s in nova and then clean it up if we want, or make these cleanup changes in nova so that when we copy it over to placement it\u0027s easier to compare, because this command has had bugs in nova so I think it might be hard to review if we refactor it while also copying 95% of it over (it will be hard to notice the differences).\n\nSo really I would try to do the easiest thing which is copy it over as-is and then refactor/cleanup separately so we understand the differences being made from the original.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"da01dd278ce9f93f33e946a8693568c0a2504763","unresolved":false,"context_lines":[{"line_number":52,"context_line":"                max_count \u003d -1"},{"line_number":53,"context_line":"            unlimited \u003d False"},{"line_number":54,"context_line":"            if max_count \u003c 1:"},{"line_number":55,"context_line":"                print(_(\u0027Must supply a positive value for max_number\u0027))"},{"line_number":56,"context_line":"                return 127"},{"line_number":57,"context_line":"        else:"},{"line_number":58,"context_line":"            unlimited \u003d True"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_cedd1843","line":55,"range":{"start_line":55,"start_character":58,"end_line":55,"end_character":68},"updated":"2019-01-16 16:25:21.000000000","message":"I think it is max-count on the CLI","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"47420a743a0037f59584f4d586f2cae94453caeb","unresolved":false,"context_lines":[{"line_number":52,"context_line":"                max_count \u003d -1"},{"line_number":53,"context_line":"            unlimited \u003d False"},{"line_number":54,"context_line":"            if max_count \u003c 1:"},{"line_number":55,"context_line":"                print(_(\u0027Must supply a positive value for max_number\u0027))"},{"line_number":56,"context_line":"                return 127"},{"line_number":57,"context_line":"        else:"},{"line_number":58,"context_line":"            unlimited \u003d True"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_95029e3c","line":55,"range":{"start_line":55,"start_character":58,"end_line":55,"end_character":68},"in_reply_to":"bfdaf3ff_3d7c0ab4","updated":"2019-01-17 18:57:41.000000000","message":"This is all mostly just copied from nova, right? So yeah this is wrong in nova as well:\n\nhttps://github.com/openstack/nova/blob/master/nova/cmd/manage.py#L715","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"161052ef2fa71bde91bee8deeea3777a456b652e","unresolved":false,"context_lines":[{"line_number":52,"context_line":"                max_count \u003d -1"},{"line_number":53,"context_line":"            unlimited \u003d False"},{"line_number":54,"context_line":"            if max_count \u003c 1:"},{"line_number":55,"context_line":"                print(_(\u0027Must supply a positive value for max_number\u0027))"},{"line_number":56,"context_line":"                return 127"},{"line_number":57,"context_line":"        else:"},{"line_number":58,"context_line":"            unlimited \u003d True"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_3d7c0ab4","line":55,"range":{"start_line":55,"start_character":58,"end_line":55,"end_character":68},"in_reply_to":"bfdaf3ff_cedd1843","updated":"2019-01-17 12:44:54.000000000","message":"Done","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"da01dd278ce9f93f33e946a8693568c0a2504763","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        ran \u003d None"},{"line_number":63,"context_line":"        migration_info \u003d {}"},{"line_number":64,"context_line":"        exceptions \u003d False"},{"line_number":65,"context_line":"        while ran is None or ran !\u003d 0:"},{"line_number":66,"context_line":"            migrations, exceptions \u003d migration.run_online_migration(max_count)"},{"line_number":67,"context_line":"            ran \u003d 0"},{"line_number":68,"context_line":"            # For each batch of migration method results, build the cumulative"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_617c3db5","line":65,"range":{"start_line":65,"start_character":29,"end_line":65,"end_character":38},"updated":"2019-01-16 16:25:21.000000000","message":"Is it here to stop when there are no progress any more?","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"161052ef2fa71bde91bee8deeea3777a456b652e","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        ran \u003d None"},{"line_number":63,"context_line":"        migration_info \u003d {}"},{"line_number":64,"context_line":"        exceptions \u003d False"},{"line_number":65,"context_line":"        while ran is None or ran !\u003d 0:"},{"line_number":66,"context_line":"            migrations, exceptions \u003d migration.run_online_migration(max_count)"},{"line_number":67,"context_line":"            ran \u003d 0"},{"line_number":68,"context_line":"            # For each batch of migration method results, build the cumulative"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_80934da3","line":65,"range":{"start_line":65,"start_character":29,"end_line":65,"end_character":38},"in_reply_to":"bfdaf3ff_617c3db5","updated":"2019-01-17 12:44:54.000000000","message":"Yes, if no there is no more records to update ran is 0 and we can go forward.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"47420a743a0037f59584f4d586f2cae94453caeb","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        ran \u003d None"},{"line_number":63,"context_line":"        migration_info \u003d {}"},{"line_number":64,"context_line":"        exceptions \u003d False"},{"line_number":65,"context_line":"        while ran is None or ran !\u003d 0:"},{"line_number":66,"context_line":"            migrations, exceptions \u003d migration.run_online_migration(max_count)"},{"line_number":67,"context_line":"            ran \u003d 0"},{"line_number":68,"context_line":"            # For each batch of migration method results, build the cumulative"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_95b05e8c","line":65,"range":{"start_line":65,"start_character":29,"end_line":65,"end_character":38},"in_reply_to":"bfdaf3ff_80934da3","updated":"2019-01-17 18:57:41.000000000","message":"Again, this is copied from nova, so nothing should really be new here. If we want to add code comments in a follow up, that would be good.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"da01dd278ce9f93f33e946a8693568c0a2504763","unresolved":false,"context_lines":[{"line_number":68,"context_line":"            # For each batch of migration method results, build the cumulative"},{"line_number":69,"context_line":"            # set of results."},{"line_number":70,"context_line":"            for name in migrations:"},{"line_number":71,"context_line":"                migration_info.setdefault(name, (0, 0))"},{"line_number":72,"context_line":"                migration_info[name] \u003d ("},{"line_number":73,"context_line":"                    migration_info[name][0] + migrations[name][0],"},{"line_number":74,"context_line":"                    migration_info[name][1] + migrations[name][1],"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_2199551e","line":71,"updated":"2019-01-16 16:25:21.000000000","message":"you can define migration_info as a default dict at L63:\nmigration_info \u003d collections.defaultdict(lambda: (0, 0))","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"161052ef2fa71bde91bee8deeea3777a456b652e","unresolved":false,"context_lines":[{"line_number":68,"context_line":"            # For each batch of migration method results, build the cumulative"},{"line_number":69,"context_line":"            # set of results."},{"line_number":70,"context_line":"            for name in migrations:"},{"line_number":71,"context_line":"                migration_info.setdefault(name, (0, 0))"},{"line_number":72,"context_line":"                migration_info[name] \u003d ("},{"line_number":73,"context_line":"                    migration_info[name][0] + migrations[name][0],"},{"line_number":74,"context_line":"                    migration_info[name][1] + migrations[name][1],"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_20e70143","line":71,"in_reply_to":"bfdaf3ff_2199551e","updated":"2019-01-17 12:44:54.000000000","message":"I left this because I chose to use OrderedDict according to your comment on L83.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"da01dd278ce9f93f33e946a8693568c0a2504763","unresolved":false,"context_lines":[{"line_number":78,"context_line":"                break"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        t \u003d prettytable.PrettyTable([_(\u0027Migration\u0027),"},{"line_number":81,"context_line":"                                     _(\u0027Total Needed\u0027),  # Really: Total Found"},{"line_number":82,"context_line":"                                     _(\u0027Completed\u0027)])"},{"line_number":83,"context_line":"        for name in sorted(migration_info.keys()):"},{"line_number":84,"context_line":"            info \u003d migration_info[name]"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_e13eedd8","line":81,"range":{"start_line":81,"start_character":37,"end_line":81,"end_character":78},"updated":"2019-01-16 16:25:21.000000000","message":"Can we just call it \"Total Found\" then?","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"47420a743a0037f59584f4d586f2cae94453caeb","unresolved":false,"context_lines":[{"line_number":78,"context_line":"                break"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        t \u003d prettytable.PrettyTable([_(\u0027Migration\u0027),"},{"line_number":81,"context_line":"                                     _(\u0027Total Needed\u0027),  # Really: Total Found"},{"line_number":82,"context_line":"                                     _(\u0027Completed\u0027)])"},{"line_number":83,"context_line":"        for name in sorted(migration_info.keys()):"},{"line_number":84,"context_line":"            info \u003d migration_info[name]"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_955e1e42","line":81,"range":{"start_line":81,"start_character":37,"end_line":81,"end_character":78},"in_reply_to":"bfdaf3ff_40e24531","updated":"2019-01-17 18:57:41.000000000","message":"Both nova and cinder (which copied from nova) use Total Needed:\n\nhttps://github.com/openstack/cinder/blob/master/cinder/cmd/manage.py#L407\n\nBut yeah total found is probably better here if we\u0027re adding a new command; changing the title in nova could be considered a change in behavior.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"161052ef2fa71bde91bee8deeea3777a456b652e","unresolved":false,"context_lines":[{"line_number":78,"context_line":"                break"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        t \u003d prettytable.PrettyTable([_(\u0027Migration\u0027),"},{"line_number":81,"context_line":"                                     _(\u0027Total Needed\u0027),  # Really: Total Found"},{"line_number":82,"context_line":"                                     _(\u0027Completed\u0027)])"},{"line_number":83,"context_line":"        for name in sorted(migration_info.keys()):"},{"line_number":84,"context_line":"            info \u003d migration_info[name]"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_40e24531","line":81,"range":{"start_line":81,"start_character":37,"end_line":81,"end_character":78},"in_reply_to":"bfdaf3ff_e13eedd8","updated":"2019-01-17 12:44:54.000000000","message":"Done","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"da01dd278ce9f93f33e946a8693568c0a2504763","unresolved":false,"context_lines":[{"line_number":80,"context_line":"        t \u003d prettytable.PrettyTable([_(\u0027Migration\u0027),"},{"line_number":81,"context_line":"                                     _(\u0027Total Needed\u0027),  # Really: Total Found"},{"line_number":82,"context_line":"                                     _(\u0027Completed\u0027)])"},{"line_number":83,"context_line":"        for name in sorted(migration_info.keys()):"},{"line_number":84,"context_line":"            info \u003d migration_info[name]"},{"line_number":85,"context_line":"            t.add_row([name, info[0], info[1]])"},{"line_number":86,"context_line":"        print(t)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_0400df00","line":83,"range":{"start_line":83,"start_character":20,"end_line":83,"end_character":48},"updated":"2019-01-16 16:25:21.000000000","message":"the alphabetical order is not the logical order of the migrations. I think it would be more logical to print the migration info in the order of their execution which is defined in online_migrations.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"161052ef2fa71bde91bee8deeea3777a456b652e","unresolved":false,"context_lines":[{"line_number":80,"context_line":"        t \u003d prettytable.PrettyTable([_(\u0027Migration\u0027),"},{"line_number":81,"context_line":"                                     _(\u0027Total Needed\u0027),  # Really: Total Found"},{"line_number":82,"context_line":"                                     _(\u0027Completed\u0027)])"},{"line_number":83,"context_line":"        for name in sorted(migration_info.keys()):"},{"line_number":84,"context_line":"            info \u003d migration_info[name]"},{"line_number":85,"context_line":"            t.add_row([name, info[0], info[1]])"},{"line_number":86,"context_line":"        print(t)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_13c0157b","line":83,"range":{"start_line":83,"start_character":20,"end_line":83,"end_character":48},"in_reply_to":"bfdaf3ff_0400df00","updated":"2019-01-17 12:44:54.000000000","message":"Sounds good to me. I changed this migration_info to explicit ordered dict for python2.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"da01dd278ce9f93f33e946a8693568c0a2504763","unresolved":false,"context_lines":[{"line_number":97,"context_line":"                    \"details.\"))"},{"line_number":98,"context_line":"            return 2"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"        # TODO(mriedem): Potentially add another return code for"},{"line_number":101,"context_line":"        # \"there are more migrations, but not completable right now\""},{"line_number":102,"context_line":"        return ran and 1 or 0"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"def add_db_command_parsers(subparsers, config):"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_a4aa33cf","line":102,"range":{"start_line":100,"start_character":0,"end_line":102,"end_character":29},"updated":"2019-01-16 16:25:21.000000000","message":"Can we fix this TODO?","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"161052ef2fa71bde91bee8deeea3777a456b652e","unresolved":false,"context_lines":[{"line_number":97,"context_line":"                    \"details.\"))"},{"line_number":98,"context_line":"            return 2"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"        # TODO(mriedem): Potentially add another return code for"},{"line_number":101,"context_line":"        # \"there are more migrations, but not completable right now\""},{"line_number":102,"context_line":"        return ran and 1 or 0"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"def add_db_command_parsers(subparsers, config):"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_b317c951","line":102,"range":{"start_line":100,"start_character":0,"end_line":102,"end_character":29},"in_reply_to":"bfdaf3ff_a4aa33cf","updated":"2019-01-17 12:44:54.000000000","message":"Looks this is done in https://review.openstack.org/#/c/574268.\nI think I should look into this separately in nova at first.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"47420a743a0037f59584f4d586f2cae94453caeb","unresolved":false,"context_lines":[{"line_number":97,"context_line":"                    \"details.\"))"},{"line_number":98,"context_line":"            return 2"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"        # TODO(mriedem): Potentially add another return code for"},{"line_number":101,"context_line":"        # \"there are more migrations, but not completable right now\""},{"line_number":102,"context_line":"        return ran and 1 or 0"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"def add_db_command_parsers(subparsers, config):"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_5502f60c","line":102,"range":{"start_line":100,"start_character":0,"end_line":102,"end_character":29},"in_reply_to":"bfdaf3ff_b317c951","updated":"2019-01-17 18:57:41.000000000","message":"Some more recent context on that here:\n\nhttps://review.openstack.org/#/c/608091/5/nova/cmd/manage.py@756","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"}],"placement/context.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"84fb227bdc9daf9137d168435918964a08138707","unresolved":false,"context_lines":[{"line_number":21,"context_line":"class RequestContext(context.RequestContext):"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":24,"context_line":"        self.config \u003d kwargs.pop(\u0027config\u0027, None)"},{"line_number":25,"context_line":"        super(RequestContext, self).__init__(*args, **kwargs)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    def can(self, action, target\u003dNone, fatal\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":7,"id":"9fdfeff1_6f51f10e","line":24,"updated":"2019-01-18 17:14:58.000000000","message":"Yeah, that\u0027s reasonable.","commit_id":"80fa50187add026578ea204578289ea077ebad32"}],"placement/db/sqlalchemy/migration.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"47420a743a0037f59584f4d586f2cae94453caeb","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"online_migrations \u003d ("},{"line_number":33,"context_line":"    # These functions are called with a DB context and a count, which is the"},{"line_number":34,"context_line":"    # maximum batch size requested by the user. They must be idempotent."},{"line_number":35,"context_line":"    # At most $count records should be migrated. The function must return a"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_c7c95dac","line":32,"range":{"start_line":32,"start_character":0,"end_line":32,"end_character":17},"updated":"2019-01-17 18:57:41.000000000","message":"Why are these added in db/sqlalchemy/migration rather than the cmd module where the actual CLI is, like in nova? It\u0027s weird IMO to have these split up like this since the code here and below only has context for the command, and should thus belong with the command code and really doesn\u0027t have anything directly to do with the database code, especially since the migrations for the most part are just calling object code. I guess you did it this way because of precedent with the stamp and upgrade methods.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"9701dec828cf8c76954779e96bb0588b601683df","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"online_migrations \u003d ("},{"line_number":33,"context_line":"    # These functions are called with a DB context and a count, which is the"},{"line_number":34,"context_line":"    # maximum batch size requested by the user. They must be idempotent."},{"line_number":35,"context_line":"    # At most $count records should be migrated. The function must return a"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_f2da1649","line":32,"range":{"start_line":32,"start_character":0,"end_line":32,"end_character":17},"in_reply_to":"bfdaf3ff_c7c95dac","updated":"2019-01-18 07:53:41.000000000","message":"Right, I just followed the way of the other db commands, but yes afterthoughts, there is no need to go to each migration methods via the db/sqlalchemy/migration.py. I\u0027ve squashed it in the cmd module.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"da01dd278ce9f93f33e946a8693568c0a2504763","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"online_migrations \u003d ("},{"line_number":33,"context_line":"    # These functions are called with a DB context and a count, which is the"},{"line_number":34,"context_line":"    # maximum batch size requested by the user. They must be idempotent."},{"line_number":35,"context_line":"    # At most $count records should be migrated. The function must return a"},{"line_number":36,"context_line":"    # tuple of (found, done). The found value indicates how many"},{"line_number":37,"context_line":"    # unmigrated/candidate records existed in the database prior to the"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_645f6b0e","line":34,"updated":"2019-01-16 16:25:21.000000000","message":"There is one more assumption about these functions. They should not depend on each other. I.e. there cannot be two methods where the second one can only be run on rows that was already migrated by the first one.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"161052ef2fa71bde91bee8deeea3777a456b652e","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"online_migrations \u003d ("},{"line_number":33,"context_line":"    # These functions are called with a DB context and a count, which is the"},{"line_number":34,"context_line":"    # maximum batch size requested by the user. They must be idempotent."},{"line_number":35,"context_line":"    # At most $count records should be migrated. The function must return a"},{"line_number":36,"context_line":"    # tuple of (found, done). The found value indicates how many"},{"line_number":37,"context_line":"    # unmigrated/candidate records existed in the database prior to the"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_9635d348","line":34,"in_reply_to":"bfdaf3ff_645f6b0e","updated":"2019-01-17 12:44:54.000000000","message":"I\u0027m not sure about that. Even if those functions have dependency it works well catching exception at L112 or returning 0 for done value and moving forward to the other function.... and then comes back after resolving the dependency.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"47420a743a0037f59584f4d586f2cae94453caeb","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"online_migrations \u003d ("},{"line_number":33,"context_line":"    # These functions are called with a DB context and a count, which is the"},{"line_number":34,"context_line":"    # maximum batch size requested by the user. They must be idempotent."},{"line_number":35,"context_line":"    # At most $count records should be migrated. The function must return a"},{"line_number":36,"context_line":"    # tuple of (found, done). The found value indicates how many"},{"line_number":37,"context_line":"    # unmigrated/candidate records existed in the database prior to the"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_e724a1b1","line":34,"in_reply_to":"bfdaf3ff_9635d348","updated":"2019-01-17 18:57:41.000000000","message":"This is all copied from nova:\n\nhttps://github.com/openstack/nova/blob/master/nova/cmd/manage.py#L377\n\nSo same comment about fixing there and copying here, or making changes in a follow up to be clear about the difference.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"da01dd278ce9f93f33e946a8693568c0a2504763","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    # At most $count records should be migrated. The function must return a"},{"line_number":36,"context_line":"    # tuple of (found, done). The found value indicates how many"},{"line_number":37,"context_line":"    # unmigrated/candidate records existed in the database prior to the"},{"line_number":38,"context_line":"    # migration (either total, or up to the $count limit provided), and a"},{"line_number":39,"context_line":"    # nonzero found value may tell the user that there is still work to do."},{"line_number":40,"context_line":"    # The done value indicates whether or not any records were actually"},{"line_number":41,"context_line":"    # migrated by the function. Thus if both (found, done) are nonzero, work"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_c122c9a4","line":38,"range":{"start_line":38,"start_character":16,"end_line":38,"end_character":66},"updated":"2019-01-16 16:25:21.000000000","message":"I don\u0027t full get this. So if the user used --max-count 10 then the returned found value will not be bigger than 10 even if there are more than 10 records that needs to be migrated? Why we need this behavior?","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"47420a743a0037f59584f4d586f2cae94453caeb","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    # At most $count records should be migrated. The function must return a"},{"line_number":36,"context_line":"    # tuple of (found, done). The found value indicates how many"},{"line_number":37,"context_line":"    # unmigrated/candidate records existed in the database prior to the"},{"line_number":38,"context_line":"    # migration (either total, or up to the $count limit provided), and a"},{"line_number":39,"context_line":"    # nonzero found value may tell the user that there is still work to do."},{"line_number":40,"context_line":"    # The done value indicates whether or not any records were actually"},{"line_number":41,"context_line":"    # migrated by the function. Thus if both (found, done) are nonzero, work"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_e7590135","line":38,"range":{"start_line":38,"start_character":16,"end_line":38,"end_character":66},"in_reply_to":"bfdaf3ff_91415d0e","updated":"2019-01-17 18:57:41.000000000","message":"Right - and again, this has all gone through review before when it was added to nova, so re-reviewing it all here is tedious. If we wanted to make changes, I would suggest doing those in a follow up.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"161052ef2fa71bde91bee8deeea3777a456b652e","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    # At most $count records should be migrated. The function must return a"},{"line_number":36,"context_line":"    # tuple of (found, done). The found value indicates how many"},{"line_number":37,"context_line":"    # unmigrated/candidate records existed in the database prior to the"},{"line_number":38,"context_line":"    # migration (either total, or up to the $count limit provided), and a"},{"line_number":39,"context_line":"    # nonzero found value may tell the user that there is still work to do."},{"line_number":40,"context_line":"    # The done value indicates whether or not any records were actually"},{"line_number":41,"context_line":"    # migrated by the function. Thus if both (found, done) are nonzero, work"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_91415d0e","line":38,"range":{"start_line":38,"start_character":16,"end_line":38,"end_character":66},"in_reply_to":"bfdaf3ff_c122c9a4","updated":"2019-01-17 12:44:54.000000000","message":"I think that\u0027s an optimization for a huge cloud. We don\u0027t want to always find *all* the possible records to update everytime we call the function.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"da01dd278ce9f93f33e946a8693568c0a2504763","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    # The done value indicates whether or not any records were actually"},{"line_number":41,"context_line":"    # migrated by the function. Thus if both (found, done) are nonzero, work"},{"line_number":42,"context_line":"    # was done and some work remains. If found is nonzero and done is zero,"},{"line_number":43,"context_line":"    # some records are not migratable (or don\u0027t need migrating), but all"},{"line_number":44,"context_line":"    # migrations that can complete have finished."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    # Added in Stein"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_014e916e","line":43,"range":{"start_line":43,"start_character":38,"end_line":43,"end_character":62},"updated":"2019-01-16 16:25:21.000000000","message":"If they don\u0027t need to be migrated then why they are in the found value?","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"161052ef2fa71bde91bee8deeea3777a456b652e","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    # The done value indicates whether or not any records were actually"},{"line_number":41,"context_line":"    # migrated by the function. Thus if both (found, done) are nonzero, work"},{"line_number":42,"context_line":"    # was done and some work remains. If found is nonzero and done is zero,"},{"line_number":43,"context_line":"    # some records are not migratable (or don\u0027t need migrating), but all"},{"line_number":44,"context_line":"    # migrations that can complete have finished."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    # Added in Stein"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_3172f112","line":43,"range":{"start_line":43,"start_character":38,"end_line":43,"end_character":62},"in_reply_to":"bfdaf3ff_014e916e","updated":"2019-01-17 12:44:54.000000000","message":"That\u0027s got in in https://review.openstack.org/#/c/605836/, but I agree that no-need-for-migration records shouldn\u0027t be in the found value, so I\u0027ve removed this.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"47420a743a0037f59584f4d586f2cae94453caeb","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    # The done value indicates whether or not any records were actually"},{"line_number":41,"context_line":"    # migrated by the function. Thus if both (found, done) are nonzero, work"},{"line_number":42,"context_line":"    # was done and some work remains. If found is nonzero and done is zero,"},{"line_number":43,"context_line":"    # some records are not migratable (or don\u0027t need migrating), but all"},{"line_number":44,"context_line":"    # migrations that can complete have finished."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    # Added in Stein"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_c7a2bdfc","line":43,"range":{"start_line":43,"start_character":38,"end_line":43,"end_character":62},"in_reply_to":"bfdaf3ff_3172f112","updated":"2019-01-17 18:57:41.000000000","message":"For context, the case is something like the nova migrate_instances_add_request_spec migration in rocky. That migration is hitting multiple databases so we don\u0027t know up front how many records need to be migrated from the initial db query. What that had to do was query the cell database to find instances, and then for each instance, query the API DB to see if a request spec existed, and if not, create one. So we could have 100 instances in the cell DB and 10 of those need a request spec, so in that case, total found would be 100 but total migrated would be 10, because the other 90 \"don\u0027t need migrating\".\n\nThere is not a case like that in placement yet, but that is the context for you.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"da01dd278ce9f93f33e946a8693568c0a2504763","unresolved":false,"context_lines":[{"line_number":100,"context_line":"    alembic.command.stamp(config, version)"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"def run_online_migration(max_count):"},{"line_number":104,"context_line":"    ctxt \u003d context.RequestContext()"},{"line_number":105,"context_line":"    ran \u003d 0"},{"line_number":106,"context_line":"    exceptions \u003d False"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_611d1dbc","line":103,"range":{"start_line":103,"start_character":25,"end_line":103,"end_character":34},"updated":"2019-01-16 16:25:21.000000000","message":"Do I understand correctly that max_count is the maximum number of db migration that this function does by execution one ore more migration_meth() methods? \nSo it is possible that run_online_migration called with max_count\u003d50 and the first method migrated 25 rows, then the second method migrated 10 rows, then the 3rd method migrated 15 and then the 4th method is not called as we already spent our 50 migration budget.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"161052ef2fa71bde91bee8deeea3777a456b652e","unresolved":false,"context_lines":[{"line_number":100,"context_line":"    alembic.command.stamp(config, version)"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"def run_online_migration(max_count):"},{"line_number":104,"context_line":"    ctxt \u003d context.RequestContext()"},{"line_number":105,"context_line":"    ran \u003d 0"},{"line_number":106,"context_line":"    exceptions \u003d False"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_b687b70f","line":103,"range":{"start_line":103,"start_character":25,"end_line":103,"end_character":34},"in_reply_to":"bfdaf3ff_611d1dbc","updated":"2019-01-17 12:44:54.000000000","message":"Right.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"da01dd278ce9f93f33e946a8693568c0a2504763","unresolved":false,"context_lines":[{"line_number":112,"context_line":"        except Exception:"},{"line_number":113,"context_line":"            msg \u003d (_(\"Error attempting to run %(method)s\") % dict("},{"line_number":114,"context_line":"                method\u003dmigration_meth))"},{"line_number":115,"context_line":"            print(msg)"},{"line_number":116,"context_line":"            LOG.exception(msg)"},{"line_number":117,"context_line":"            exceptions \u003d True"},{"line_number":118,"context_line":"            found \u003d done \u003d 0"},{"line_number":119,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_e1126d09","line":116,"range":{"start_line":115,"start_character":0,"end_line":116,"end_character":30},"updated":"2019-01-16 16:25:21.000000000","message":"Why do we mix print() and logging?","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"47420a743a0037f59584f4d586f2cae94453caeb","unresolved":false,"context_lines":[{"line_number":112,"context_line":"        except Exception:"},{"line_number":113,"context_line":"            msg \u003d (_(\"Error attempting to run %(method)s\") % dict("},{"line_number":114,"context_line":"                method\u003dmigration_meth))"},{"line_number":115,"context_line":"            print(msg)"},{"line_number":116,"context_line":"            LOG.exception(msg)"},{"line_number":117,"context_line":"            exceptions \u003d True"},{"line_number":118,"context_line":"            found \u003d done \u003d 0"},{"line_number":119,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_c7f07d00","line":116,"range":{"start_line":115,"start_character":0,"end_line":116,"end_character":30},"in_reply_to":"bfdaf3ff_966dd305","updated":"2019-01-17 18:57:41.000000000","message":"It was originally just print in nova because it\u0027s a CLI (another reason why having the actual migration runner code in the db tree is weird for me) but Ib684091af0b19e62396f6becc78c656c49a60504 recently added the logging for the exception traceback. So the logging was intentional.\n\nAgain ( :) ) - this is existing code that\u0027s undergone review before, so it\u0027s tedious to be re-reviewing this all over again.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"161052ef2fa71bde91bee8deeea3777a456b652e","unresolved":false,"context_lines":[{"line_number":112,"context_line":"        except Exception:"},{"line_number":113,"context_line":"            msg \u003d (_(\"Error attempting to run %(method)s\") % dict("},{"line_number":114,"context_line":"                method\u003dmigration_meth))"},{"line_number":115,"context_line":"            print(msg)"},{"line_number":116,"context_line":"            LOG.exception(msg)"},{"line_number":117,"context_line":"            exceptions \u003d True"},{"line_number":118,"context_line":"            found \u003d done \u003d 0"},{"line_number":119,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_966dd305","line":116,"range":{"start_line":115,"start_character":0,"end_line":116,"end_character":30},"in_reply_to":"bfdaf3ff_e1126d09","updated":"2019-01-17 12:44:54.000000000","message":"removed the logging part.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"da01dd278ce9f93f33e946a8693568c0a2504763","unresolved":false,"context_lines":[{"line_number":118,"context_line":"            found \u003d done \u003d 0"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"        name \u003d migration_meth.__name__"},{"line_number":121,"context_line":"        if found:"},{"line_number":122,"context_line":"            print(_(\u0027%(total)i rows matched query %(meth)s, %(done)i \u0027"},{"line_number":123,"context_line":"                    \u0027migrated\u0027) % {\u0027total\u0027: found,"},{"line_number":124,"context_line":"                                   \u0027meth\u0027: name,"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_6150bdd5","line":121,"updated":"2019-01-16 16:25:21.000000000","message":"Note for the future reader: this if is here to avoid printing  \"0 rows matched\"... messages when a done migration method called again and again as other migration methods still needs to progress.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"da01dd278ce9f93f33e946a8693568c0a2504763","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                                   \u0027meth\u0027: name,"},{"line_number":125,"context_line":"                                   \u0027done\u0027: done})"},{"line_number":126,"context_line":"        # This is the per-migration method result for this batch, and"},{"line_number":127,"context_line":"        # _run_migration will either continue on to the next migration,"},{"line_number":128,"context_line":"        # or stop if up to this point we\u0027ve processed max_count of"},{"line_number":129,"context_line":"        # records across all migration methods."},{"line_number":130,"context_line":"        migrations[name] \u003d found, done"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_219f15b5","line":127,"range":{"start_line":127,"start_character":10,"end_line":127,"end_character":24},"updated":"2019-01-16 16:25:21.000000000","message":"Do you mean _run_online_migration?","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"47420a743a0037f59584f4d586f2cae94453caeb","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                                   \u0027meth\u0027: name,"},{"line_number":125,"context_line":"                                   \u0027done\u0027: done})"},{"line_number":126,"context_line":"        # This is the per-migration method result for this batch, and"},{"line_number":127,"context_line":"        # _run_migration will either continue on to the next migration,"},{"line_number":128,"context_line":"        # or stop if up to this point we\u0027ve processed max_count of"},{"line_number":129,"context_line":"        # records across all migration methods."},{"line_number":130,"context_line":"        migrations[name] \u003d found, done"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_47e48d3c","line":127,"range":{"start_line":127,"start_character":10,"end_line":127,"end_character":24},"in_reply_to":"bfdaf3ff_219f15b5","updated":"2019-01-17 18:57:41.000000000","message":"\"run_online_migration\" yes, the method in nova is _run_migration:\n\nhttps://github.com/openstack/nova/blob/master/nova/cmd/manage.py#L671","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"da01dd278ce9f93f33e946a8693568c0a2504763","unresolved":false,"context_lines":[{"line_number":128,"context_line":"        # or stop if up to this point we\u0027ve processed max_count of"},{"line_number":129,"context_line":"        # records across all migration methods."},{"line_number":130,"context_line":"        migrations[name] \u003d found, done"},{"line_number":131,"context_line":"        if max_count is not None:"},{"line_number":132,"context_line":"            ran +\u003d done"},{"line_number":133,"context_line":"            if ran \u003e\u003d max_count:"},{"line_number":134,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_01b9b1ca","line":131,"range":{"start_line":131,"start_character":8,"end_line":131,"end_character":33},"updated":"2019-01-16 16:25:21.000000000","message":"I think the caller always set max_count to 50 or the user specified value","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"da01dd278ce9f93f33e946a8693568c0a2504763","unresolved":false,"context_lines":[{"line_number":132,"context_line":"            ran +\u003d done"},{"line_number":133,"context_line":"            if ran \u003e\u003d max_count:"},{"line_number":134,"context_line":"                break"},{"line_number":135,"context_line":"    return migrations, exceptions"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_41e579a7","line":135,"updated":"2019-01-16 16:25:21.000000000","message":"I feel that this code would be easier to understand if the (some of the ) core of the for loop would be split out to a separate function.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"aef8e66b3bde0b71d6f71a04e5af7ee622ac0e27","unresolved":false,"context_lines":[{"line_number":113,"context_line":"        except Exception:"},{"line_number":114,"context_line":"            msg \u003d (_(\"Error attempting to run %(method)s\") % dict("},{"line_number":115,"context_line":"                method\u003dmigration_meth))"},{"line_number":116,"context_line":"            print(msg)"},{"line_number":117,"context_line":"            exceptions \u003d True"},{"line_number":118,"context_line":"            found \u003d done \u003d 0"},{"line_number":119,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"bfdaf3ff_d63ee989","line":116,"range":{"start_line":116,"start_character":12,"end_line":116,"end_character":22},"updated":"2019-01-17 22:58:16.000000000","message":"Remember how I mentioned that we explicitly added (in nova) a LOG.exception of the traceback here - that\u0027s because the error message is useless without it, like I\u0027m seeing in the grenade change that runs on this series:\n\nhttp://logs.openstack.org/14/631614/1/check/neutron-grenade/dbae07d/logs/grenade.sh.txt.gz#_2019-01-17_21_58_14_513","commit_id":"04055a64dae001361cc3790de05c600b5a2b406d"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"9701dec828cf8c76954779e96bb0588b601683df","unresolved":false,"context_lines":[{"line_number":113,"context_line":"        except Exception:"},{"line_number":114,"context_line":"            msg \u003d (_(\"Error attempting to run %(method)s\") % dict("},{"line_number":115,"context_line":"                method\u003dmigration_meth))"},{"line_number":116,"context_line":"            print(msg)"},{"line_number":117,"context_line":"            exceptions \u003d True"},{"line_number":118,"context_line":"            found \u003d done \u003d 0"},{"line_number":119,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"bfdaf3ff_d2cef280","line":116,"range":{"start_line":116,"start_character":12,"end_line":116,"end_character":22},"in_reply_to":"bfdaf3ff_d63ee989","updated":"2019-01-18 07:53:41.000000000","message":"I\u0027ve moved it back. Good to know. (Sorry for removing not looking into it enough).\nThanks!","commit_id":"04055a64dae001361cc3790de05c600b5a2b406d"}],"placement/tests/unit/cmd/test_manage.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"a98a71c682c5d36bdd35d54319f579f3b7e76166","unresolved":false,"context_lines":[{"line_number":125,"context_line":"        conf_fixture.register_cli_opts(command_opts)"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        self.useFixture("},{"line_number":128,"context_line":"            fixtures.MonkeyPatch(\u0027sys.stdout\u0027, six.moves.StringIO()))"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    def test_online_migrations(self):"},{"line_number":131,"context_line":"        # Mock two online migrations"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_13d52319","line":128,"updated":"2018-12-21 10:00:34.000000000","message":"I think you can probably use the same fixture used at line 37 here.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"da01dd278ce9f93f33e946a8693568c0a2504763","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"bfdaf3ff_c4cc17be","line":157,"updated":"2019-01-16 16:25:21.000000000","message":"We need more test cases to cover the implementation of --max-count and the handling of exception.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"10677b9359e2085a21877dee22f623b294cc0206","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"bfdaf3ff_e22b397f","line":157,"in_reply_to":"bfdaf3ff_c4cc17be","updated":"2019-01-17 12:47:37.000000000","message":"I\u0027m going to do this tomorrow if possible.","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"47420a743a0037f59584f4d586f2cae94453caeb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"bfdaf3ff_276709c0","line":157,"in_reply_to":"bfdaf3ff_e22b397f","updated":"2019-01-17 18:57:41.000000000","message":"Just copy the nova tests:\n\nhttps://github.com/openstack/nova/blob/master/nova/tests/unit/test_nova_manage.py#L748-L859","commit_id":"f2a5a6ec248f5b3e23dfeb029c0385beb6a1e677"}]}