)]}'
{"nova/virt/libvirt/driver.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"84ce893396e5b75493f2c55ea93d120c142b6c13","unresolved":false,"context_lines":[{"line_number":6489,"context_line":""},{"line_number":6490,"context_line":"        finish_event \u003d eventlet.event.Event()"},{"line_number":6491,"context_line":"        # The active migrations running on this compute. The dict is"},{"line_number":6492,"context_line":"        # returning a tuple composed of: - A queue to trigge tasks for"},{"line_number":6493,"context_line":"        # the migration process of an instance. - A boolean value to"},{"line_number":6494,"context_line":"        # indicate whether the migration finished with success or not,"},{"line_number":6495,"context_line":"        # None indicates that the process is still running or not"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba5201f7_cb890d28","line":6492,"range":{"start_line":6492,"start_character":41,"end_line":6492,"end_character":70},"updated":"2017-01-05 15:06:58.000000000","message":"this looks like it\u0027s supposed to be on a new line","commit_id":"1fe4a5320cf007c9c7237495bca5b7bc9084aa1f"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"84ce893396e5b75493f2c55ea93d120c142b6c13","unresolved":false,"context_lines":[{"line_number":6489,"context_line":""},{"line_number":6490,"context_line":"        finish_event \u003d eventlet.event.Event()"},{"line_number":6491,"context_line":"        # The active migrations running on this compute. The dict is"},{"line_number":6492,"context_line":"        # returning a tuple composed of: - A queue to trigge tasks for"},{"line_number":6493,"context_line":"        # the migration process of an instance. - A boolean value to"},{"line_number":6494,"context_line":"        # indicate whether the migration finished with success or not,"},{"line_number":6495,"context_line":"        # None indicates that the process is still running or not"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba5201f7_0ba95590","line":6492,"range":{"start_line":6492,"start_character":54,"end_line":6492,"end_character":60},"updated":"2017-01-05 15:06:58.000000000","message":"trigger","commit_id":"1fe4a5320cf007c9c7237495bca5b7bc9084aa1f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d8229e3199ac33703ee41eae992d72091952419e","unresolved":false,"context_lines":[{"line_number":6494,"context_line":"        # indicate whether the migration finished with success or not,"},{"line_number":6495,"context_line":"        # None indicates that the process is still running or not"},{"line_number":6496,"context_line":"        # started yet."},{"line_number":6497,"context_line":"        self.active_migrations[instance.uuid] \u003d (deque(), None)"},{"line_number":6498,"context_line":""},{"line_number":6499,"context_line":"        def thread_finished(thread, event):"},{"line_number":6500,"context_line":"            LOG.debug(\"Migration operation thread notification\","}],"source_content_type":"text/x-python","patch_set":5,"id":"ba5201f7_562860e2","line":6497,"range":{"start_line":6497,"start_character":58,"end_line":6497,"end_character":62},"updated":"2017-01-05 15:11:43.000000000","message":"Where does this actually get set?\n\nAlso, could we use a namedtuple or something for this? I don\u0027t really like mapping to a tuple and then having to be extra careful about indexes and such when using it, that\u0027s just going to bite us at some point with a bug because someone didn\u0027t know what was in the thing, or in what order.","commit_id":"1fe4a5320cf007c9c7237495bca5b7bc9084aa1f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"da696f7d57b69750c93071323788973905d2e502","unresolved":false,"context_lines":[{"line_number":6494,"context_line":"        # indicate whether the migration finished with success or not,"},{"line_number":6495,"context_line":"        # None indicates that the process is still running or not"},{"line_number":6496,"context_line":"        # started yet."},{"line_number":6497,"context_line":"        self.active_migrations[instance.uuid] \u003d (deque(), None)"},{"line_number":6498,"context_line":""},{"line_number":6499,"context_line":"        def thread_finished(thread, event):"},{"line_number":6500,"context_line":"            LOG.debug(\"Migration operation thread notification\","}],"source_content_type":"text/x-python","patch_set":5,"id":"ba5201f7_d661b03a","line":6497,"range":{"start_line":6497,"start_character":58,"end_line":6497,"end_character":62},"in_reply_to":"ba5201f7_562860e2","updated":"2017-01-05 15:12:47.000000000","message":"I guess it\u0027s set in the next patch:\n\nhttps://review.openstack.org/#/c/409261/\n\nWould still prefer to use a namedtuple for clarity.","commit_id":"1fe4a5320cf007c9c7237495bca5b7bc9084aa1f"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"64b0d16d5a7c43f7b8a483574fbe71120e5032af","unresolved":false,"context_lines":[{"line_number":6494,"context_line":"        # indicate whether the migration finished with success or not,"},{"line_number":6495,"context_line":"        # None indicates that the process is still running or not"},{"line_number":6496,"context_line":"        # started yet."},{"line_number":6497,"context_line":"        self.active_migrations[instance.uuid] \u003d (deque(), None)"},{"line_number":6498,"context_line":""},{"line_number":6499,"context_line":"        def thread_finished(thread, event):"},{"line_number":6500,"context_line":"            LOG.debug(\"Migration operation thread notification\","}],"source_content_type":"text/x-python","patch_set":5,"id":"ba5201f7_b6e0ac97","line":6497,"range":{"start_line":6497,"start_character":58,"end_line":6497,"end_character":62},"in_reply_to":"ba5201f7_562860e2","updated":"2017-01-05 15:14:37.000000000","message":"That caught me too - the actual setting of it is in the follow up patch. Agree on the namedtuple though","commit_id":"1fe4a5320cf007c9c7237495bca5b7bc9084aa1f"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"8cb8fb1b6662b0ab159adff3271ca3e719c6ae6c","unresolved":false,"context_lines":[{"line_number":6505,"context_line":"        # indicate whether the migration finished with success or not,"},{"line_number":6506,"context_line":"        # None indicates that the process is still running or not"},{"line_number":6507,"context_line":"        # started yet."},{"line_number":6508,"context_line":"        self.active_migrations[instance.uuid] \u003d ActiveMigrations(deque(), None)"},{"line_number":6509,"context_line":""},{"line_number":6510,"context_line":"        def thread_finished(thread, event):"},{"line_number":6511,"context_line":"            LOG.debug(\"Migration operation thread notification\","}],"source_content_type":"text/x-python","patch_set":11,"id":"7a3c09a3_216d11cc","line":6508,"range":{"start_line":6508,"start_character":74,"end_line":6508,"end_character":78},"updated":"2017-01-18 12:43:40.000000000","message":"tbh, I\u0027d prefer enum here, rather than boolean, it will be much easier to extend it in future.","commit_id":"a9baf1c5ec0e565baeb591d87806de91afb004fc"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"7eed9fbbe0dd2675106751d88d18b9877437c11e","unresolved":false,"context_lines":[{"line_number":6499,"context_line":"        # the migration process of an instance. - A boolean value to"},{"line_number":6500,"context_line":"        # indicate whether the migration finished with success or not,"},{"line_number":6501,"context_line":"        # None indicates that the process is still running or not"},{"line_number":6502,"context_line":"        # started yet."},{"line_number":6503,"context_line":"        self.active_migrations[instance.uuid] \u003d ActiveMigrations(deque(), None)"},{"line_number":6504,"context_line":""},{"line_number":6505,"context_line":"        def thread_finished(thread, event):"}],"source_content_type":"text/x-python","patch_set":12,"id":"1a430d35_343b1fd3","line":6502,"updated":"2017-02-06 11:53:11.000000000","message":"This combination of True with two different forms Falsey to indicate three different states strikes me as particularly ripe for errors. An enum of constant values define somewhere might be more robust? I guess it largely depends on how it is going to be used and what precedents have been set elsewhere. But just looking at this standalone, without prior context, it\u0027s not great.","commit_id":"1f737693d3ea5478ac673469a06f58a61928b3c5"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"274e5fef8ac65bcf52ab694bcd23fca7d785b3e3","unresolved":false,"context_lines":[{"line_number":6499,"context_line":"        # the migration process of an instance. - A boolean value to"},{"line_number":6500,"context_line":"        # indicate whether the migration finished with success or not,"},{"line_number":6501,"context_line":"        # None indicates that the process is still running or not"},{"line_number":6502,"context_line":"        # started yet."},{"line_number":6503,"context_line":"        self.active_migrations[instance.uuid] \u003d ActiveMigrations(deque(), None)"},{"line_number":6504,"context_line":""},{"line_number":6505,"context_line":"        def thread_finished(thread, event):"}],"source_content_type":"text/x-python","patch_set":12,"id":"1a430d35_90e93183","line":6502,"in_reply_to":"1a430d35_343b1fd3","updated":"2017-02-06 16:34:51.000000000","message":"It does not make sense to use an enum here, the state can only be True/False. None just means that it\u0027s not yet initialized. The state is never evaluated as None. I fixed the comment to avoid any confusion.","commit_id":"1f737693d3ea5478ac673469a06f58a61928b3c5"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"1f5a9cf935a81ce3b87feb5fc3df2f724746aa48","unresolved":false,"context_lines":[{"line_number":6498,"context_line":"        # returning a tuple composed of: - A queue to trigger tasks for"},{"line_number":6499,"context_line":"        # the migration process of an instance. - A boolean value to"},{"line_number":6500,"context_line":"        # indicate whether the migration finished with success or not."},{"line_number":6501,"context_line":"        self.active_migrations[instance.uuid] \u003d ActiveMigrations(deque(), None)"},{"line_number":6502,"context_line":""},{"line_number":6503,"context_line":"        def thread_finished(thread, event):"},{"line_number":6504,"context_line":"            LOG.debug(\"Migration operation thread notification\","}],"source_content_type":"text/x-python","patch_set":13,"id":"1a430d35_53b7d3d7","line":6501,"updated":"2017-02-06 16:44:01.000000000","message":"Okay, the comment is better now, but if that\u0027s the case then why not initialize to False instead of None. We\u0027ve still got a situation there you\u0027re claiming a typed interface, but then using something that\u0027s not that type to initialize it. Is there a reason for that?","commit_id":"836c67ed49619705af0ed88e2e05040914c656a8"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"85fc27967d0b2983946a57a150613021e4bdfc17","unresolved":false,"context_lines":[{"line_number":6498,"context_line":"        # returning a tuple composed of: - A queue to trigger tasks for"},{"line_number":6499,"context_line":"        # the migration process of an instance. - A boolean value to"},{"line_number":6500,"context_line":"        # indicate whether the migration finished with success or not."},{"line_number":6501,"context_line":"        self.active_migrations[instance.uuid] \u003d ActiveMigrations(deque(), None)"},{"line_number":6502,"context_line":""},{"line_number":6503,"context_line":"        def thread_finished(thread, event):"},{"line_number":6504,"context_line":"            LOG.debug(\"Migration operation thread notification\","}],"source_content_type":"text/x-python","patch_set":13,"id":"1a430d35_24bae30a","line":6501,"in_reply_to":"1a430d35_53b7d3d7","updated":"2017-02-07 09:12:27.000000000","message":"We need to specifically set a value to initialize the named tuple, that could be whatever you want. As indicated in the comment \u0027False\u0027 is representing a state of the migration and same for True, so IMHO \u0027None\u0027 is the right choice.","commit_id":"836c67ed49619705af0ed88e2e05040914c656a8"}],"nova/virt/libvirt/migration.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"7eed9fbbe0dd2675106751d88d18b9877437c11e","unresolved":false,"context_lines":[{"line_number":431,"context_line":"    instance object. The active migrations dict should use instance"},{"line_number":432,"context_line":"    UUIDs for keys and a tuple composed of a queue of tasks as the"},{"line_number":433,"context_line":"    values and a Boolean to indicate whether or not the migration"},{"line_number":434,"context_line":"    succeed."},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"    Currently the valid tasks that can be requested"},{"line_number":437,"context_line":"    are \"pause\" and \"force-complete\". Other tasks will"}],"source_content_type":"text/x-python","patch_set":12,"id":"1a430d35_b4746f9c","line":434,"updated":"2017-02-06 11:53:11.000000000","message":"This comment make the concern in my previous comment even stronger. Now we have a situation where we\u0027re declaim a boolean, but really it is not.","commit_id":"1f737693d3ea5478ac673469a06f58a61928b3c5"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"7eed9fbbe0dd2675106751d88d18b9877437c11e","unresolved":false,"context_lines":[{"line_number":436,"context_line":"    Currently the valid tasks that can be requested"},{"line_number":437,"context_line":"    are \"pause\" and \"force-complete\". Other tasks will"},{"line_number":438,"context_line":"    be ignored."},{"line_number":439,"context_line":""},{"line_number":440,"context_line":"    \"\"\""},{"line_number":441,"context_line":""},{"line_number":442,"context_line":"    tasks \u003d deque()"}],"source_content_type":"text/x-python","patch_set":12,"id":"1a430d35_b49b8fc8","line":439,"updated":"2017-02-06 11:53:11.000000000","message":"Whitespace not require nor desired.","commit_id":"1f737693d3ea5478ac673469a06f58a61928b3c5"}]}