)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"9c7dcbb252ce602cb788d325fefc6b717882d99d","unresolved":false,"context_lines":[{"line_number":10,"context_line":"parallelism and performance improvements, that we\u0027re"},{"line_number":11,"context_line":"going to have to do something about locking. Seems the"},{"line_number":12,"context_line":"end logical point is to support etcd, but we\u0027ve wired"},{"line_number":13,"context_line":"database locking on nodes to our core."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Adds locking interface, with a single driver module"},{"line_number":16,"context_line":"to be loaded that wraps the calls to the database."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f79a3b5_51a3efb1","line":13,"updated":"2018-09-02 07:53:57.000000000","message":"This is definitely useful as a means of trying to see the future. I would like to see some words on the problems that need to be solved also. If not a spec immediately then perhaps an etherpad? Ideally a set of short bullet points outlining the issues in simple terms, then perhaps some additional words to cover the caveats etc.","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"2e64021dc92aa0e27106bd421b6a6943a29041e7","unresolved":false,"context_lines":[{"line_number":10,"context_line":"parallelism and performance improvements, that we\u0027re"},{"line_number":11,"context_line":"going to have to do something about locking. Seems the"},{"line_number":12,"context_line":"end logical point is to support etcd, but we\u0027ve wired"},{"line_number":13,"context_line":"database locking on nodes to our core."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Adds locking interface, with a single driver module"},{"line_number":16,"context_line":"to be loaded that wraps the calls to the database."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f79a3b5_1207abcb","line":13,"in_reply_to":"3f79a3b5_51a3efb1","updated":"2018-09-03 13:43:17.000000000","message":"++, I\u0027ll put some in the next revision. We\u0027ve got a few different references all over the PTG topics so I suspect it is going to come up a few times at the ptg.\n\nI\u0027m kind of hoping to keep this as utilitarian as possible and avoiding a spec in the grand scheme of the universe.  The idea generally improves things, but we also have some bad locking concepts burried into our code that I\u0027m finding as time goes on.","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"3ba1baf8ff19ff9a0a95e46c1371a8cca220da7d","unresolved":false,"context_lines":[{"line_number":15,"context_line":"It seems clear with many of the discussions about"},{"line_number":16,"context_line":"parallelism and performance improvements that would be"},{"line_number":17,"context_line":"necessary with the edge use cases that have been discussed"},{"line_number":18,"context_line":"in the ironic communitythat we\u0027re going to have to do"},{"line_number":19,"context_line":"something about locking."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Presently we lock all resources associated with a field"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"3f79a3b5_95ca2ca1","line":18,"range":{"start_line":18,"start_character":22,"end_line":18,"end_character":23},"updated":"2018-10-31 08:28:25.000000000","message":"missing a space","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d967b2004f3b2c8300171698292cff2d72a14035","unresolved":false,"context_lines":[{"line_number":15,"context_line":"It seems clear with many of the discussions about"},{"line_number":16,"context_line":"parallelism and performance improvements that would be"},{"line_number":17,"context_line":"necessary with the edge use cases that have been discussed"},{"line_number":18,"context_line":"in the ironic communitythat we\u0027re going to have to do"},{"line_number":19,"context_line":"something about locking."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Presently we lock all resources associated with a field"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"3f79a3b5_36227403","line":18,"range":{"start_line":18,"start_character":22,"end_line":18,"end_character":23},"in_reply_to":"3f79a3b5_95ca2ca1","updated":"2018-11-28 16:24:04.000000000","message":"Done","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"3ba1baf8ff19ff9a0a95e46c1371a8cca220da7d","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":"With that, we can possibly even continue to keep our"},{"line_number":46,"context_line":"current locking model, but levarage more up to date"},{"line_number":47,"context_line":"locking infrmation. Currently we also set ourselves"},{"line_number":48,"context_line":"up for operaiting with out-of-date locking information"},{"line_number":49,"context_line":"by executing a query and then looping upon it expecting"},{"line_number":50,"context_line":"the locking data to be up to date."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"3f79a3b5_f5db6077","line":47,"range":{"start_line":47,"start_character":8,"end_line":47,"end_character":18},"updated":"2018-10-31 08:28:25.000000000","message":"typo","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d967b2004f3b2c8300171698292cff2d72a14035","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":"With that, we can possibly even continue to keep our"},{"line_number":46,"context_line":"current locking model, but levarage more up to date"},{"line_number":47,"context_line":"locking infrmation. Currently we also set ourselves"},{"line_number":48,"context_line":"up for operaiting with out-of-date locking information"},{"line_number":49,"context_line":"by executing a query and then looping upon it expecting"},{"line_number":50,"context_line":"the locking data to be up to date."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"3f79a3b5_967ca8e9","line":47,"range":{"start_line":47,"start_character":8,"end_line":47,"end_character":18},"in_reply_to":"3f79a3b5_f5db6077","updated":"2018-11-28 16:24:04.000000000","message":"Done","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"3ba1baf8ff19ff9a0a95e46c1371a8cca220da7d","unresolved":false,"context_lines":[{"line_number":45,"context_line":"With that, we can possibly even continue to keep our"},{"line_number":46,"context_line":"current locking model, but levarage more up to date"},{"line_number":47,"context_line":"locking infrmation. Currently we also set ourselves"},{"line_number":48,"context_line":"up for operaiting with out-of-date locking information"},{"line_number":49,"context_line":"by executing a query and then looping upon it expecting"},{"line_number":50,"context_line":"the locking data to be up to date."},{"line_number":51,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"3f79a3b5_b5cf28b1","line":48,"range":{"start_line":48,"start_character":7,"end_line":48,"end_character":17},"updated":"2018-10-31 08:28:25.000000000","message":"typo","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d967b2004f3b2c8300171698292cff2d72a14035","unresolved":false,"context_lines":[{"line_number":45,"context_line":"With that, we can possibly even continue to keep our"},{"line_number":46,"context_line":"current locking model, but levarage more up to date"},{"line_number":47,"context_line":"locking infrmation. Currently we also set ourselves"},{"line_number":48,"context_line":"up for operaiting with out-of-date locking information"},{"line_number":49,"context_line":"by executing a query and then looping upon it expecting"},{"line_number":50,"context_line":"the locking data to be up to date."},{"line_number":51,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"3f79a3b5_56629042","line":48,"range":{"start_line":48,"start_character":7,"end_line":48,"end_character":17},"in_reply_to":"3f79a3b5_b5cf28b1","updated":"2018-11-28 16:24:04.000000000","message":"Done","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"3ba1baf8ff19ff9a0a95e46c1371a8cca220da7d","unresolved":false,"context_lines":[{"line_number":64,"context_line":"locked from being picked up by the periodic tasks. Code"},{"line_number":65,"context_line":"to handle lock allocation failures was already present"},{"line_number":66,"context_line":"in every single case."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"Story: #1526751"},{"line_number":69,"context_line":"Task: #10387"},{"line_number":70,"context_line":"Change-Id: I5c02f994552278526c202d04ce1e99b21f63122e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"3f79a3b5_d5de646a","line":67,"updated":"2018-10-31 08:28:25.000000000","message":"I feel like reading a novel \\o/","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"}],"ironic/common/exception.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"36d3b2fab689b38939089f158a7f55ae13b0c1f2","unresolved":false,"context_lines":[{"line_number":836,"context_line":""},{"line_number":837,"context_line":""},{"line_number":838,"context_line":"class LockingLoadError(IronicException):"},{"line_number":839,"context_line":"    _msg_fmt \u003d _(\"Failed to load Locking provider %(locking_provider_name)s, \""},{"line_number":840,"context_line":"                 \"reason: %(reason)s\")"}],"source_content_type":"text/x-python","patch_set":20,"id":"ffb9cba7_91c0702c","line":839,"updated":"2019-04-29 04:04:16.000000000","message":"nit: \"locking\", just %(name)s","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"e5c75bba02f8156a0c3b3a8bed2aa268d5cdca7c","unresolved":false,"context_lines":[{"line_number":836,"context_line":""},{"line_number":837,"context_line":""},{"line_number":838,"context_line":"# This is presently NOT used, but should be available to the first"},{"line_number":839,"context_line":"# non-database lockind driver is supported."},{"line_number":840,"context_line":"class LockingLoadError(IronicException):"},{"line_number":841,"context_line":"    _msg_fmt \u003d _(\"Failed to load locking provider %(name)s, \""},{"line_number":842,"context_line":"                 \"reason: %(reason)s\")"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_cd5446cd","line":839,"updated":"2019-06-06 09:22:25.000000000","message":"nit: locking","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"}],"ironic/common/locking_factory.py":[{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"3ba1baf8ff19ff9a0a95e46c1371a8cca220da7d","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from ironic.common import exception"},{"line_number":17,"context_line":"from ironic.conf import CONF"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"_locking_provider \u003d None"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"EM_SEMAPHORE \u003d \u0027locking_provider\u0027"},{"line_number":22,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_9558ecfc","line":19,"range":{"start_line":19,"start_character":0,"end_line":19,"end_character":24},"updated":"2018-10-31 08:28:25.000000000","message":"it appears that this is not in use","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"365eef31e0a1f1f5967601c5226b3458c7116a04","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    _locking_provider \u003d None"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    def __init__(self, **kwargs):"},{"line_number":30,"context_line":"        if not LockingFactory._locking_provider:"},{"line_number":31,"context_line":"            LockingFactory._set_locking_provider(**kwargs)"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    # NOTE(lucasagomes): Use lockutils to avoid a potential race in eventlet"}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_e91a7415","line":30,"range":{"start_line":30,"start_character":30,"end_line":30,"end_character":47},"updated":"2019-02-07 18:22:14.000000000","message":"nit: given we are dealing with class attribute and method here, would it make sense to move this locking provider boot strapping into `__new__`?","commit_id":"468d436bd3c7e82888b60748d073ea09588908e0"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d8c0d6bc2c72efd851816d11056a389ba063364f","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    _locking_provider \u003d None"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    def __init__(self, **kwargs):"},{"line_number":30,"context_line":"        if not LockingFactory._locking_provider:"},{"line_number":31,"context_line":"            LockingFactory._set_locking_provider(**kwargs)"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    # NOTE(lucasagomes): Use lockutils to avoid a potential race in eventlet"}],"source_content_type":"text/x-python","patch_set":18,"id":"5fc1f717_2887f263","line":30,"range":{"start_line":30,"start_character":30,"end_line":30,"end_character":47},"in_reply_to":"9fdfeff1_e91a7415","updated":"2019-03-22 21:41:40.000000000","message":"I think __new__ would get awkward if we need to do anything locking wise through the interface without invocation.... which I think we already do in this entire patch.\n\nI think __init__ is what we\u0027re doing in other portions of the code and I\u0027d kind of prefer to keep it the same.","commit_id":"468d436bd3c7e82888b60748d073ea09588908e0"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"36d3b2fab689b38939089f158a7f55ae13b0c1f2","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        except Exception as e:"},{"line_number":57,"context_line":"            raise exception.LockingLoadError("},{"line_number":58,"context_line":"                locking_provider_name\u003dlocking_provider_name, reason\u003de"},{"line_number":59,"context_line":"            )"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"        cls._locking_provider \u003d _extension_manager.driver"},{"line_number":62,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"ffb9cba7_71d85c1d","line":59,"updated":"2019-04-29 04:04:16.000000000","message":"Let\u0027s maybe not do this? I\u0027ve been thinking of dropping similar code from the drivers factory, since it drops tracebacks, making some exceptions ver hard to debug.","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"03def507e4ad023edbd7286681ddac1eaec12047","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        except Exception as e:"},{"line_number":57,"context_line":"            raise exception.LockingLoadError("},{"line_number":58,"context_line":"                locking_provider_name\u003dlocking_provider_name, reason\u003de"},{"line_number":59,"context_line":"            )"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"        cls._locking_provider \u003d _extension_manager.driver"},{"line_number":62,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"bfb3d3c7_b3912d9c","line":59,"in_reply_to":"ffb9cba7_71d85c1d","updated":"2019-05-22 18:58:09.000000000","message":"You raise a really good point, and if locking is not working it should be a fairly hard failure and things should go *boom*","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"}],"ironic/conductor/base_manager.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"36d3b2fab689b38939089f158a7f55ae13b0c1f2","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        # clear all target_power_state with locks by this conductor"},{"line_number":148,"context_line":"        self.dbapi.clear_node_target_power_state(self.host)"},{"line_number":149,"context_line":"        # clear all locks held by this conductor before registering"},{"line_number":150,"context_line":"        locking \u003d locking_factory.LockingFactory().provider"},{"line_number":151,"context_line":"        locking.clear_reservations_for_conductor(self.host)"},{"line_number":152,"context_line":"        try:"},{"line_number":153,"context_line":"            # Register this conductor with the cluster"}],"source_content_type":"text/x-python","patch_set":20,"id":"ffb9cba7_91d5d0e7","line":150,"updated":"2019-04-29 04:04:16.000000000","message":"nit: create a helper for this","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"03def507e4ad023edbd7286681ddac1eaec12047","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        # clear all target_power_state with locks by this conductor"},{"line_number":148,"context_line":"        self.dbapi.clear_node_target_power_state(self.host)"},{"line_number":149,"context_line":"        # clear all locks held by this conductor before registering"},{"line_number":150,"context_line":"        locking \u003d locking_factory.LockingFactory().provider"},{"line_number":151,"context_line":"        locking.clear_reservations_for_conductor(self.host)"},{"line_number":152,"context_line":"        try:"},{"line_number":153,"context_line":"            # Register this conductor with the cluster"}],"source_content_type":"text/x-python","patch_set":20,"id":"bfb3d3c7_f3948546","line":150,"in_reply_to":"ffb9cba7_91d5d0e7","updated":"2019-05-22 18:58:09.000000000","message":"a helper do the same thing the line above is doing? I\u0027m not sure I understand where your coming from.","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"36d3b2fab689b38939089f158a7f55ae13b0c1f2","unresolved":false,"context_lines":[{"line_number":233,"context_line":"        the nodes (dbapi.clear_node_reservations_for_conductor) is not enough"},{"line_number":234,"context_line":"        to unstick it, so let\u0027s gracefully fail the process."},{"line_number":235,"context_line":"        \"\"\""},{"line_number":236,"context_line":"        filters \u003d {\u0027provision_state\u0027: state}"},{"line_number":237,"context_line":"        self._fail_if_in_state(ironic_context.get_admin_context(), filters,"},{"line_number":238,"context_line":"                               state, \u0027provision_updated_at\u0027,"},{"line_number":239,"context_line":"                               last_error\u003dlast_error)"}],"source_content_type":"text/x-python","patch_set":20,"id":"ffb9cba7_3108a490","line":236,"updated":"2019-04-29 04:04:16.000000000","message":"/me is pondering self.locking.add_node_filters(filters, locked\u003dFalse)","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"03def507e4ad023edbd7286681ddac1eaec12047","unresolved":false,"context_lines":[{"line_number":233,"context_line":"        the nodes (dbapi.clear_node_reservations_for_conductor) is not enough"},{"line_number":234,"context_line":"        to unstick it, so let\u0027s gracefully fail the process."},{"line_number":235,"context_line":"        \"\"\""},{"line_number":236,"context_line":"        filters \u003d {\u0027provision_state\u0027: state}"},{"line_number":237,"context_line":"        self._fail_if_in_state(ironic_context.get_admin_context(), filters,"},{"line_number":238,"context_line":"                               state, \u0027provision_updated_at\u0027,"},{"line_number":239,"context_line":"                               last_error\u003dlast_error)"}],"source_content_type":"text/x-python","patch_set":20,"id":"bfb3d3c7_73d3b539","line":236,"in_reply_to":"ffb9cba7_3108a490","updated":"2019-05-22 18:58:09.000000000","message":"I\u0027m not sure that makes any sense for locking code to have code about knowledge and interaction with the database.","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"36d3b2fab689b38939089f158a7f55ae13b0c1f2","unresolved":false,"context_lines":[{"line_number":479,"context_line":"        for node_uuid, driver, conductor_group in node_iter:"},{"line_number":480,"context_line":"            try:"},{"line_number":481,"context_line":"                # NOTE(TheJulia): By default, locks are exclusive unless"},{"line_number":482,"context_line":"                # explicitly shared."},{"line_number":483,"context_line":"                with task_manager.acquire(context, node_uuid,"},{"line_number":484,"context_line":"                                          purpose\u003d\u0027node state check\u0027) as task:"},{"line_number":485,"context_line":"                    if (task.node.maintenance"}],"source_content_type":"text/x-python","patch_set":20,"id":"ffb9cba7_510d189f","line":482,"updated":"2019-04-29 04:04:16.000000000","message":"(Thinking aloud), this will cause more locking retries in periodic tasks. I wonder if we need to create a generic way to guess if a node is currently reserved..","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"03def507e4ad023edbd7286681ddac1eaec12047","unresolved":false,"context_lines":[{"line_number":479,"context_line":"        for node_uuid, driver, conductor_group in node_iter:"},{"line_number":480,"context_line":"            try:"},{"line_number":481,"context_line":"                # NOTE(TheJulia): By default, locks are exclusive unless"},{"line_number":482,"context_line":"                # explicitly shared."},{"line_number":483,"context_line":"                with task_manager.acquire(context, node_uuid,"},{"line_number":484,"context_line":"                                          purpose\u003d\u0027node state check\u0027) as task:"},{"line_number":485,"context_line":"                    if (task.node.maintenance"}],"source_content_type":"text/x-python","patch_set":20,"id":"bfb3d3c7_73ddf512","line":482,"in_reply_to":"ffb9cba7_510d189f","updated":"2019-05-22 18:58:09.000000000","message":"It absolutely would if allow ourselves to loosen the process model ;)","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"36d3b2fab689b38939089f158a7f55ae13b0c1f2","unresolved":false,"context_lines":[{"line_number":481,"context_line":"                # NOTE(TheJulia): By default, locks are exclusive unless"},{"line_number":482,"context_line":"                # explicitly shared."},{"line_number":483,"context_line":"                with task_manager.acquire(context, node_uuid,"},{"line_number":484,"context_line":"                                          purpose\u003d\u0027node state check\u0027) as task:"},{"line_number":485,"context_line":"                    if (task.node.maintenance"},{"line_number":486,"context_line":"                            or task.node.provision_state"},{"line_number":487,"context_line":"                            not in provision_state):"}],"source_content_type":"text/x-python","patch_set":20,"id":"ffb9cba7_f1fd2c6e","line":484,"updated":"2019-04-29 04:04:16.000000000","message":".. or let us at least disable retries here (support for that has recently landed).","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"03def507e4ad023edbd7286681ddac1eaec12047","unresolved":false,"context_lines":[{"line_number":481,"context_line":"                # NOTE(TheJulia): By default, locks are exclusive unless"},{"line_number":482,"context_line":"                # explicitly shared."},{"line_number":483,"context_line":"                with task_manager.acquire(context, node_uuid,"},{"line_number":484,"context_line":"                                          purpose\u003d\u0027node state check\u0027) as task:"},{"line_number":485,"context_line":"                    if (task.node.maintenance"},{"line_number":486,"context_line":"                            or task.node.provision_state"},{"line_number":487,"context_line":"                            not in provision_state):"}],"source_content_type":"text/x-python","patch_set":20,"id":"bfb3d3c7_53ea31ac","line":484,"in_reply_to":"ffb9cba7_f1fd2c6e","updated":"2019-05-22 18:58:09.000000000","message":"That is a great idea!","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"e5c75bba02f8156a0c3b3a8bed2aa268d5cdca7c","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        # clear all target_power_state with locks by this conductor"},{"line_number":151,"context_line":"        self.dbapi.clear_node_target_power_state(self.host)"},{"line_number":152,"context_line":"        # clear all locks held by this conductor before registering"},{"line_number":153,"context_line":"        locking \u003d locking_factory.LockingFactory().provider"},{"line_number":154,"context_line":"        locking.clear_reservations_for_conductor(self.host)"},{"line_number":155,"context_line":"        try:"},{"line_number":156,"context_line":"            # Register this conductor with the cluster"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_0d749e67","line":153,"updated":"2019-06-06 09:22:25.000000000","message":"nit: create a helper function for this long line?\n\n locking \u003d locking_factory.get_provider()","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"e5c75bba02f8156a0c3b3a8bed2aa268d5cdca7c","unresolved":false,"context_lines":[{"line_number":516,"context_line":"            except (exception.NodeLocked, exception.NodeNotFound):"},{"line_number":517,"context_line":"                # NOTE(TheJulia): In most cases, NodeLocked, has never been"},{"line_number":518,"context_line":"                # encountered before the locking changes as the callers"},{"line_number":519,"context_line":"                # filter out nodes with reservations."},{"line_number":520,"context_line":"                continue"},{"line_number":521,"context_line":"            workers_count +\u003d 1"},{"line_number":522,"context_line":"            if workers_count \u003e\u003d CONF.conductor.periodic_max_workers:"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_2d6f2293","line":519,"updated":"2019-06-06 09:22:25.000000000","message":"nit: This note is useful in the context of this commit, but will seem out of order without this context","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1e32917fd7133f375594552a0cc84d18bb2c999e","unresolved":false,"context_lines":[{"line_number":516,"context_line":"            except (exception.NodeLocked, exception.NodeNotFound):"},{"line_number":517,"context_line":"                # NOTE(TheJulia): In most cases, NodeLocked, has never been"},{"line_number":518,"context_line":"                # encountered before the locking changes as the callers"},{"line_number":519,"context_line":"                # filter out nodes with reservations."},{"line_number":520,"context_line":"                continue"},{"line_number":521,"context_line":"            workers_count +\u003d 1"},{"line_number":522,"context_line":"            if workers_count \u003e\u003d CONF.conductor.periodic_max_workers:"}],"source_content_type":"text/x-python","patch_set":23,"id":"3fa7e38b_d7ca789e","line":519,"in_reply_to":"9fb8cfa7_2d6f2293","updated":"2019-12-23 23:34:28.000000000","message":"reworded to hopefully be more useful later in time.","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"}],"ironic/conductor/manager.py":[{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"9c7dcbb252ce602cb788d325fefc6b717882d99d","unresolved":false,"context_lines":[{"line_number":1798,"context_line":"        # TODO(TheJulia): The line above needs to be refactored to work"},{"line_number":1799,"context_line":"        # through a lock manager, not as a db filter."},{"line_number":1800,"context_line":"        # This will liekly need to be an enumeration through the offline"},{"line_number":1801,"context_line":"        # conductor\u0027s locks?"},{"line_number":1802,"context_line":"        state_cleanup_required \u003d []"},{"line_number":1803,"context_line":""},{"line_number":1804,"context_line":"        for (node_uuid, driver, conductor_group, node_id, conductor_hostname,"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_f1a9db8f","line":1801,"updated":"2018-09-02 07:53:57.000000000","message":"Could do some funky check and set operation to transfer ownership to this conductor.","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"b63a10adf49215de6b5ecfd2a7ea8bafa2ed387e","unresolved":false,"context_lines":[{"line_number":1798,"context_line":"        # TODO(TheJulia): The line above needs to be refactored to work"},{"line_number":1799,"context_line":"        # through a lock manager, not as a db filter."},{"line_number":1800,"context_line":"        # This will liekly need to be an enumeration through the offline"},{"line_number":1801,"context_line":"        # conductor\u0027s locks?"},{"line_number":1802,"context_line":"        state_cleanup_required \u003d []"},{"line_number":1803,"context_line":""},{"line_number":1804,"context_line":"        for (node_uuid, driver, conductor_group, node_id, conductor_hostname,"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_e304a380","line":1801,"in_reply_to":"3f79a3b5_52b9a312","updated":"2018-09-03 18:07:40.000000000","message":"Doesn\u0027t offline_conductors cover the heartbeat timeout though?","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"2e64021dc92aa0e27106bd421b6a6943a29041e7","unresolved":false,"context_lines":[{"line_number":1798,"context_line":"        # TODO(TheJulia): The line above needs to be refactored to work"},{"line_number":1799,"context_line":"        # through a lock manager, not as a db filter."},{"line_number":1800,"context_line":"        # This will liekly need to be an enumeration through the offline"},{"line_number":1801,"context_line":"        # conductor\u0027s locks?"},{"line_number":1802,"context_line":"        state_cleanup_required \u003d []"},{"line_number":1803,"context_line":""},{"line_number":1804,"context_line":"        for (node_uuid, driver, conductor_group, node_id, conductor_hostname,"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_52b9a312","line":1801,"in_reply_to":"3f79a3b5_f1a9db8f","updated":"2018-09-03 13:43:17.000000000","message":"I think the thing to do would be to check the conductor table to make sure the heartbeat has occurred in x amount of time. That way we\u0027re not  stomping on the lock. We could load it into memory once before we iterate through things, that was we\u0027re being relatively kind to the DB, and also cover the case where a conductor was down for a brief restart and the hashring recalculated in that time.","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"ec5156ec30402674269b0280af986456764804f6","unresolved":false,"context_lines":[{"line_number":1805,"context_line":"             maintenance, provision_state, target_power_state) in node_iter:"},{"line_number":1806,"context_line":"            # NOTE(lucasagomes): Although very rare, this may lead to a"},{"line_number":1807,"context_line":"            # race condition. By the time we release the lock the conductor"},{"line_number":1808,"context_line":"            # that was previously managing the node could be back online."},{"line_number":1809,"context_line":"            # NOTE(TheJulia): The resevation here should be from the lock"},{"line_number":1810,"context_line":"            # manager."},{"line_number":1811,"context_line":"            lock_holder \u003d self.locking.get_lock_owner(node_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_388ba1c0","line":1808,"updated":"2018-09-04 07:50:48.000000000","message":"A new lock manager could help with this kind of corner case. If the lock owner were to include some unique per-acquisition identifier, we could use check and set operations to ensure that the original stale lock is still active.","commit_id":"f945339c88285fe9c5fa8fb14188a45ff68e96b1"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d641b3319a53a7a674b395273c8e489307da2bfe","unresolved":false,"context_lines":[{"line_number":1805,"context_line":"             maintenance, provision_state, target_power_state) in node_iter:"},{"line_number":1806,"context_line":"            # NOTE(lucasagomes): Although very rare, this may lead to a"},{"line_number":1807,"context_line":"            # race condition. By the time we release the lock the conductor"},{"line_number":1808,"context_line":"            # that was previously managing the node could be back online."},{"line_number":1809,"context_line":"            # NOTE(TheJulia): The resevation here should be from the lock"},{"line_number":1810,"context_line":"            # manager."},{"line_number":1811,"context_line":"            lock_holder \u003d self.locking.get_lock_owner(node_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_d099196d","line":1808,"in_reply_to":"3f79a3b5_388ba1c0","updated":"2018-09-04 15:27:17.000000000","message":"I was kind of thinking that myself, although we do minimize that race even further by polling for the lock right before acting, as opposed to doing it when we\u0027re parsing node_iter data which becomes more stale with every iteration. Upon returning online, the conductor should clear the locks for things it was responsible for as well, which should remedy the entire situation for the node.","commit_id":"f945339c88285fe9c5fa8fb14188a45ff68e96b1"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"41922af381f3acbec5233a04dcb769565a8a12ff","unresolved":false,"context_lines":[{"line_number":1806,"context_line":"        #              seem right."},{"line_number":1807,"context_line":"        callback_timeout \u003d CONF.conductor.deploy_callback_timeout"},{"line_number":1808,"context_line":""},{"line_number":1809,"context_line":"        filters \u003d {\u0027provision_state\u0027: states.DEPLOYWAIT,"},{"line_number":1810,"context_line":"                   \u0027maintenance\u0027: False,"},{"line_number":1811,"context_line":"                   \u0027provisioned_before\u0027: callback_timeout}"},{"line_number":1812,"context_line":"        sort_key \u003d \u0027provision_updated_at\u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_de6667be","line":1809,"updated":"2018-12-13 17:07:26.000000000","message":"A potential middle ground would be\n\n filters \u003d self.locking.add_database_filters(filters)\n\nI\u0027m not sure it\u0027s worth the effort though. This is racy anyway.","commit_id":"c601696b7d5a8b6ac3704f127dc335bf85c56b99"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"cb69cb54288d3cf72d3b3403484cfa205600da24","unresolved":false,"context_lines":[{"line_number":1806,"context_line":"        #              seem right."},{"line_number":1807,"context_line":"        callback_timeout \u003d CONF.conductor.deploy_callback_timeout"},{"line_number":1808,"context_line":""},{"line_number":1809,"context_line":"        filters \u003d {\u0027provision_state\u0027: states.DEPLOYWAIT,"},{"line_number":1810,"context_line":"                   \u0027maintenance\u0027: False,"},{"line_number":1811,"context_line":"                   \u0027provisioned_before\u0027: callback_timeout}"},{"line_number":1812,"context_line":"        sort_key \u003d \u0027provision_updated_at\u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_dffaf0bf","line":1809,"updated":"2018-12-13 08:37:05.000000000","message":"this seems like a workaround to avoid heavy refactoring on the db, but by doing so, we\u0027ll introduce unnecessary calls on locked nodes.","commit_id":"c601696b7d5a8b6ac3704f127dc335bf85c56b99"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1cb803a11e87b58496b6d4b9a5b3d3e8e6a420c0","unresolved":false,"context_lines":[{"line_number":1806,"context_line":"        #              seem right."},{"line_number":1807,"context_line":"        callback_timeout \u003d CONF.conductor.deploy_callback_timeout"},{"line_number":1808,"context_line":""},{"line_number":1809,"context_line":"        filters \u003d {\u0027provision_state\u0027: states.DEPLOYWAIT,"},{"line_number":1810,"context_line":"                   \u0027maintenance\u0027: False,"},{"line_number":1811,"context_line":"                   \u0027provisioned_before\u0027: callback_timeout}"},{"line_number":1812,"context_line":"        sort_key \u003d \u0027provision_updated_at\u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_a40fdc2a","line":1809,"in_reply_to":"3f79a3b5_de6667be","updated":"2018-12-14 01:07:20.000000000","message":"I think that was the other thought I had, that database state does not represent locking state. (and if the loop takes a while to enumerate through.... the database could be out of date.)","commit_id":"c601696b7d5a8b6ac3704f127dc335bf85c56b99"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"296749680efbad88ff4ced78b26eb8217b4b8455","unresolved":false,"context_lines":[{"line_number":1806,"context_line":"        #              seem right."},{"line_number":1807,"context_line":"        callback_timeout \u003d CONF.conductor.deploy_callback_timeout"},{"line_number":1808,"context_line":""},{"line_number":1809,"context_line":"        filters \u003d {\u0027provision_state\u0027: states.DEPLOYWAIT,"},{"line_number":1810,"context_line":"                   \u0027maintenance\u0027: False,"},{"line_number":1811,"context_line":"                   \u0027provisioned_before\u0027: callback_timeout}"},{"line_number":1812,"context_line":"        sort_key \u003d \u0027provision_updated_at\u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_4dc873a9","line":1809,"in_reply_to":"3f79a3b5_dffaf0bf","updated":"2018-12-13 15:28:02.000000000","message":"A semi good point. It is not a workaround. It was more a \"I don\u0027t think it is really valid for us to do this anymore on the filters. The reserved field just really can\u0027t be trusted (or can it? did I fix that?) (I guess I need to go look).","commit_id":"c601696b7d5a8b6ac3704f127dc335bf85c56b99"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"41922af381f3acbec5233a04dcb769565a8a12ff","unresolved":false,"context_lines":[{"line_number":1849,"context_line":"            offline_conductors)"},{"line_number":1850,"context_line":"        # If nothing is returned for stale locks, it makes no"},{"line_number":1851,"context_line":"        # sense to continue."},{"line_number":1852,"context_line":"        if stale_locks is {}:"},{"line_number":1853,"context_line":"            return"},{"line_number":1854,"context_line":"        state_cleanup_required \u003d []"},{"line_number":1855,"context_line":"        for (node_uuid, driver, conductor_group, node_id,"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_7e6393ad","line":1852,"updated":"2018-12-13 17:07:26.000000000","message":"if not stale_locks\n\n\"is\" is not guaranteed to work this way.","commit_id":"c601696b7d5a8b6ac3704f127dc335bf85c56b99"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"365eef31e0a1f1f5967601c5226b3458c7116a04","unresolved":false,"context_lines":[{"line_number":1915,"context_line":"            offline_conductors)"},{"line_number":1916,"context_line":"        # If nothing is returned for stale locks, it makes no"},{"line_number":1917,"context_line":"        # sense to continue."},{"line_number":1918,"context_line":"        if stale_locks is {}:"},{"line_number":1919,"context_line":"            return"},{"line_number":1920,"context_line":"        state_cleanup_required \u003d []"},{"line_number":1921,"context_line":"        for (node_uuid, driver, conductor_group, node_id,"}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_a9b78cd8","line":1918,"range":{"start_line":1918,"start_character":11,"end_line":1918,"end_character":28},"updated":"2019-02-07 18:22:14.000000000","message":"hold on, will this ever be True? The id\u0027s of `stale_locks` and `{}` will always be different I think.","commit_id":"468d436bd3c7e82888b60748d073ea09588908e0"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d8c0d6bc2c72efd851816d11056a389ba063364f","unresolved":false,"context_lines":[{"line_number":1915,"context_line":"            offline_conductors)"},{"line_number":1916,"context_line":"        # If nothing is returned for stale locks, it makes no"},{"line_number":1917,"context_line":"        # sense to continue."},{"line_number":1918,"context_line":"        if stale_locks is {}:"},{"line_number":1919,"context_line":"            return"},{"line_number":1920,"context_line":"        state_cleanup_required \u003d []"},{"line_number":1921,"context_line":"        for (node_uuid, driver, conductor_group, node_id,"}],"source_content_type":"text/x-python","patch_set":18,"id":"5fc1f717_c8f2ee06","line":1918,"range":{"start_line":1918,"start_character":11,"end_line":1918,"end_character":28},"in_reply_to":"9fdfeff1_a9b78cd8","updated":"2019-03-22 21:41:40.000000000","message":"it _should_ be true if there are no locks and {} is returned. When {} is returned we\u0027re just going to skip out of the periodic task otherwise.","commit_id":"468d436bd3c7e82888b60748d073ea09588908e0"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"365eef31e0a1f1f5967601c5226b3458c7116a04","unresolved":false,"context_lines":[{"line_number":1926,"context_line":"            # NOTE(TheJulia): The resevation here should be from the lock"},{"line_number":1927,"context_line":"            # manager."},{"line_number":1928,"context_line":"            lock_holder \u003d self.locking.get_lock_owner(node_uuid)"},{"line_number":1929,"context_line":"            if lock_holder is None:"},{"line_number":1930,"context_line":"                # No lock here, carry on!"},{"line_number":1931,"context_line":"                continue"},{"line_number":1932,"context_line":"            if lock_holder not in stale_locks:"}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_c9df700a","line":1929,"range":{"start_line":1929,"start_character":27,"end_line":1929,"end_character":29},"updated":"2019-02-07 18:22:14.000000000","message":"Why `is`, not just evaluation for being false?","commit_id":"468d436bd3c7e82888b60748d073ea09588908e0"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d8c0d6bc2c72efd851816d11056a389ba063364f","unresolved":false,"context_lines":[{"line_number":1926,"context_line":"            # NOTE(TheJulia): The resevation here should be from the lock"},{"line_number":1927,"context_line":"            # manager."},{"line_number":1928,"context_line":"            lock_holder \u003d self.locking.get_lock_owner(node_uuid)"},{"line_number":1929,"context_line":"            if lock_holder is None:"},{"line_number":1930,"context_line":"                # No lock here, carry on!"},{"line_number":1931,"context_line":"                continue"},{"line_number":1932,"context_line":"            if lock_holder not in stale_locks:"}],"source_content_type":"text/x-python","patch_set":18,"id":"5fc1f717_a8d7624f","line":1929,"range":{"start_line":1929,"start_character":27,"end_line":1929,"end_character":29},"in_reply_to":"9fdfeff1_c9df700a","updated":"2019-03-22 21:41:40.000000000","message":"Hmm, I think that should work.","commit_id":"468d436bd3c7e82888b60748d073ea09588908e0"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"365eef31e0a1f1f5967601c5226b3458c7116a04","unresolved":false,"context_lines":[{"line_number":1929,"context_line":"            if lock_holder is None:"},{"line_number":1930,"context_line":"                # No lock here, carry on!"},{"line_number":1931,"context_line":"                continue"},{"line_number":1932,"context_line":"            if lock_holder not in stale_locks:"},{"line_number":1933,"context_line":"                # Literally no point if there are no possibly stale"},{"line_number":1934,"context_line":"                # locks."},{"line_number":1935,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_49cb8049","line":1932,"range":{"start_line":1932,"start_character":12,"end_line":1932,"end_character":46},"updated":"2019-02-07 18:22:14.000000000","message":"Given this condition, the one on line 1929 might not be required...","commit_id":"468d436bd3c7e82888b60748d073ea09588908e0"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d8c0d6bc2c72efd851816d11056a389ba063364f","unresolved":false,"context_lines":[{"line_number":1929,"context_line":"            if lock_holder is None:"},{"line_number":1930,"context_line":"                # No lock here, carry on!"},{"line_number":1931,"context_line":"                continue"},{"line_number":1932,"context_line":"            if lock_holder not in stale_locks:"},{"line_number":1933,"context_line":"                # Literally no point if there are no possibly stale"},{"line_number":1934,"context_line":"                # locks."},{"line_number":1935,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":18,"id":"5fc1f717_c8598efe","line":1932,"range":{"start_line":1932,"start_character":12,"end_line":1932,"end_character":46},"in_reply_to":"9fdfeff1_49cb8049","updated":"2019-03-22 21:41:40.000000000","message":"yeah, but we save cpu cycles going ahead and skipping the compare if there is no lock.\n\nIf there are any stales locks, we end up looping through all of the nodes and if there are many then we start doing tons of compares.... (Just thinking out loud...)","commit_id":"468d436bd3c7e82888b60748d073ea09588908e0"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"36d3b2fab689b38939089f158a7f55ae13b0c1f2","unresolved":false,"context_lines":[{"line_number":1755,"context_line":"                    if (task.node.provision_state in SYNC_EXCLUDED_STATES"},{"line_number":1756,"context_line":"                            or task.node.maintenance"},{"line_number":1757,"context_line":"                            or task.node.target_power_state"},{"line_number":1758,"context_line":"                            or self.locking.get_lock_owner(node_uuid)):"},{"line_number":1759,"context_line":"                        continue"},{"line_number":1760,"context_line":"                    count \u003d do_sync_power_state("},{"line_number":1761,"context_line":"                        task, self.power_state_sync_count[node_uuid])"}],"source_content_type":"text/x-python","patch_set":20,"id":"ffb9cba7_11ea40ab","line":1758,"updated":"2019-04-29 04:04:16.000000000","message":"we can make it more efficient if we have something like self.locking.is_locked(task) since for the database the implementation will be the same","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"03def507e4ad023edbd7286681ddac1eaec12047","unresolved":false,"context_lines":[{"line_number":1755,"context_line":"                    if (task.node.provision_state in SYNC_EXCLUDED_STATES"},{"line_number":1756,"context_line":"                            or task.node.maintenance"},{"line_number":1757,"context_line":"                            or task.node.target_power_state"},{"line_number":1758,"context_line":"                            or self.locking.get_lock_owner(node_uuid)):"},{"line_number":1759,"context_line":"                        continue"},{"line_number":1760,"context_line":"                    count \u003d do_sync_power_state("},{"line_number":1761,"context_line":"                        task, self.power_state_sync_count[node_uuid])"}],"source_content_type":"text/x-python","patch_set":20,"id":"bfb3d3c7_53bb1171","line":1758,"in_reply_to":"ffb9cba7_11ea40ab","updated":"2019-05-22 18:58:09.000000000","message":"True, but I wonder if we\u0027re just at method naming... Of course I\u0027d also like to make it such that only standalone and small scale operators use database locking.","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"36d3b2fab689b38939089f158a7f55ae13b0c1f2","unresolved":false,"context_lines":[{"line_number":1800,"context_line":"                    not task.node.maintenance or"},{"line_number":1801,"context_line":"                    task.node.fault !\u003d faults.POWER_FAILURE or"},{"line_number":1802,"context_line":"                    task.node.target_power_state or"},{"line_number":1803,"context_line":"                    self.locking.get_lock_owner(task.node.uuid)):"},{"line_number":1804,"context_line":"                return False"},{"line_number":1805,"context_line":"            return True"},{"line_number":1806,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"ffb9cba7_b1d254ff","line":1803,"updated":"2019-04-29 04:04:16.000000000","message":"ditto","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"fdbf07d0250241e0613f7abcc7c4e7fee7c9cc5a","unresolved":false,"context_lines":[{"line_number":1926,"context_line":"        # If nothing is returned for stale locks, it makes no"},{"line_number":1927,"context_line":"        # sense to continue and get_locks_for_conductor returns"},{"line_number":1928,"context_line":"        # an empty dictionary."},{"line_number":1929,"context_line":"        if stale_locks is {}:"},{"line_number":1930,"context_line":"            return"},{"line_number":1931,"context_line":"        state_cleanup_required \u003d []"},{"line_number":1932,"context_line":"        for (node_uuid, driver, conductor_group, node_id,"}],"source_content_type":"text/x-python","patch_set":20,"id":"3fce034c_b1359ddc","line":1929,"range":{"start_line":1929,"start_character":0,"end_line":1929,"end_character":29},"updated":"2019-04-16 11:39:48.000000000","message":"I\u0027m not sure this works, `is` is supposed to be used as comparison for object identities, not values\nI\u0027d use `\u003d\u003d` (if you want to exclude None) or just `if not stale_locks`\nAdding a test case should help verify this condition.","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"7b1fb9164f6509147ce9d03917bc374406dc17b7","unresolved":false,"context_lines":[{"line_number":1926,"context_line":"        # If nothing is returned for stale locks, it makes no"},{"line_number":1927,"context_line":"        # sense to continue and get_locks_for_conductor returns"},{"line_number":1928,"context_line":"        # an empty dictionary."},{"line_number":1929,"context_line":"        if stale_locks is {}:"},{"line_number":1930,"context_line":"            return"},{"line_number":1931,"context_line":"        state_cleanup_required \u003d []"},{"line_number":1932,"context_line":"        for (node_uuid, driver, conductor_group, node_id,"}],"source_content_type":"text/x-python","patch_set":20,"id":"3fce034c_51ea39ac","line":1929,"range":{"start_line":1929,"start_character":0,"end_line":1929,"end_character":29},"in_reply_to":"3fce034c_b1359ddc","updated":"2019-04-16 11:43:43.000000000","message":"You\u0027re right, \u0027is\u0027 must not be used for any comparison, except for None, True, False and when you literally mean \"this very object in memory\".","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"36d3b2fab689b38939089f158a7f55ae13b0c1f2","unresolved":false,"context_lines":[{"line_number":2169,"context_line":"                    # TODO(TheJulia): We ought to make this a shared task,"},{"line_number":2170,"context_line":"                    # then check the lock status, remove the old lock since"},{"line_number":2171,"context_line":"                    # the prior conductor is gone. Then, upgrade the lock,"},{"line_number":2172,"context_line":"                    # and proceed with triggering takeover."},{"line_number":2173,"context_line":"                    # NOTE(deva): now that we have the lock, check again to"},{"line_number":2174,"context_line":"                    # avoid racing with deletes and other state changes"},{"line_number":2175,"context_line":"                    node \u003d task.node"}],"source_content_type":"text/x-python","patch_set":20,"id":"ffb9cba7_d1cfc853","line":2172,"updated":"2019-04-29 04:04:16.000000000","message":"++","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"e5c75bba02f8156a0c3b3a8bed2aa268d5cdca7c","unresolved":false,"context_lines":[{"line_number":2170,"context_line":"                    # then check the lock status, remove the old lock since"},{"line_number":2171,"context_line":"                    # the prior conductor is gone. Then, upgrade the lock,"},{"line_number":2172,"context_line":"                    # and proceed with triggering takeover."},{"line_number":2173,"context_line":"                    # NOTE(TheJulia): And Dmitry agrees with the to do item."},{"line_number":2174,"context_line":"                    # NOTE(deva): now that we have the lock, check again to"},{"line_number":2175,"context_line":"                    # avoid racing with deletes and other state changes"},{"line_number":2176,"context_line":"                    node \u003d task.node"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_4d5a96f3","line":2173,"updated":"2019-06-06 09:22:25.000000000","message":"nit: I like my mention, but it may be a bit redundant","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1e32917fd7133f375594552a0cc84d18bb2c999e","unresolved":false,"context_lines":[{"line_number":2170,"context_line":"                    # then check the lock status, remove the old lock since"},{"line_number":2171,"context_line":"                    # the prior conductor is gone. Then, upgrade the lock,"},{"line_number":2172,"context_line":"                    # and proceed with triggering takeover."},{"line_number":2173,"context_line":"                    # NOTE(TheJulia): And Dmitry agrees with the to do item."},{"line_number":2174,"context_line":"                    # NOTE(deva): now that we have the lock, check again to"},{"line_number":2175,"context_line":"                    # avoid racing with deletes and other state changes"},{"line_number":2176,"context_line":"                    node \u003d task.node"}],"source_content_type":"text/x-python","patch_set":23,"id":"3fa7e38b_97bc0008","line":2173,"in_reply_to":"9fb8cfa7_4d5a96f3","updated":"2019-12-23 23:34:28.000000000","message":"Done","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"e5c75bba02f8156a0c3b3a8bed2aa268d5cdca7c","unresolved":false,"context_lines":[{"line_number":2185,"context_line":"            except exception.NoFreeConductorWorker:"},{"line_number":2186,"context_line":"                break"},{"line_number":2187,"context_line":"            except (exception.NodeLocked, exception.NodeNotFound):"},{"line_number":2188,"context_line":"                # TODO(TheJulia): We should remove NodeLocked from this list."},{"line_number":2189,"context_line":"                continue"},{"line_number":2190,"context_line":"            workers_count +\u003d 1"},{"line_number":2191,"context_line":"            if workers_count \u003d\u003d CONF.conductor.periodic_max_workers:"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_6d5d1ae8","line":2188,"updated":"2019-06-06 09:22:25.000000000","message":"I\u0027d keep it here just in case","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1e32917fd7133f375594552a0cc84d18bb2c999e","unresolved":false,"context_lines":[{"line_number":2185,"context_line":"            except exception.NoFreeConductorWorker:"},{"line_number":2186,"context_line":"                break"},{"line_number":2187,"context_line":"            except (exception.NodeLocked, exception.NodeNotFound):"},{"line_number":2188,"context_line":"                # TODO(TheJulia): We should remove NodeLocked from this list."},{"line_number":2189,"context_line":"                continue"},{"line_number":2190,"context_line":"            workers_count +\u003d 1"},{"line_number":2191,"context_line":"            if workers_count \u003d\u003d CONF.conductor.periodic_max_workers:"}],"source_content_type":"text/x-python","patch_set":23,"id":"3fa7e38b_57b68827","line":2188,"in_reply_to":"9fb8cfa7_6d5d1ae8","updated":"2019-12-23 23:34:28.000000000","message":"Reworded as a note to hopefully convey why we may want to remove it and under what conditions that should be safe.","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"}],"ironic/conductor/task_manager.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"e5c75bba02f8156a0c3b3a8bed2aa268d5cdca7c","unresolved":false,"context_lines":[{"line_number":124,"context_line":"CONF \u003d cfg.CONF"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"# NOTE(TheJulia): Do this once here, instead of each"},{"line_number":127,"context_line":"# time we load a task."},{"line_number":128,"context_line":"LOCKING \u003d locking_factory.LockingFactory().provider"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_ed4fca31","line":127,"updated":"2019-06-06 09:22:25.000000000","message":"nit: The provider is cached, so you\u0027re not winning much","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1e32917fd7133f375594552a0cc84d18bb2c999e","unresolved":false,"context_lines":[{"line_number":124,"context_line":"CONF \u003d cfg.CONF"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"# NOTE(TheJulia): Do this once here, instead of each"},{"line_number":127,"context_line":"# time we load a task."},{"line_number":128,"context_line":"LOCKING \u003d locking_factory.LockingFactory().provider"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"3fa7e38b_d73fd883","line":127,"in_reply_to":"9fb8cfa7_ed4fca31","updated":"2019-12-23 23:34:28.000000000","message":"I wasn\u0027t thinking performance when I wrote this, I was thinking not delaying a hard failure into at task. If this is instantiated up front, things should implode upon startup... I think.","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"}],"ironic/db/sqlalchemy/api.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"41922af381f3acbec5233a04dcb769565a8a12ff","unresolved":false,"context_lines":[{"line_number":249,"context_line":"            if filters[\u0027reserved\u0027]:"},{"line_number":250,"context_line":"                query \u003d query.filter(models.Node.reservation !\u003d sql.null())"},{"line_number":251,"context_line":"            else:"},{"line_number":252,"context_line":"                query \u003d query.filter(models.Node.reservation \u003d\u003d sql.null())"},{"line_number":253,"context_line":"        if \u0027reserved_by_any_of\u0027 in filters:"},{"line_number":254,"context_line":"            query \u003d query.filter(models.Node.reservation.in_("},{"line_number":255,"context_line":"                filters[\u0027reserved_by_any_of\u0027]))"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_3efbdbc5","line":252,"updated":"2018-12-13 17:07:26.000000000","message":"No idea :(","commit_id":"c601696b7d5a8b6ac3704f127dc335bf85c56b99"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"296749680efbad88ff4ced78b26eb8217b4b8455","unresolved":false,"context_lines":[{"line_number":246,"context_line":"            else:"},{"line_number":247,"context_line":"                query \u003d query.filter(models.Node.instance_uuid \u003d\u003d sql.null())"},{"line_number":248,"context_line":"        if \u0027reserved\u0027 in filters:"},{"line_number":249,"context_line":"            if filters[\u0027reserved\u0027]:"},{"line_number":250,"context_line":"                query \u003d query.filter(models.Node.reservation !\u003d sql.null())"},{"line_number":251,"context_line":"            else:"},{"line_number":252,"context_line":"                query \u003d query.filter(models.Node.reservation \u003d\u003d sql.null())"},{"line_number":253,"context_line":"        if \u0027reserved_by_any_of\u0027 in filters:"},{"line_number":254,"context_line":"            query \u003d query.filter(models.Node.reservation.in_("},{"line_number":255,"context_line":"                filters[\u0027reserved_by_any_of\u0027]))"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_0dacbba6","line":252,"range":{"start_line":249,"start_character":35,"end_line":252,"end_character":75},"updated":"2018-12-13 15:28:02.000000000","message":"ugh, this has a different meaning with non-db locking. why did we do this to ourselves. :(","commit_id":"c601696b7d5a8b6ac3704f127dc335bf85c56b99"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1cb803a11e87b58496b6d4b9a5b3d3e8e6a420c0","unresolved":false,"context_lines":[{"line_number":249,"context_line":"            if filters[\u0027reserved\u0027]:"},{"line_number":250,"context_line":"                query \u003d query.filter(models.Node.reservation !\u003d sql.null())"},{"line_number":251,"context_line":"            else:"},{"line_number":252,"context_line":"                query \u003d query.filter(models.Node.reservation \u003d\u003d sql.null())"},{"line_number":253,"context_line":"        if \u0027reserved_by_any_of\u0027 in filters:"},{"line_number":254,"context_line":"            query \u003d query.filter(models.Node.reservation.in_("},{"line_number":255,"context_line":"                filters[\u0027reserved_by_any_of\u0027]))"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_048fb021","line":252,"in_reply_to":"3f79a3b5_3efbdbc5","updated":"2018-12-14 01:07:20.000000000","message":"We don\u0027t actually use it now outside of tests. I\u0027m just going to deprecate it.","commit_id":"c601696b7d5a8b6ac3704f127dc335bf85c56b99"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"cb69cb54288d3cf72d3b3403484cfa205600da24","unresolved":false,"context_lines":[{"line_number":291,"context_line":"    def reserve_node(self, tag, node_id):"},{"line_number":292,"context_line":"        # NOTE(TheJulia): While this is superceeded by the locking"},{"line_number":293,"context_line":"        # provider, this is still used by the default locking"},{"line_number":294,"context_line":"        # mechanism behavior to ensure compatability."},{"line_number":295,"context_line":"        with _session_for_write():"},{"line_number":296,"context_line":"            query \u003d _get_node_query_with_all()"},{"line_number":297,"context_line":"            query \u003d add_identity_filter(query, node_id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_8dc53561","line":294,"updated":"2018-12-13 08:37:05.000000000","message":"what about the node filtering on the reserved field?\nif we use another locking provider, there are more interfaces affected beyond these commented.\ni feel we\u0027ll need an abstraction above the db api, or even move reserved field out of the node table.","commit_id":"c601696b7d5a8b6ac3704f127dc335bf85c56b99"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1cb803a11e87b58496b6d4b9a5b3d3e8e6a420c0","unresolved":false,"context_lines":[{"line_number":291,"context_line":"    def reserve_node(self, tag, node_id):"},{"line_number":292,"context_line":"        # NOTE(TheJulia): While this is superceeded by the locking"},{"line_number":293,"context_line":"        # provider, this is still used by the default locking"},{"line_number":294,"context_line":"        # mechanism behavior to ensure compatability."},{"line_number":295,"context_line":"        with _session_for_write():"},{"line_number":296,"context_line":"            query \u003d _get_node_query_with_all()"},{"line_number":297,"context_line":"            query \u003d add_identity_filter(query, node_id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_c4c53860","line":294,"in_reply_to":"3f79a3b5_2d941735","updated":"2018-12-14 01:07:20.000000000","message":"\\o/ we didn\u0027t allow reservation search \\o/","commit_id":"c601696b7d5a8b6ac3704f127dc335bf85c56b99"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"296749680efbad88ff4ced78b26eb8217b4b8455","unresolved":false,"context_lines":[{"line_number":291,"context_line":"    def reserve_node(self, tag, node_id):"},{"line_number":292,"context_line":"        # NOTE(TheJulia): While this is superceeded by the locking"},{"line_number":293,"context_line":"        # provider, this is still used by the default locking"},{"line_number":294,"context_line":"        # mechanism behavior to ensure compatability."},{"line_number":295,"context_line":"        with _session_for_write():"},{"line_number":296,"context_line":"            query \u003d _get_node_query_with_all()"},{"line_number":297,"context_line":"            query \u003d add_identity_filter(query, node_id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_2d941735","line":294,"in_reply_to":"3f79a3b5_8dc53561","updated":"2018-12-13 15:28:02.000000000","message":"I think we do two things.\n\n1) We deprecate internal use of reserved filter against the db.\n2) For API interactions, we query the locking interface for a list of active lock holders, and then use those results to filter the api result back to the client so the effect and behavior is the same.","commit_id":"c601696b7d5a8b6ac3704f127dc335bf85c56b99"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1cb803a11e87b58496b6d4b9a5b3d3e8e6a420c0","unresolved":false,"context_lines":[{"line_number":291,"context_line":"    def reserve_node(self, tag, node_id):"},{"line_number":292,"context_line":"        # NOTE(TheJulia): While this is superceeded by the locking"},{"line_number":293,"context_line":"        # provider, this is still used by the default locking"},{"line_number":294,"context_line":"        # mechanism behavior to ensure compatability."},{"line_number":295,"context_line":"        with _session_for_write():"},{"line_number":296,"context_line":"            query \u003d _get_node_query_with_all()"},{"line_number":297,"context_line":"            query \u003d add_identity_filter(query, node_id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_44592866","line":294,"in_reply_to":"3f79a3b5_8dc53561","updated":"2018-12-14 01:07:20.000000000","message":"In pondering, I think maintaining the same database level functionality is the wrong path to take. While it does provide a combined surface, we got into a pattern of relying the two to be same and consistent when they are not. Hence why I think we ended up heading in the direction (now, and in the past desire wise) of using a dlm.","commit_id":"c601696b7d5a8b6ac3704f127dc335bf85c56b99"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"36d3b2fab689b38939089f158a7f55ae13b0c1f2","unresolved":false,"context_lines":[{"line_number":324,"context_line":"                        \u0027interface. locking.get_lock_owner(uuid) should be \u0027"},{"line_number":325,"context_line":"                        \u0027used instead.\u0027)"},{"line_number":326,"context_line":"            # TODO(TheJulia): The reserved keyword has been deprecated and"},{"line_number":327,"context_line":"            # will be removed in a future release. (Earliest would be Train)"},{"line_number":328,"context_line":"            if filters[\u0027reserved\u0027]:"},{"line_number":329,"context_line":"                query \u003d query.filter(models.Node.reservation !\u003d sql.null())"},{"line_number":330,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":20,"id":"ffb9cba7_71c37c39","line":327,"updated":"2019-04-29 04:04:16.000000000","message":"not Train any more..","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"03def507e4ad023edbd7286681ddac1eaec12047","unresolved":false,"context_lines":[{"line_number":324,"context_line":"                        \u0027interface. locking.get_lock_owner(uuid) should be \u0027"},{"line_number":325,"context_line":"                        \u0027used instead.\u0027)"},{"line_number":326,"context_line":"            # TODO(TheJulia): The reserved keyword has been deprecated and"},{"line_number":327,"context_line":"            # will be removed in a future release. (Earliest would be Train)"},{"line_number":328,"context_line":"            if filters[\u0027reserved\u0027]:"},{"line_number":329,"context_line":"                query \u003d query.filter(models.Node.reservation !\u003d sql.null())"},{"line_number":330,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":20,"id":"bfb3d3c7_d37b81bb","line":327,"in_reply_to":"ffb9cba7_71c37c39","updated":"2019-05-22 18:58:09.000000000","message":"Done","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"}],"ironic/drivers/modules/drac/raid.py":[{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"179965bc649c172749b862b5e3c61b361f1047c9","unresolved":false,"context_lines":[{"line_number":825,"context_line":"                         \"%(node)s was not found and presumed deleted by \""},{"line_number":826,"context_line":"                         \"another process.\", {\u0027node\u0027: node_uuid})"},{"line_number":827,"context_line":"            except exception.NodeLocked:"},{"line_number":828,"context_line":"                LOG.info(\"During query_raid_config_job_status, node \""},{"line_number":829,"context_line":"                         \"%(node)s was already locked by another process. \""},{"line_number":830,"context_line":"                         \"Skip.\", {\u0027node\u0027: node_uuid})"},{"line_number":831,"context_line":""},{"line_number":832,"context_line":"    @METRICS.timer(\u0027DracRAID._check_node_raid_jobs\u0027)"},{"line_number":833,"context_line":"    def _check_node_raid_jobs(self, task):"}],"source_content_type":"text/x-python","patch_set":14,"id":"ffd0ebdf_19918c53","line":830,"range":{"start_line":828,"start_character":0,"end_line":830,"end_character":54},"updated":"2019-01-03 11:29:26.000000000","message":"This will now be a more regular occurrence, so should probably be downgraded to debug.","commit_id":"fe46cbbd6619cc67e158c99b92abb15605bd0d04"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"a7f22590d2c199f9a35c1395903e25de526b8c3f","unresolved":false,"context_lines":[{"line_number":825,"context_line":"                         \"%(node)s was not found and presumed deleted by \""},{"line_number":826,"context_line":"                         \"another process.\", {\u0027node\u0027: node_uuid})"},{"line_number":827,"context_line":"            except exception.NodeLocked:"},{"line_number":828,"context_line":"                LOG.info(\"During query_raid_config_job_status, node \""},{"line_number":829,"context_line":"                         \"%(node)s was already locked by another process. \""},{"line_number":830,"context_line":"                         \"Skip.\", {\u0027node\u0027: node_uuid})"},{"line_number":831,"context_line":""},{"line_number":832,"context_line":"    @METRICS.timer(\u0027DracRAID._check_node_raid_jobs\u0027)"},{"line_number":833,"context_line":"    def _check_node_raid_jobs(self, task):"}],"source_content_type":"text/x-python","patch_set":14,"id":"dfd5e7cf_b4e2ce2e","line":830,"range":{"start_line":828,"start_character":0,"end_line":830,"end_character":54},"in_reply_to":"ffd0ebdf_19918c53","updated":"2019-01-04 23:51:07.000000000","message":"I concur.","commit_id":"fe46cbbd6619cc67e158c99b92abb15605bd0d04"}],"ironic/drivers/modules/irmc/raid.py":[{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"3ba1baf8ff19ff9a0a95e46c1371a8cca220da7d","unresolved":false,"context_lines":[{"line_number":428,"context_line":"        spacing\u003dCONF.irmc.query_raid_config_fgi_status_interval)"},{"line_number":429,"context_line":"    def _query_raid_config_fgi_status(self, manager, context):"},{"line_number":430,"context_line":"        \"\"\"Periodic tasks to check the progress of running RAID config.\"\"\""},{"line_number":431,"context_line":"        filters \u003d {\u0027provision_state\u0027: states.CLEANWAIT,"},{"line_number":432,"context_line":"                   \u0027maintenance\u0027: False}"},{"line_number":433,"context_line":"        fields \u003d [\u0027raid_config\u0027]"},{"line_number":434,"context_line":"        node_list \u003d manager.iter_nodes(fields\u003dfields, filters\u003dfilters)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_00633cad","line":431,"updated":"2018-10-31 08:28:25.000000000","message":"the change on periodic tasks can go into another patch as an improvement, imo.","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d967b2004f3b2c8300171698292cff2d72a14035","unresolved":false,"context_lines":[{"line_number":428,"context_line":"        spacing\u003dCONF.irmc.query_raid_config_fgi_status_interval)"},{"line_number":429,"context_line":"    def _query_raid_config_fgi_status(self, manager, context):"},{"line_number":430,"context_line":"        \"\"\"Periodic tasks to check the progress of running RAID config.\"\"\""},{"line_number":431,"context_line":"        filters \u003d {\u0027provision_state\u0027: states.CLEANWAIT,"},{"line_number":432,"context_line":"                   \u0027maintenance\u0027: False}"},{"line_number":433,"context_line":"        fields \u003d [\u0027raid_config\u0027]"},{"line_number":434,"context_line":"        node_list \u003d manager.iter_nodes(fields\u003dfields, filters\u003dfilters)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_39c7fc99","line":431,"in_reply_to":"3f79a3b5_00633cad","updated":"2018-11-28 16:24:04.000000000","message":"I guess I\u0027m just hoping to prevent a weird intermediate state.\n\nActually, they are semi-necessary. They are database search queries, so the database may no longer be a valid source of truth for lock status.","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"179965bc649c172749b862b5e3c61b361f1047c9","unresolved":false,"context_lines":[{"line_number":488,"context_line":"                         \u0027%(node)s was not found raid_config and presumed \u0027"},{"line_number":489,"context_line":"                         \u0027deleted by another process.\u0027, {\u0027node\u0027: node_uuid})"},{"line_number":490,"context_line":"            except exception.NodeLocked:"},{"line_number":491,"context_line":"                LOG.info(\u0027During query_raid_config_job_status, node \u0027"},{"line_number":492,"context_line":"                         \u0027%(node)s was already locked by another process. \u0027"},{"line_number":493,"context_line":"                         \u0027Skip.\u0027, {\u0027node\u0027: node_uuid})"},{"line_number":494,"context_line":""},{"line_number":495,"context_line":"    def _set_clean_failed(self, task, fgi_status_dict):"},{"line_number":496,"context_line":"        LOG.error(\u0027RAID configuration task failed for node %(node)s. \u0027"}],"source_content_type":"text/x-python","patch_set":14,"id":"ffd0ebdf_b976403e","line":493,"range":{"start_line":491,"start_character":16,"end_line":493,"end_character":54},"updated":"2019-01-03 11:29:26.000000000","message":"debug","commit_id":"fe46cbbd6619cc67e158c99b92abb15605bd0d04"}],"ironic/locking/base.py":[{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"9c7dcbb252ce602cb788d325fefc6b717882d99d","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"@six.add_metaclass(abc.ABCMeta)"},{"line_number":23,"context_line":"class BaseLocking(object):"},{"line_number":24,"context_line":"    \"\"\"Base class for DHCP provider APIs.\"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    @abc.abstractmethod"},{"line_number":27,"context_line":"    def is_locked(self, task_or_uuid):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_b193635c","line":24,"updated":"2018-09-02 07:53:57.000000000","message":"DHCP","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"9c7dcbb252ce602cb788d325fefc6b717882d99d","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    \"\"\"Base class for DHCP provider APIs.\"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    @abc.abstractmethod"},{"line_number":27,"context_line":"    def is_locked(self, task_or_uuid):"},{"line_number":28,"context_line":"        \"\"\"Return string if node in the `task` is locked."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"        :param task: A TaskManager instance."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_518c4f34","line":27,"updated":"2018-09-02 07:53:57.000000000","message":"For this and the other tasks, I\u0027d suggest that we make it explicit that it\u0027s a node resource that\u0027s being locked. We may want to extend the locking interface to support locking other resources in future, such as ports, volumes, or attributes such as the power state of a node.\n\nThis could be done in a few ways, such as adding an additional argument that describes the resource to lock, or adding a new set of methods for each resource (is_node_locked etc.), or a separate subobject, a little like the clients (e.g. provider.nodes.is_locked()). The latter could easily allow for a DRY design.\n\nAlso, should be task_or_id?","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"d46b5430a2be2296ab1b7d2daa1504c1dbe40737","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    \"\"\"Base class for DHCP provider APIs.\"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    @abc.abstractmethod"},{"line_number":27,"context_line":"    def is_locked(self, task_or_uuid):"},{"line_number":28,"context_line":"        \"\"\"Return string if node in the `task` is locked."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"        :param task: A TaskManager instance."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_8c6d9f33","line":27,"in_reply_to":"3f79a3b5_396dbc02","updated":"2018-09-06 08:32:43.000000000","message":"It would only require extra BMC requests if the state is observed to change.\n\nIf we want a consistent power state (which I think we do), then we must be holding the (same) lock when querying the power state from the BMC, and updating the node. Otherwise we\u0027ll get races, and see \u0027switch bounce\u0027 type behaviour. Of course the sync will always make it correct in the long term, but that could be 2-3 minutes away.\n\nExample: Node starts in power off. t1 is a periodic power sync, t2 is an API call to set the power state to on.\n\nt1: READ power off\nt2: API set power on\nt2: SET \u0026 WAIT power on\nt2: LOCK\nt2: Update node power on\nt2: UNLOCK\nt1: LOCK\nt1: Update node power off\nt1: UNLOCK\n\nSo the node ends up briefly on then off again in the DB, but the node is on. The sync will run again eventually and sort this out, but an API observer will see power state transitions that did not happen.","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"b3cfe4d0ee2d1250f3d3d239845f9ee7cd4ccfb2","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    \"\"\"Base class for DHCP provider APIs.\"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    @abc.abstractmethod"},{"line_number":27,"context_line":"    def is_locked(self, task_or_uuid):"},{"line_number":28,"context_line":"        \"\"\"Return string if node in the `task` is locked."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"        :param task: A TaskManager instance."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_a2b73721","line":27,"in_reply_to":"3f79a3b5_4200a301","updated":"2018-09-06 18:57:38.000000000","message":"Except that t1 is trying to sync the state to off, and t2 has set it to on, so t1 will see a difference and set it to off.","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"71ea96e609c1f6d8784e5ed925a4f6610b559b19","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    \"\"\"Base class for DHCP provider APIs.\"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    @abc.abstractmethod"},{"line_number":27,"context_line":"    def is_locked(self, task_or_uuid):"},{"line_number":28,"context_line":"        \"\"\"Return string if node in the `task` is locked."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"        :param task: A TaskManager instance."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_a8517c32","line":27,"in_reply_to":"3f79a3b5_51097a28","updated":"2018-09-05 14:57:37.000000000","message":"I completely agree, but I also think we need to approach this one step at a time. I feel like step 1 is improving the overall logic for how we handle locking with periodic tasks. Perhaps the key for checking power state is to actually not lock until we recognize we need to change data about the node. Or Maybe this is the difference that we should track, between shared lock and non-shared lock... I feel like we\u0027re in the territory of another patch set for that.\n\nI guess we just under-use upgrade_lock on the task manager...","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"2e64021dc92aa0e27106bd421b6a6943a29041e7","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    \"\"\"Base class for DHCP provider APIs.\"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    @abc.abstractmethod"},{"line_number":27,"context_line":"    def is_locked(self, task_or_uuid):"},{"line_number":28,"context_line":"        \"\"\"Return string if node in the `task` is locked."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"        :param task: A TaskManager instance."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_d2105303","line":27,"in_reply_to":"3f79a3b5_518c4f34","updated":"2018-09-03 13:43:17.000000000","message":"++ Yeah, this all needs to be cleaned up. I started down one path and had to change along the way because of the way some of the conductor code works.","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"986adefd1df99054bf8f2c94db3d9f801d3e202c","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    \"\"\"Base class for DHCP provider APIs.\"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    @abc.abstractmethod"},{"line_number":27,"context_line":"    def is_locked(self, task_or_uuid):"},{"line_number":28,"context_line":"        \"\"\"Return string if node in the `task` is locked."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"        :param task: A TaskManager instance."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_51097a28","line":27,"in_reply_to":"3f79a3b5_5249a3b4","updated":"2018-09-05 07:32:23.000000000","message":"It depends on what the underlying issues are. If it\u0027s simply the overhead of acquiring and releasing locks, then a lock manager could help. If there is contention for locks, then the only way to improve matters is to use more fine-grained locking. The classic example is power state update or sync, which can take some time, and cause other requests to 409.","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1b0cb0383a5ef78e642c66f70c8a268579a400ef","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    \"\"\"Base class for DHCP provider APIs.\"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    @abc.abstractmethod"},{"line_number":27,"context_line":"    def is_locked(self, task_or_uuid):"},{"line_number":28,"context_line":"        \"\"\"Return string if node in the `task` is locked."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"        :param task: A TaskManager instance."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_4200a301","line":27,"in_reply_to":"3f79a3b5_8c6d9f33","updated":"2018-09-06 18:41:20.000000000","message":"So there is one difference in that the node is refreshed upon the lock and the other lock is not supposed to release until state is achieved... so we should end up in a situation where t2 releases its lock, t1 upgrades task to a lock, the code should check if t1\u0027s node now has the desired end state, if so, it can just release the lock and move on.","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"0200e238fa80cf9d4345031cde6053b98f4b20d2","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    \"\"\"Base class for DHCP provider APIs.\"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    @abc.abstractmethod"},{"line_number":27,"context_line":"    def is_locked(self, task_or_uuid):"},{"line_number":28,"context_line":"        \"\"\"Return string if node in the `task` is locked."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"        :param task: A TaskManager instance."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_c3b05943","line":27,"in_reply_to":"3f79a3b5_a8517c32","updated":"2018-09-05 15:01:57.000000000","message":"Yeah I\u0027m not suggesting we do it all in one strike. But if we\u0027re adding in a new interface it helps to know what it might be required to do in future.\n\nWe need to be careful about late locking - if we didn\u0027t have the lock when we queried the power state, then updating the node with that power state could lead to inconsistencies. I guess we could grab the lock if we detect a change, then query the power state again with it held.","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"7e7de9f4cc451e11510edd332d7501180f80b53a","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    \"\"\"Base class for DHCP provider APIs.\"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    @abc.abstractmethod"},{"line_number":27,"context_line":"    def is_locked(self, task_or_uuid):"},{"line_number":28,"context_line":"        \"\"\"Return string if node in the `task` is locked."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"        :param task: A TaskManager instance."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_396dbc02","line":27,"in_reply_to":"3f79a3b5_c3b05943","updated":"2018-09-05 16:42:09.000000000","message":"And doing so in the ipmi driver specifically would lead to increased load and super grumpy bmcs. It occurs to me that the idea of multiple distinct worker processes kind of raises this further. So on a plus side at least with the power state example, if we are iterating through a list queried say 2-3 minutes ago, and a request snuck in to change that state, the lock upgrade will reveal a refreshed object. If observed state !\u003d desired state, then I think it makes sense to act.","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d641b3319a53a7a674b395273c8e489307da2bfe","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    \"\"\"Base class for DHCP provider APIs.\"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    @abc.abstractmethod"},{"line_number":27,"context_line":"    def is_locked(self, task_or_uuid):"},{"line_number":28,"context_line":"        \"\"\"Return string if node in the `task` is locked."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"        :param task: A TaskManager instance."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_5249a3b4","line":27,"in_reply_to":"3f79a3b5_d2105303","updated":"2018-09-04 15:27:17.000000000","message":"In thinking about this, I think there was some agreement that it would be a little silly to try and do resource level because to lock a node we would then lock all of the ports, so one database interaction becomes 2 or 8... which seems less than ideal. I think node level locking is good for now, and we can kind of go there. I\u0027ll likely rename the method before I\u0027m done.","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"9c7dcbb252ce602cb788d325fefc6b717882d99d","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    def is_locked(self, task_or_uuid):"},{"line_number":28,"context_line":"        \"\"\"Return string if node in the `task` is locked."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"        :param task: A TaskManager instance."},{"line_number":31,"context_line":"        :returns: String representing the lock holder or"},{"line_number":32,"context_line":"                  None."},{"line_number":33,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_f1823b00","line":30,"updated":"2018-09-02 07:53:57.000000000","message":"task_or_id?","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"9c7dcbb252ce602cb788d325fefc6b717882d99d","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"        :param task: A TaskManager instance."},{"line_number":31,"context_line":"        :returns: String representing the lock holder or"},{"line_number":32,"context_line":"                  None."},{"line_number":33,"context_line":"        \"\"\""},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    @abc.abstractmethod"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_d177ff3a","line":32,"updated":"2018-09-02 07:53:57.000000000","message":"The name suggests a boolean return value. How about get_locker or get_lock_owner?","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d641b3319a53a7a674b395273c8e489307da2bfe","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"        :param task: A TaskManager instance."},{"line_number":31,"context_line":"        :returns: String representing the lock holder or"},{"line_number":32,"context_line":"                  None."},{"line_number":33,"context_line":"        \"\"\""},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    @abc.abstractmethod"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_b2cfd727","line":32,"in_reply_to":"3f79a3b5_d177ff3a","updated":"2018-09-04 15:27:17.000000000","message":"I like get_lock_owner!","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"3ba1baf8ff19ff9a0a95e46c1371a8cca220da7d","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"        This code may be used at multiple different times during the"},{"line_number":31,"context_line":"        interaction with a node, such as when creationg the API node"},{"line_number":32,"context_line":"        object, we need to inform the reader of the stauts of the node."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"        :param task: A TaskManager instance."},{"line_number":35,"context_line":"        :param context: Optional context"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_156cdc9c","line":32,"range":{"start_line":32,"start_character":52,"end_line":32,"end_character":58},"updated":"2018-10-31 08:28:25.000000000","message":"typo","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"3ba1baf8ff19ff9a0a95e46c1371a8cca220da7d","unresolved":false,"context_lines":[{"line_number":31,"context_line":"        interaction with a node, such as when creationg the API node"},{"line_number":32,"context_line":"        object, we need to inform the reader of the stauts of the node."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"        :param task: A TaskManager instance."},{"line_number":35,"context_line":"        :param context: Optional context"},{"line_number":36,"context_line":"        :returns: String representing the lock holder or"},{"line_number":37,"context_line":"                  None."}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_55a334b1","line":34,"range":{"start_line":34,"start_character":15,"end_line":34,"end_character":19},"updated":"2018-10-31 08:28:25.000000000","message":"task_or_id\n\nalso I think this is coupled with the db implementation, I would prefer a task here, db provider can retrieve integer id from task.node.id by itself.","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d967b2004f3b2c8300171698292cff2d72a14035","unresolved":false,"context_lines":[{"line_number":31,"context_line":"        interaction with a node, such as when creationg the API node"},{"line_number":32,"context_line":"        object, we need to inform the reader of the stauts of the node."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"        :param task: A TaskManager instance."},{"line_number":35,"context_line":"        :param context: Optional context"},{"line_number":36,"context_line":"        :returns: String representing the lock holder or"},{"line_number":37,"context_line":"                  None."}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_9634a8fa","line":34,"range":{"start_line":34,"start_character":15,"end_line":34,"end_character":19},"in_reply_to":"3f79a3b5_55a334b1","updated":"2018-11-28 16:24:04.000000000","message":"The ability to perform an ID driven lookup is because we might not have a shared task when we need to check the status of a lock. Similar to how we presently use db filters.","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"3ba1baf8ff19ff9a0a95e46c1371a8cca220da7d","unresolved":false,"context_lines":[{"line_number":45,"context_line":"                        the operation."},{"line_number":46,"context_line":"        :param host: Conductor hostname or label tag to"},{"line_number":47,"context_line":"                     present the lock as being held by."},{"line_number":48,"context_line":"        :param node_id: Database node ID representing the"},{"line_number":49,"context_line":"                        node to be locked."},{"line_number":50,"context_line":"        :param purpose: Purpose of the lock."},{"line_number":51,"context_line":"        :returns: A node object representing a node with a locking"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_15dffc39","line":48,"range":{"start_line":48,"start_character":15,"end_line":48,"end_character":22},"updated":"2018-10-31 08:28:25.000000000","message":"It\u0027s a row id or uuid? if this gonna be a base class, I think we shouldn\u0027t have db specific information here.","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d967b2004f3b2c8300171698292cff2d72a14035","unresolved":false,"context_lines":[{"line_number":45,"context_line":"                        the operation."},{"line_number":46,"context_line":"        :param host: Conductor hostname or label tag to"},{"line_number":47,"context_line":"                     present the lock as being held by."},{"line_number":48,"context_line":"        :param node_id: Database node ID representing the"},{"line_number":49,"context_line":"                        node to be locked."},{"line_number":50,"context_line":"        :param purpose: Purpose of the lock."},{"line_number":51,"context_line":"        :returns: A node object representing a node with a locking"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_b9951796","line":48,"range":{"start_line":48,"start_character":15,"end_line":48,"end_character":22},"in_reply_to":"3f79a3b5_15dffc39","updated":"2018-11-28 16:24:04.000000000","message":"The database oriented ID as described, not uuid. It seemed like the node id made more sense because we always had it as opposed to having to always reach into the database to perform the lookup. See ironic/locking/database.py line 53","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"3ba1baf8ff19ff9a0a95e46c1371a8cca220da7d","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        \"\"\""},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    @abc.abstractmethod"},{"line_number":57,"context_line":"    def unlock(self, context, conductor_hostname, node_id):"},{"line_number":58,"context_line":"        \"\"\"Removes the lock for the node provided in the `task`."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        :param context: Context in which to perform"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_9526ac6a","line":57,"range":{"start_line":57,"start_character":30,"end_line":57,"end_character":48},"updated":"2018-10-31 08:28:25.000000000","message":"s/conductor_hostname/host/","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d967b2004f3b2c8300171698292cff2d72a14035","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        \"\"\""},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    @abc.abstractmethod"},{"line_number":57,"context_line":"    def unlock(self, context, conductor_hostname, node_id):"},{"line_number":58,"context_line":"        \"\"\"Removes the lock for the node provided in the `task`."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        :param context: Context in which to perform"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_f9870f5b","line":57,"range":{"start_line":57,"start_character":30,"end_line":57,"end_character":48},"in_reply_to":"3f79a3b5_9526ac6a","updated":"2018-11-28 16:24:04.000000000","message":"Done","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"3ba1baf8ff19ff9a0a95e46c1371a8cca220da7d","unresolved":false,"context_lines":[{"line_number":79,"context_line":"        \"\"\"Return a dictionary describing locks for the list of conductors."},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"        In order to reconcile locking and recover from failures, we need"},{"line_number":82,"context_line":"        to obtain the curent state on the perception of locks. The only way"},{"line_number":83,"context_line":"        to do this is to reconcile the current states of the locks against"},{"line_number":84,"context_line":"        what we know, which in many cases will be that a conductor is no"},{"line_number":85,"context_line":"        longer operating."}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_151a9c18","line":82,"range":{"start_line":82,"start_character":22,"end_line":82,"end_character":28},"updated":"2018-10-31 08:28:25.000000000","message":"typo","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d967b2004f3b2c8300171698292cff2d72a14035","unresolved":false,"context_lines":[{"line_number":79,"context_line":"        \"\"\"Return a dictionary describing locks for the list of conductors."},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"        In order to reconcile locking and recover from failures, we need"},{"line_number":82,"context_line":"        to obtain the curent state on the perception of locks. The only way"},{"line_number":83,"context_line":"        to do this is to reconcile the current states of the locks against"},{"line_number":84,"context_line":"        what we know, which in many cases will be that a conductor is no"},{"line_number":85,"context_line":"        longer operating."}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_1983eb50","line":82,"range":{"start_line":82,"start_character":22,"end_line":82,"end_character":28},"in_reply_to":"3f79a3b5_151a9c18","updated":"2018-11-28 16:24:04.000000000","message":"Done","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"3ba1baf8ff19ff9a0a95e46c1371a8cca220da7d","unresolved":false,"context_lines":[{"line_number":88,"context_line":"                          hostnames."},{"line_number":89,"context_line":"        :returns: Dictonary of key value pairs, with a key name"},{"line_number":90,"context_line":"                  consisting of the conductor hostname, and a list"},{"line_number":91,"context_line":"                  of ID values representing each node. Defaults to an"},{"line_number":92,"context_line":"                  empty dictionary if no locks are found."},{"line_number":93,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_d5e8a4e1","line":91,"range":{"start_line":91,"start_character":21,"end_line":91,"end_character":23},"updated":"2018-10-31 08:28:25.000000000","message":"UUID if comment at L48 is valid.","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"6976d810b13b41b67581e02eeca3c26c1d43a055","unresolved":false,"context_lines":[{"line_number":50,"context_line":"        :returns: A node object representing a node with a locking"},{"line_number":51,"context_line":"                  reservation present."},{"line_number":52,"context_line":"        :raises: NodeLocked if node is already locked."},{"line_number":53,"context_line":"        \"\"\""},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    @abc.abstractmethod"},{"line_number":56,"context_line":"    def unlock(self, context, host, node_uuid):"}],"source_content_type":"text/x-python","patch_set":20,"id":"3fce034c_85762606","line":53,"updated":"2019-04-16 06:43:47.000000000","message":"nit: NodeNotFound could be raised as well.","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"6976d810b13b41b67581e02eeca3c26c1d43a055","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        \"\"\""},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    @abc.abstractmethod"},{"line_number":69,"context_line":"    def clear_reservations_for_conductor(self, host):"},{"line_number":70,"context_line":"        \"\"\"Removes all locks for the conductor."},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        :param host: host name representing the lock holder."}],"source_content_type":"text/x-python","patch_set":20,"id":"3fce034c_854f06bc","line":69,"range":{"start_line":69,"start_character":14,"end_line":69,"end_character":26},"updated":"2019-04-16 06:43:47.000000000","message":"It\u0027s a bit oriented to current implementation, as a base layer, I think it would be better to say clear_locks_for_conductor\n\nJust thinking if we have an etcd driver, I guess the host argument might be just ignored.\n\nI am wondering how it would be implemented in case of etcd locking driver, it appears tooz concealed some information to support interface defined in the base layer which affects whether it\u0027s possible (or efficient) to do, at least I feel a sense of danger here.\n\nE.g., we need a coordinator instance to get all locks held by the coordinator (which I thought identical to a conductor in our case). Nevertheless to say it\u0027s an internal attribute _acquired_locks.","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"03def507e4ad023edbd7286681ddac1eaec12047","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        \"\"\""},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    @abc.abstractmethod"},{"line_number":69,"context_line":"    def clear_reservations_for_conductor(self, host):"},{"line_number":70,"context_line":"        \"\"\"Removes all locks for the conductor."},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        :param host: host name representing the lock holder."}],"source_content_type":"text/x-python","patch_set":20,"id":"bfb3d3c7_b307ed1c","line":69,"range":{"start_line":69,"start_character":14,"end_line":69,"end_character":26},"in_reply_to":"3fce034c_854f06bc","updated":"2019-05-22 18:58:09.000000000","message":"Its a distinct possibility, and we may just have to enhance tooz or directly use etcd since we still need to be able to invalidate locks. thinking about it, I\u0027m not sure we can track an internal attribute and have it available for use unless we\u0027re storing it someplace that can survive a process restart.","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"6976d810b13b41b67581e02eeca3c26c1d43a055","unresolved":false,"context_lines":[{"line_number":75,"context_line":"    @abc.abstractmethod"},{"line_number":76,"context_line":"    def get_locks_for_conductors(self, host_list):"},{"line_number":77,"context_line":"        \"\"\"Return a dictionary describing locks for the list of conductors."},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        In order to reconcile locking and recover from failures, we need"},{"line_number":80,"context_line":"        to obtain the current state on the perception of locks. The only way"},{"line_number":81,"context_line":"        to do this is to reconcile the current states of the locks against"}],"source_content_type":"text/x-python","patch_set":20,"id":"3fce034c_50beba65","line":78,"updated":"2019-04-16 06:43:47.000000000","message":"If we assume a locking instance is on per-conductor basis, then I think this interface appears like a helper around the base locking layer.\n\nSo what I am actually concerned is whether the interface definition copes well with other locking mechanisms, because in current form I feel it\u0027s based on the assumption on how our db locking is implemented.","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"03def507e4ad023edbd7286681ddac1eaec12047","unresolved":false,"context_lines":[{"line_number":75,"context_line":"    @abc.abstractmethod"},{"line_number":76,"context_line":"    def get_locks_for_conductors(self, host_list):"},{"line_number":77,"context_line":"        \"\"\"Return a dictionary describing locks for the list of conductors."},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        In order to reconcile locking and recover from failures, we need"},{"line_number":80,"context_line":"        to obtain the current state on the perception of locks. The only way"},{"line_number":81,"context_line":"        to do this is to reconcile the current states of the locks against"}],"source_content_type":"text/x-python","patch_set":20,"id":"bfb3d3c7_f35fa53e","line":78,"in_reply_to":"3fce034c_50beba65","updated":"2019-05-22 18:58:09.000000000","message":"It entirely is based upon that, but I don\u0027t believe there is any reason to limit it to that.","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"f2c830e863d07f2cb3b5477da9c06945b1f3eaf1","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        \"\"\""},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    @abc.abstractmethod"},{"line_number":95,"context_line":"    def update_node(self, node):"},{"line_number":96,"context_line":"        \"\"\"Update the node object to represent current lock state.\"\"\""},{"line_number":97,"context_line":"        node.reservation \u003d self.get_lock_owner(node.uuid)"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_17514037","line":95,"updated":"2019-06-04 04:09:39.000000000","message":"I feel it\u0027s strange to have this interface in the locking api, and the update_node from DatabaseLocking overrides this one, which means it never executed..","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"e5c75bba02f8156a0c3b3a8bed2aa268d5cdca7c","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        \"\"\""},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    @abc.abstractmethod"},{"line_number":95,"context_line":"    def update_node(self, node):"},{"line_number":96,"context_line":"        \"\"\"Update the node object to represent current lock state.\"\"\""},{"line_number":97,"context_line":"        node.reservation \u003d self.get_lock_owner(node.uuid)"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_ad6312b1","line":95,"updated":"2019-06-06 09:22:25.000000000","message":"Yeah, I think we need either abstractmethod or the body, having both looks weird (and I\u0027m not sure you can call such method even via super).","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"258f1757dfcc22ca1e9ace62c297d8012a2660e5","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        \"\"\""},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    @abc.abstractmethod"},{"line_number":95,"context_line":"    def update_node(self, node):"},{"line_number":96,"context_line":"        \"\"\"Update the node object to represent current lock state.\"\"\""},{"line_number":97,"context_line":"        node.reservation \u003d self.get_lock_owner(node.uuid)"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_e9e4b1ae","line":95,"in_reply_to":"9fb8cfa7_17514037","updated":"2019-06-05 13:45:36.000000000","message":"This is essentially what any interface besides database would do. It feels strange too, but it makes sense to at least put someplace for now until we write the etcd driver.","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1e32917fd7133f375594552a0cc84d18bb2c999e","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        \"\"\""},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    @abc.abstractmethod"},{"line_number":95,"context_line":"    def update_node(self, node):"},{"line_number":96,"context_line":"        \"\"\"Update the node object to represent current lock state.\"\"\""},{"line_number":97,"context_line":"        node.reservation \u003d self.get_lock_owner(node.uuid)"}],"source_content_type":"text/x-python","patch_set":23,"id":"3fa7e38b_3750acd5","line":95,"in_reply_to":"9fb8cfa7_ad6312b1","updated":"2019-12-23 23:34:28.000000000","message":"I think you can, but anyway removed the decorator.","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"fe24e4534b4287c6dd5c332754cff97fcee33293","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        \"\"\""},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    @abc.abstractmethod"},{"line_number":95,"context_line":"    def update_node(self, node):"},{"line_number":96,"context_line":"        \"\"\"Update the node object to represent current lock state.\"\"\""},{"line_number":97,"context_line":"        node.reservation \u003d self.get_lock_owner(node.uuid)"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_ea4a5525","line":95,"in_reply_to":"9fb8cfa7_e9e4b1ae","updated":"2019-06-06 01:32:59.000000000","message":"Well, I am not sure what it looks like when we have etcd driver, but the -1 is mainly because L97 is not required. update_node is an abstract method, an etcd driver has to override, this line never gets executed.\nOtherwise, we could remove the abstractmethod decorator to have a default implementation (I guess that\u0027s what you intend to do?)","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"}],"ironic/locking/database.py":[{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"9c7dcbb252ce602cb788d325fefc6b717882d99d","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        # modular without substantial changes."},{"line_number":40,"context_line":"        if isinstance(task_or_id, int):"},{"line_number":41,"context_line":"            if not context:"},{"line_number":42,"context_line":"                context \u003d self.context"},{"line_number":43,"context_line":"            ir_node \u003d objects.Node.get_by_id(context, task_or_id)"},{"line_number":44,"context_line":"            return ir_node.reservation"},{"line_number":45,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_71762b41","line":42,"updated":"2018-09-02 07:53:57.000000000","message":"I don\u0027t see where self.context is set.","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"2e64021dc92aa0e27106bd421b6a6943a29041e7","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        # modular without substantial changes."},{"line_number":40,"context_line":"        if isinstance(task_or_id, int):"},{"line_number":41,"context_line":"            if not context:"},{"line_number":42,"context_line":"                context \u003d self.context"},{"line_number":43,"context_line":"            ir_node \u003d objects.Node.get_by_id(context, task_or_id)"},{"line_number":44,"context_line":"            return ir_node.reservation"},{"line_number":45,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_321ae716","line":42,"in_reply_to":"3f79a3b5_71762b41","updated":"2018-09-03 13:43:17.000000000","message":"I was thinking from the parent, that is not reality.  I don\u0027t think we really need it, and the case where we pass in none is deep inside of failover logic I think.","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"9c7dcbb252ce602cb788d325fefc6b717882d99d","unresolved":false,"context_lines":[{"line_number":69,"context_line":"    def clear_reservations_for_conductor(self, host):"},{"line_number":70,"context_line":"        \"\"\"Removes all locks for the conductor.\"\"\""},{"line_number":71,"context_line":"        if not host:"},{"line_number":72,"context_line":"            host \u003d self.host"},{"line_number":73,"context_line":"        db \u003d dbapi.get_instance()"},{"line_number":74,"context_line":"        db.clear_node_reservations_for_conductor(host)"},{"line_number":75,"context_line":"        # Explicitly delete the instantiation"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_91718747","line":72,"updated":"2018-09-02 07:53:57.000000000","message":"I don\u0027t see where self.host is set.","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d823892b5a433c6adf1e9c293bf5f1e0fce35c13","unresolved":false,"context_lines":[{"line_number":121,"context_line":"                    if host not in lock_list:"},{"line_number":122,"context_line":"                        lock_list[host] \u003d []"},{"line_number":123,"context_line":"                    lock_list[host].append(node_id)"},{"line_number":124,"context_line":"        del db"},{"line_number":125,"context_line":"        return lock_list"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_0564eb20","line":124,"updated":"2018-09-07 08:21:33.000000000","message":"unneeded","commit_id":"00fe89b2172b65d84da07acb29486fb9bec267cb"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d51a499cfe1c47ba7c5616e4b6eb43e95cc1bd77","unresolved":false,"context_lines":[{"line_number":121,"context_line":"                    if host not in lock_list:"},{"line_number":122,"context_line":"                        lock_list[host] \u003d []"},{"line_number":123,"context_line":"                    lock_list[host].append(node_id)"},{"line_number":124,"context_line":"        del db"},{"line_number":125,"context_line":"        return lock_list"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_8f5b3452","line":124,"in_reply_to":"3f79a3b5_0564eb20","updated":"2018-09-07 13:52:47.000000000","message":"this is totally 1000% paranoia regarding GC","commit_id":"00fe89b2172b65d84da07acb29486fb9bec267cb"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"3ba1baf8ff19ff9a0a95e46c1371a8cca220da7d","unresolved":false,"context_lines":[{"line_number":91,"context_line":"        db \u003d dbapi.get_instance()"},{"line_number":92,"context_line":"        db.clear_node_reservations_for_conductor(host)"},{"line_number":93,"context_line":"        # Explicitly delete the instantiation"},{"line_number":94,"context_line":"        del db"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    def get_locks_for_conductors(self, host_list):"},{"line_number":97,"context_line":"        \"\"\"Return a dictionary describing locks for the list of conductors."}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_a0b048e1","line":94,"range":{"start_line":94,"start_character":8,"end_line":94,"end_character":14},"updated":"2018-10-31 08:28:25.000000000","message":"unneeded?","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d967b2004f3b2c8300171698292cff2d72a14035","unresolved":false,"context_lines":[{"line_number":91,"context_line":"        db \u003d dbapi.get_instance()"},{"line_number":92,"context_line":"        db.clear_node_reservations_for_conductor(host)"},{"line_number":93,"context_line":"        # Explicitly delete the instantiation"},{"line_number":94,"context_line":"        del db"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    def get_locks_for_conductors(self, host_list):"},{"line_number":97,"context_line":"        \"\"\"Return a dictionary describing locks for the list of conductors."}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_9963b0ac","line":94,"range":{"start_line":94,"start_character":8,"end_line":94,"end_character":14},"in_reply_to":"3f79a3b5_a0b048e1","updated":"2018-11-28 16:24:04.000000000","message":"my paranoia, will remove.","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"cb69cb54288d3cf72d3b3403484cfa205600da24","unresolved":false,"context_lines":[{"line_number":26,"context_line":"class DatabaseLocking(base.BaseLocking):"},{"line_number":27,"context_line":"    \"\"\"Wrapper database locking.\"\"\""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    def get_lock_owner(self, task_or_id, context\u003dNone):"},{"line_number":30,"context_line":"        \"\"\"Return string representing the lock holder for the node."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"        This code may be used at multiple different times during the"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_ad5fd176","line":29,"range":{"start_line":29,"start_character":29,"end_line":29,"end_character":39},"updated":"2018-12-13 08:37:05.000000000","message":"this is really a versatile field, it can be an uuid, an int, or a task..\ni would prefer not using this as an prototype :)","commit_id":"c601696b7d5a8b6ac3704f127dc335bf85c56b99"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"296749680efbad88ff4ced78b26eb8217b4b8455","unresolved":false,"context_lines":[{"line_number":26,"context_line":"class DatabaseLocking(base.BaseLocking):"},{"line_number":27,"context_line":"    \"\"\"Wrapper database locking.\"\"\""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    def get_lock_owner(self, task_or_id, context\u003dNone):"},{"line_number":30,"context_line":"        \"\"\"Return string representing the lock holder for the node."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"        This code may be used at multiple different times during the"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_4dfcf373","line":29,"range":{"start_line":29,"start_character":29,"end_line":29,"end_character":39},"in_reply_to":"3f79a3b5_ad5fd176","updated":"2018-12-13 15:28:02.000000000","message":"Yeah, node_uuid doesn\u0027t seem horrible. We would need to add it to the periodic queries that don\u0027t already collect that up.","commit_id":"c601696b7d5a8b6ac3704f127dc335bf85c56b99"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"41922af381f3acbec5233a04dcb769565a8a12ff","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        # interaction because we\u0027re accessing it an additional time or"},{"line_number":44,"context_line":"        # two, but there is likely no way around this in order to make"},{"line_number":45,"context_line":"        # locking modular without substantial changes."},{"line_number":46,"context_line":"        print(task_or_id)"},{"line_number":47,"context_line":"        if (isinstance(task_or_id, six.string_types)"},{"line_number":48,"context_line":"            or isinstance(task_or_id, int)):"},{"line_number":49,"context_line":"            ir_node \u003d objects.Node.get(context, task_or_id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_9e60efa0","line":46,"updated":"2018-12-13 17:07:26.000000000","message":"nit: remove in the final version (or replace with proper logging)","commit_id":"c601696b7d5a8b6ac3704f127dc335bf85c56b99"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1cb803a11e87b58496b6d4b9a5b3d3e8e6a420c0","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        # interaction because we\u0027re accessing it an additional time or"},{"line_number":44,"context_line":"        # two, but there is likely no way around this in order to make"},{"line_number":45,"context_line":"        # locking modular without substantial changes."},{"line_number":46,"context_line":"        print(task_or_id)"},{"line_number":47,"context_line":"        if (isinstance(task_or_id, six.string_types)"},{"line_number":48,"context_line":"            or isinstance(task_or_id, int)):"},{"line_number":49,"context_line":"            ir_node \u003d objects.Node.get(context, task_or_id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_45c27f03","line":46,"in_reply_to":"3f79a3b5_9e60efa0","updated":"2018-12-14 01:07:20.000000000","message":"Done","commit_id":"c601696b7d5a8b6ac3704f127dc335bf85c56b99"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"365eef31e0a1f1f5967601c5226b3458c7116a04","unresolved":false,"context_lines":[{"line_number":97,"context_line":"                  of UUID values representing each node. Defaults to an"},{"line_number":98,"context_line":"                  empty dictionary if no locks are found."},{"line_number":99,"context_line":"        \"\"\""},{"line_number":100,"context_line":"        if not isinstance(host_list, list):"},{"line_number":101,"context_line":"            host_list \u003d [host_list]"},{"line_number":102,"context_line":"        lock_list \u003d {}"},{"line_number":103,"context_line":"        db \u003d dbapi.get_instance()"}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_29dabce8","line":100,"range":{"start_line":100,"start_character":0,"end_line":100,"end_character":43},"updated":"2019-02-07 18:22:14.000000000","message":"Do we really need this type-based condition? Feels quite rigid and not overly ducky in the sense that it can\u0027t be a list-like object or even a tuple.","commit_id":"468d436bd3c7e82888b60748d073ea09588908e0"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d8c0d6bc2c72efd851816d11056a389ba063364f","unresolved":false,"context_lines":[{"line_number":97,"context_line":"                  of UUID values representing each node. Defaults to an"},{"line_number":98,"context_line":"                  empty dictionary if no locks are found."},{"line_number":99,"context_line":"        \"\"\""},{"line_number":100,"context_line":"        if not isinstance(host_list, list):"},{"line_number":101,"context_line":"            host_list \u003d [host_list]"},{"line_number":102,"context_line":"        lock_list \u003d {}"},{"line_number":103,"context_line":"        db \u003d dbapi.get_instance()"}],"source_content_type":"text/x-python","patch_set":18,"id":"5fc1f717_e811ea05","line":100,"range":{"start_line":100,"start_character":0,"end_line":100,"end_character":43},"in_reply_to":"9fdfeff1_29dabce8","updated":"2019-03-22 21:41:40.000000000","message":"The thought was to allow a single string value to be checked. ... but hey it should always be a list of hosts \\o/","commit_id":"468d436bd3c7e82888b60748d073ea09588908e0"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"365eef31e0a1f1f5967601c5226b3458c7116a04","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        \"\"\""},{"line_number":100,"context_line":"        if not isinstance(host_list, list):"},{"line_number":101,"context_line":"            host_list \u003d [host_list]"},{"line_number":102,"context_line":"        lock_list \u003d {}"},{"line_number":103,"context_line":"        db \u003d dbapi.get_instance()"},{"line_number":104,"context_line":"        node_list \u003d db.get_nodeinfo_list("},{"line_number":105,"context_line":"            filters\u003d{\u0027reserved_by_any_of\u0027: host_list},"}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_c99770eb","line":102,"range":{"start_line":102,"start_character":20,"end_line":102,"end_character":22},"updated":"2019-02-07 18:22:14.000000000","message":"can it be a `defaultdict(list)`?","commit_id":"468d436bd3c7e82888b60748d073ea09588908e0"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"365eef31e0a1f1f5967601c5226b3458c7116a04","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        \"\"\""},{"line_number":100,"context_line":"        if not isinstance(host_list, list):"},{"line_number":101,"context_line":"            host_list \u003d [host_list]"},{"line_number":102,"context_line":"        lock_list \u003d {}"},{"line_number":103,"context_line":"        db \u003d dbapi.get_instance()"},{"line_number":104,"context_line":"        node_list \u003d db.get_nodeinfo_list("},{"line_number":105,"context_line":"            filters\u003d{\u0027reserved_by_any_of\u0027: host_list},"}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_69b76488","line":102,"range":{"start_line":102,"start_character":8,"end_line":102,"end_character":17},"updated":"2019-02-07 18:22:14.000000000","message":"nit: it\u0027s not really a \"list\", but a dict of lists","commit_id":"468d436bd3c7e82888b60748d073ea09588908e0"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"36d3b2fab689b38939089f158a7f55ae13b0c1f2","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        # NOTE(TheJulia): This is likely less ideal in terms of database"},{"line_number":41,"context_line":"        # interaction because we\u0027re accessing it an additional time or"},{"line_number":42,"context_line":"        # two, but there is likely no way around this in order to make"},{"line_number":43,"context_line":"        # locking modular without substantial changes."},{"line_number":44,"context_line":"        ir_node \u003d objects.Node.get_by_uuid(context, str(node_uuid))"},{"line_number":45,"context_line":"        return ir_node.reservation"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"ffb9cba7_31df440e","line":43,"updated":"2019-04-29 04:04:16.000000000","message":"Well, in many places we have a node objects, so I\u0027d prefer this split into two calls.","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"03def507e4ad023edbd7286681ddac1eaec12047","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        # NOTE(TheJulia): This is likely less ideal in terms of database"},{"line_number":41,"context_line":"        # interaction because we\u0027re accessing it an additional time or"},{"line_number":42,"context_line":"        # two, but there is likely no way around this in order to make"},{"line_number":43,"context_line":"        # locking modular without substantial changes."},{"line_number":44,"context_line":"        ir_node \u003d objects.Node.get_by_uuid(context, str(node_uuid))"},{"line_number":45,"context_line":"        return ir_node.reservation"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"bfb3d3c7_2e571605","line":43,"in_reply_to":"ffb9cba7_31df440e","updated":"2019-05-22 18:58:09.000000000","message":"Could you elaborate on what your thinking? I guess the alternative is not load the object but directly select data from the column :\\","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"e5c75bba02f8156a0c3b3a8bed2aa268d5cdca7c","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        # NOTE(TheJulia): This is likely less ideal in terms of database"},{"line_number":41,"context_line":"        # interaction because we\u0027re accessing it an additional time or"},{"line_number":42,"context_line":"        # two, but there is likely no way around this in order to make"},{"line_number":43,"context_line":"        # locking modular without substantial changes."},{"line_number":44,"context_line":"        ir_node \u003d objects.Node.get_by_uuid(context, str(node_uuid))"},{"line_number":45,"context_line":"        return ir_node.reservation"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_8d608ea7","line":43,"updated":"2019-06-06 09:22:25.000000000","message":"Actually, you can accept task in this call. So far this seems to be used when we have a task anyway. We can be flexible and do task_or_uuid, distinguising them by their type.","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1e32917fd7133f375594552a0cc84d18bb2c999e","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        # NOTE(TheJulia): This is likely less ideal in terms of database"},{"line_number":41,"context_line":"        # interaction because we\u0027re accessing it an additional time or"},{"line_number":42,"context_line":"        # two, but there is likely no way around this in order to make"},{"line_number":43,"context_line":"        # locking modular without substantial changes."},{"line_number":44,"context_line":"        ir_node \u003d objects.Node.get_by_uuid(context, str(node_uuid))"},{"line_number":45,"context_line":"        return ir_node.reservation"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"3fa7e38b_574be826","line":43,"in_reply_to":"9fb8cfa7_8d608ea7","updated":"2019-12-23 23:34:28.000000000","message":"I can agree to accept a task. I think an early rev did the same actually, but we still need to query the db for the current state, in my opinion.","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"}],"ironic/objects/node.py":[{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"9c7dcbb252ce602cb788d325fefc6b717882d99d","unresolved":false,"context_lines":[{"line_number":30,"context_line":"from ironic.objects import notification"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"# TODO(TheJulia): With properties based scheduling eventually"},{"line_number":33,"context_line":"# being removed from nova, we should drop these as requirements."},{"line_number":34,"context_line":"REQUIRED_INT_PROPERTIES \u003d [\u0027local_gb\u0027, \u0027cpus\u0027, \u0027memory_mb\u0027]"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_f1979b39","line":33,"updated":"2018-09-02 07:53:57.000000000","message":"It\u0027s already possible to not specify them, this just validates they are non-negative ints if present. In fact if you want scheduling to work with mixed BM/VM, you should not specify these, right?","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"2e64021dc92aa0e27106bd421b6a6943a29041e7","unresolved":false,"context_lines":[{"line_number":30,"context_line":"from ironic.objects import notification"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"# TODO(TheJulia): With properties based scheduling eventually"},{"line_number":33,"context_line":"# being removed from nova, we should drop these as requirements."},{"line_number":34,"context_line":"REQUIRED_INT_PROPERTIES \u003d [\u0027local_gb\u0027, \u0027cpus\u0027, \u0027memory_mb\u0027]"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_f2d1cf30","line":33,"in_reply_to":"3f79a3b5_f1979b39","updated":"2018-09-03 13:43:17.000000000","message":"Yeah, I spotted it and was like \"oh, that needs to change, let me put a todo there\"","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"9c7dcbb252ce602cb788d325fefc6b717882d99d","unresolved":false,"context_lines":[{"line_number":203,"context_line":"            # don\u0027t double every database query to refresh the node if"},{"line_number":204,"context_line":"            # we are using a database. That and if this is with the database"},{"line_number":205,"context_line":"            # this kind of becomes recursive."},{"line_number":206,"context_line":"            self.reservation \u003d LOCKING.is_locked(self.uuid)"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"    # NOTE(xek): We don\u0027t want to enable RPC on this call just yet. Remotable"},{"line_number":209,"context_line":"    # methods can be used in the future to replace current explicit RPC calls."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_11e9f7b5","line":206,"updated":"2018-09-02 07:53:57.000000000","message":"For non-DB locking providers the reservation attribute should never be touched, so we shouldn\u0027t need to set it here, right? Is there any way we could make it a bomb that explodes when referenced?","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"2e64021dc92aa0e27106bd421b6a6943a29041e7","unresolved":false,"context_lines":[{"line_number":203,"context_line":"            # don\u0027t double every database query to refresh the node if"},{"line_number":204,"context_line":"            # we are using a database. That and if this is with the database"},{"line_number":205,"context_line":"            # this kind of becomes recursive."},{"line_number":206,"context_line":"            self.reservation \u003d LOCKING.is_locked(self.uuid)"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"    # NOTE(xek): We don\u0027t want to enable RPC on this call just yet. Remotable"},{"line_number":209,"context_line":"    # methods can be used in the future to replace current explicit RPC calls."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_52ea03d8","line":206,"in_reply_to":"3f79a3b5_11e9f7b5","updated":"2018-09-03 13:43:17.000000000","message":"It is returned as an API field.... so... yeah.  :(\n\nI was kind of thinking of turning the object into a bit of a bomb via the model, and then realized that we would horribly break the API since some of the code out there backs off if there is a reservation active on the node, because it is unlikely that the request will go through if there is an active lock on the  node.","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"b63a10adf49215de6b5ecfd2a7ea8bafa2ed387e","unresolved":false,"context_lines":[{"line_number":203,"context_line":"            # don\u0027t double every database query to refresh the node if"},{"line_number":204,"context_line":"            # we are using a database. That and if this is with the database"},{"line_number":205,"context_line":"            # this kind of becomes recursive."},{"line_number":206,"context_line":"            self.reservation \u003d LOCKING.is_locked(self.uuid)"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"    # NOTE(xek): We don\u0027t want to enable RPC on this call just yet. Remotable"},{"line_number":209,"context_line":"    # methods can be used in the future to replace current explicit RPC calls."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_23f29b9b","line":206,"in_reply_to":"3f79a3b5_52ea03d8","updated":"2018-09-03 18:07:40.000000000","message":"Ah, I see. I guess we need to preserve that.","commit_id":"72bffadb6f725a6237b5c692dfb3fcbeaf787978"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d823892b5a433c6adf1e9c293bf5f1e0fce35c13","unresolved":false,"context_lines":[{"line_number":32,"context_line":"# TODO(TheJulia): With properties based scheduling eventually"},{"line_number":33,"context_line":"# being removed from nova, we should drop these as requirements."},{"line_number":34,"context_line":"# This note is totally unrelated to the current change, but this"},{"line_number":35,"context_line":"# just adds clarity and a note to come back to at some point."},{"line_number":36,"context_line":"REQUIRED_INT_PROPERTIES \u003d [\u0027local_gb\u0027, \u0027cpus\u0027, \u0027memory_mb\u0027]"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_85b23b93","line":35,"updated":"2018-09-07 08:21:33.000000000","message":"There will be no \"current change\" when this lands.","commit_id":"00fe89b2172b65d84da07acb29486fb9bec267cb"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d51a499cfe1c47ba7c5616e4b6eb43e95cc1bd77","unresolved":false,"context_lines":[{"line_number":32,"context_line":"# TODO(TheJulia): With properties based scheduling eventually"},{"line_number":33,"context_line":"# being removed from nova, we should drop these as requirements."},{"line_number":34,"context_line":"# This note is totally unrelated to the current change, but this"},{"line_number":35,"context_line":"# just adds clarity and a note to come back to at some point."},{"line_number":36,"context_line":"REQUIRED_INT_PROPERTIES \u003d [\u0027local_gb\u0027, \u0027cpus\u0027, \u0027memory_mb\u0027]"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_4a142a41","line":35,"in_reply_to":"3f79a3b5_85b23b93","updated":"2018-09-07 13:52:47.000000000","message":"I\u0027ll just remove this.","commit_id":"00fe89b2172b65d84da07acb29486fb9bec267cb"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d823892b5a433c6adf1e9c293bf5f1e0fce35c13","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        self.traits.obj_reset_changes()"},{"line_number":203,"context_line":"        if CONF.conductor.locking_provider !\u003d \u0027database\u0027:"},{"line_number":204,"context_line":"            # NOTE(TheJulia): Super special case for database locking so we"},{"line_number":205,"context_line":"            # don\u0027t double every database query to refresh the node if"},{"line_number":206,"context_line":"            # we are using a database. That and if this is with the database"},{"line_number":207,"context_line":"            # this kind of becomes recursive."},{"line_number":208,"context_line":"            self.reservation \u003d LOCKING.get_lock_owner(self.uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_a5adf731","line":205,"updated":"2018-09-07 08:21:33.000000000","message":"I get it, but it\u0027s still bad.. As before, it assumes that there cannot be out-of-tree locking providers.","commit_id":"00fe89b2172b65d84da07acb29486fb9bec267cb"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d51a499cfe1c47ba7c5616e4b6eb43e95cc1bd77","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        self.traits.obj_reset_changes()"},{"line_number":203,"context_line":"        if CONF.conductor.locking_provider !\u003d \u0027database\u0027:"},{"line_number":204,"context_line":"            # NOTE(TheJulia): Super special case for database locking so we"},{"line_number":205,"context_line":"            # don\u0027t double every database query to refresh the node if"},{"line_number":206,"context_line":"            # we are using a database. That and if this is with the database"},{"line_number":207,"context_line":"            # this kind of becomes recursive."},{"line_number":208,"context_line":"            self.reservation \u003d LOCKING.get_lock_owner(self.uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_0a2cf296","line":205,"in_reply_to":"3f79a3b5_a5adf731","updated":"2018-09-07 13:52:47.000000000","message":"That is untrue though, it is if it is anything aside from database, then to call get_lock_owner.","commit_id":"00fe89b2172b65d84da07acb29486fb9bec267cb"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d823892b5a433c6adf1e9c293bf5f1e0fce35c13","unresolved":false,"context_lines":[{"line_number":204,"context_line":"            # NOTE(TheJulia): Super special case for database locking so we"},{"line_number":205,"context_line":"            # don\u0027t double every database query to refresh the node if"},{"line_number":206,"context_line":"            # we are using a database. That and if this is with the database"},{"line_number":207,"context_line":"            # this kind of becomes recursive."},{"line_number":208,"context_line":"            self.reservation \u003d LOCKING.get_lock_owner(self.uuid)"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"    # NOTE(xek): We don\u0027t want to enable RPC on this call just yet. Remotable"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_45a8a31e","line":207,"updated":"2018-09-07 08:21:33.000000000","message":"nit: Sorry, I don\u0027t quite get the last sentence, it has too many \"this\".","commit_id":"00fe89b2172b65d84da07acb29486fb9bec267cb"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d51a499cfe1c47ba7c5616e4b6eb43e95cc1bd77","unresolved":false,"context_lines":[{"line_number":204,"context_line":"            # NOTE(TheJulia): Super special case for database locking so we"},{"line_number":205,"context_line":"            # don\u0027t double every database query to refresh the node if"},{"line_number":206,"context_line":"            # we are using a database. That and if this is with the database"},{"line_number":207,"context_line":"            # this kind of becomes recursive."},{"line_number":208,"context_line":"            self.reservation \u003d LOCKING.get_lock_owner(self.uuid)"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"    # NOTE(xek): We don\u0027t want to enable RPC on this call just yet. Remotable"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_8a0ec22f","line":207,"in_reply_to":"3f79a3b5_45a8a31e","updated":"2018-09-07 13:52:47.000000000","message":"Okay, will revise, I need to stop using this.","commit_id":"00fe89b2172b65d84da07acb29486fb9bec267cb"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d823892b5a433c6adf1e9c293bf5f1e0fce35c13","unresolved":false,"context_lines":[{"line_number":428,"context_line":"            self.driver_internal_info \u003d {}"},{"line_number":429,"context_line":"            updates \u003d self.do_version_changes_for_db()"},{"line_number":430,"context_line":"        if (\u0027reservation\u0027 in updates"},{"line_number":431,"context_line":"            and CONF.conductor.locking_provider !\u003d \u0027database\u0027):"},{"line_number":432,"context_line":"                # NOTE(TheJulia): Special case where locking is"},{"line_number":433,"context_line":"                # not performed in the database, we don\u0027t want"},{"line_number":434,"context_line":"                # a lock holder to be preserved in the database"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_65a35f44","line":431,"updated":"2018-09-07 08:21:33.000000000","message":"ditto. maybe a special group of methods for this?","commit_id":"00fe89b2172b65d84da07acb29486fb9bec267cb"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d51a499cfe1c47ba7c5616e4b6eb43e95cc1bd77","unresolved":false,"context_lines":[{"line_number":428,"context_line":"            self.driver_internal_info \u003d {}"},{"line_number":429,"context_line":"            updates \u003d self.do_version_changes_for_db()"},{"line_number":430,"context_line":"        if (\u0027reservation\u0027 in updates"},{"line_number":431,"context_line":"            and CONF.conductor.locking_provider !\u003d \u0027database\u0027):"},{"line_number":432,"context_line":"                # NOTE(TheJulia): Special case where locking is"},{"line_number":433,"context_line":"                # not performed in the database, we don\u0027t want"},{"line_number":434,"context_line":"                # a lock holder to be preserved in the database"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_aa05be11","line":431,"in_reply_to":"3f79a3b5_65a35f44","updated":"2018-09-07 13:52:47.000000000","message":"Do you mean a method that essentially just returns true/false if we\u0027re using a non-database locking manager?","commit_id":"00fe89b2172b65d84da07acb29486fb9bec267cb"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"f37ab0cf614f9360cdf360b0163d6300dd606b3a","unresolved":false,"context_lines":[{"line_number":428,"context_line":"            self.driver_internal_info \u003d {}"},{"line_number":429,"context_line":"            updates \u003d self.do_version_changes_for_db()"},{"line_number":430,"context_line":"        if (\u0027reservation\u0027 in updates"},{"line_number":431,"context_line":"            and CONF.conductor.locking_provider !\u003d \u0027database\u0027):"},{"line_number":432,"context_line":"                # NOTE(TheJulia): Special case where locking is"},{"line_number":433,"context_line":"                # not performed in the database, we don\u0027t want"},{"line_number":434,"context_line":"                # a lock holder to be preserved in the database"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_e1d078cb","line":431,"in_reply_to":"3f79a3b5_aa05be11","updated":"2018-10-05 23:18:05.000000000","message":"I kind of wonder if it should be named something like \"ironic-db\" or maybe use something like interface capabilities on the class.... Just thinking out loud.","commit_id":"00fe89b2172b65d84da07acb29486fb9bec267cb"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"3ba1baf8ff19ff9a0a95e46c1371a8cca220da7d","unresolved":false,"context_lines":[{"line_number":202,"context_line":"            # database locking is not in use. This is so we don\u0027t double every"},{"line_number":203,"context_line":"            # database query to refresh the node lock status as the"},{"line_number":204,"context_line":"            # reservation field already represents the current status."},{"line_number":205,"context_line":"            self.reservation \u003d LOCKING.get_lock_owner(self.uuid)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    # NOTE(xek): We don\u0027t want to enable RPC on this call just yet. Remotable"},{"line_number":208,"context_line":"    # methods can be used in the future to replace current explicit RPC calls."}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_7538b0ec","line":205,"range":{"start_line":205,"start_character":54,"end_line":205,"end_character":63},"updated":"2018-10-31 08:28:25.000000000","message":"not quite sure i understand the purpose of this code block, but from the implementation of DatabaseLocking, the get_lock_owner accept a task or integer id right? self.uuid falls neither of them.","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d967b2004f3b2c8300171698292cff2d72a14035","unresolved":false,"context_lines":[{"line_number":202,"context_line":"            # database locking is not in use. This is so we don\u0027t double every"},{"line_number":203,"context_line":"            # database query to refresh the node lock status as the"},{"line_number":204,"context_line":"            # reservation field already represents the current status."},{"line_number":205,"context_line":"            self.reservation \u003d LOCKING.get_lock_owner(self.uuid)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    # NOTE(xek): We don\u0027t want to enable RPC on this call just yet. Remotable"},{"line_number":208,"context_line":"    # methods can be used in the future to replace current explicit RPC calls."}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_99213b51","line":205,"range":{"start_line":205,"start_character":54,"end_line":205,"end_character":63},"in_reply_to":"3f79a3b5_7538b0ec","updated":"2018-11-28 16:24:04.000000000","message":"So if and when we have something like an etcd lock manager and we are not using the database, the reservation field on the object from the database is invalid. This helps set the status of a lock and helps honor the existing behavior contract we have in place. That being said, I\u0027m noticing the self.uuid lock owner lookup. Hmmmm!","commit_id":"4fdb973430d84854da04620b12db2969f9a89a77"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"41922af381f3acbec5233a04dcb769565a8a12ff","unresolved":false,"context_lines":[{"line_number":203,"context_line":"            context, objects.TraitList(context),"},{"line_number":204,"context_line":"            objects.Trait, db_object[\u0027traits\u0027])"},{"line_number":205,"context_line":"        self.traits.obj_reset_changes()"},{"line_number":206,"context_line":"        if CONF.conductor.locking_provider !\u003d \u0027database\u0027:"},{"line_number":207,"context_line":"            # NOTE(TheJulia): Super special case that is only when internal"},{"line_number":208,"context_line":"            # database locking is not in use. This is so we don\u0027t double every"},{"line_number":209,"context_line":"            # database query to refresh the node lock status as the"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_1e55bfc1","line":206,"updated":"2018-12-13 17:07:26.000000000","message":"This means that nobody can subclass the \"database\" provider.. I\u0027d really prefer not to hardcode it here, but I don\u0027t have better ideas either.","commit_id":"c601696b7d5a8b6ac3704f127dc335bf85c56b99"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1cb803a11e87b58496b6d4b9a5b3d3e8e6a420c0","unresolved":false,"context_lines":[{"line_number":203,"context_line":"            context, objects.TraitList(context),"},{"line_number":204,"context_line":"            objects.Trait, db_object[\u0027traits\u0027])"},{"line_number":205,"context_line":"        self.traits.obj_reset_changes()"},{"line_number":206,"context_line":"        if CONF.conductor.locking_provider !\u003d \u0027database\u0027:"},{"line_number":207,"context_line":"            # NOTE(TheJulia): Super special case that is only when internal"},{"line_number":208,"context_line":"            # database locking is not in use. This is so we don\u0027t double every"},{"line_number":209,"context_line":"            # database query to refresh the node lock status as the"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_e594ebe8","line":206,"in_reply_to":"3f79a3b5_1e55bfc1","updated":"2018-12-14 01:07:20.000000000","message":"Your not really a huge fan of isinstance :)\n\nI feel like this is something that can be tackled if and when the time comes.","commit_id":"c601696b7d5a8b6ac3704f127dc335bf85c56b99"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"41922af381f3acbec5233a04dcb769565a8a12ff","unresolved":false,"context_lines":[{"line_number":207,"context_line":"            # NOTE(TheJulia): Super special case that is only when internal"},{"line_number":208,"context_line":"            # database locking is not in use. This is so we don\u0027t double every"},{"line_number":209,"context_line":"            # database query to refresh the node lock status as the"},{"line_number":210,"context_line":"            # reservation field already represents the current status."},{"line_number":211,"context_line":"            self.reservation \u003d LOCKING.get_lock_owner(self.uuid)"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    # NOTE(xek): We don\u0027t want to enable RPC on this call just yet. Remotable"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_be692b8f","line":210,"updated":"2018-12-13 17:07:26.000000000","message":"Even worse, it would be a recursion, since get_lock_owner calls Node.get.","commit_id":"c601696b7d5a8b6ac3704f127dc335bf85c56b99"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1cb803a11e87b58496b6d4b9a5b3d3e8e6a420c0","unresolved":false,"context_lines":[{"line_number":207,"context_line":"            # NOTE(TheJulia): Super special case that is only when internal"},{"line_number":208,"context_line":"            # database locking is not in use. This is so we don\u0027t double every"},{"line_number":209,"context_line":"            # database query to refresh the node lock status as the"},{"line_number":210,"context_line":"            # reservation field already represents the current status."},{"line_number":211,"context_line":"            self.reservation \u003d LOCKING.get_lock_owner(self.uuid)"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    # NOTE(xek): We don\u0027t want to enable RPC on this call just yet. Remotable"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_45903ff3","line":210,"in_reply_to":"3f79a3b5_be692b8f","updated":"2018-12-14 01:07:20.000000000","message":"Well, yeah. I didn\u0027t want to make it sound _that_ bad.... :)","commit_id":"c601696b7d5a8b6ac3704f127dc335bf85c56b99"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"36d3b2fab689b38939089f158a7f55ae13b0c1f2","unresolved":false,"context_lines":[{"line_number":211,"context_line":"            # NOTE(TheJulia): Super special case that is only when internal"},{"line_number":212,"context_line":"            # database locking is not in use. This is so we don\u0027t double every"},{"line_number":213,"context_line":"            # database query to refresh the node lock status as the"},{"line_number":214,"context_line":"            # reservation field already represents the current status."},{"line_number":215,"context_line":"            self.reservation \u003d LOCKING.get_lock_owner(self.uuid)"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    # NOTE(xek): We don\u0027t want to enable RPC on this call just yet. Remotable"}],"source_content_type":"text/x-python","patch_set":20,"id":"ffb9cba7_51e438e1","line":214,"updated":"2019-04-29 04:04:16.000000000","message":"Sigh. Maybe LOCKING.update_node(self, locked\u003dTrue) ?","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"03def507e4ad023edbd7286681ddac1eaec12047","unresolved":false,"context_lines":[{"line_number":211,"context_line":"            # NOTE(TheJulia): Super special case that is only when internal"},{"line_number":212,"context_line":"            # database locking is not in use. This is so we don\u0027t double every"},{"line_number":213,"context_line":"            # database query to refresh the node lock status as the"},{"line_number":214,"context_line":"            # reservation field already represents the current status."},{"line_number":215,"context_line":"            self.reservation \u003d LOCKING.get_lock_owner(self.uuid)"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    # NOTE(xek): We don\u0027t want to enable RPC on this call just yet. Remotable"}],"source_content_type":"text/x-python","patch_set":20,"id":"bfb3d3c7_ae6306dc","line":214,"in_reply_to":"ffb9cba7_51e438e1","updated":"2019-05-22 18:58:09.000000000","message":"There is actually no reason to start with the lock in place. This is just loading an object.","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"36d3b2fab689b38939089f158a7f55ae13b0c1f2","unresolved":false,"context_lines":[{"line_number":440,"context_line":"                # not performed in the database, we don\u0027t want"},{"line_number":441,"context_line":"                # a lock holder to be preserved in the database"},{"line_number":442,"context_line":"                # as this is an incorrect source of truth."},{"line_number":443,"context_line":"                self.reservation \u003d None"},{"line_number":444,"context_line":"        self._validate_and_remove_traits(updates)"},{"line_number":445,"context_line":"        self._validate_and_format_conductor_group(updates)"},{"line_number":446,"context_line":"        db_node \u003d self.dbapi.update_node(self.uuid, updates)"}],"source_content_type":"text/x-python","patch_set":20,"id":"ffb9cba7_f1e44ce2","line":443,"updated":"2019-04-29 04:04:16.000000000","message":".. and ditto with locked\u003dFalse?","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"03def507e4ad023edbd7286681ddac1eaec12047","unresolved":false,"context_lines":[{"line_number":440,"context_line":"                # not performed in the database, we don\u0027t want"},{"line_number":441,"context_line":"                # a lock holder to be preserved in the database"},{"line_number":442,"context_line":"                # as this is an incorrect source of truth."},{"line_number":443,"context_line":"                self.reservation \u003d None"},{"line_number":444,"context_line":"        self._validate_and_remove_traits(updates)"},{"line_number":445,"context_line":"        self._validate_and_format_conductor_group(updates)"},{"line_number":446,"context_line":"        db_node \u003d self.dbapi.update_node(self.uuid, updates)"}],"source_content_type":"text/x-python","patch_set":20,"id":"bfb3d3c7_0eaa52d1","line":443,"in_reply_to":"ffb9cba7_f1e44ce2","updated":"2019-05-22 18:58:09.000000000","message":"I see why your asking for the above now. :\\ ugh.","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"f2c830e863d07f2cb3b5477da9c06945b1f3eaf1","unresolved":false,"context_lines":[{"line_number":210,"context_line":"            context, objects.TraitList(context),"},{"line_number":211,"context_line":"            objects.Trait, db_object[\u0027traits\u0027])"},{"line_number":212,"context_line":"        self.traits.obj_reset_changes()"},{"line_number":213,"context_line":"        LOCKING.update_node(self)"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    # NOTE(xek): We don\u0027t want to enable RPC on this call just yet. Remotable"},{"line_number":216,"context_line":"    # methods can be used in the future to replace current explicit RPC calls."}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_f24cba77","line":213,"updated":"2019-06-04 04:09:39.000000000","message":"For the database implementation, I think this is not required at all, so maybe the intention here is to separate the locking backend. But this line makes me feel that we are using reserved field from the database as a lock indicator (no matter what locking provider is in use), IMHO the lock provider should be the source of truth about the lock status.\nConsider we are using the etcd locking, could we just ignore the node.reserved?","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"fe24e4534b4287c6dd5c332754cff97fcee33293","unresolved":false,"context_lines":[{"line_number":210,"context_line":"            context, objects.TraitList(context),"},{"line_number":211,"context_line":"            objects.Trait, db_object[\u0027traits\u0027])"},{"line_number":212,"context_line":"        self.traits.obj_reset_changes()"},{"line_number":213,"context_line":"        LOCKING.update_node(self)"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    # NOTE(xek): We don\u0027t want to enable RPC on this call just yet. Remotable"},{"line_number":216,"context_line":"    # methods can be used in the future to replace current explicit RPC calls."}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_cafd11b2","line":213,"in_reply_to":"9fb8cfa7_e94fd1ba","updated":"2019-06-06 01:32:59.000000000","message":"Acknowledged, make sense to me, we can iterate with other things putting in.","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"258f1757dfcc22ca1e9ace62c297d8012a2660e5","unresolved":false,"context_lines":[{"line_number":210,"context_line":"            context, objects.TraitList(context),"},{"line_number":211,"context_line":"            objects.Trait, db_object[\u0027traits\u0027])"},{"line_number":212,"context_line":"        self.traits.obj_reset_changes()"},{"line_number":213,"context_line":"        LOCKING.update_node(self)"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    # NOTE(xek): We don\u0027t want to enable RPC on this call just yet. Remotable"},{"line_number":216,"context_line":"    # methods can be used in the future to replace current explicit RPC calls."}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_e94fd1ba","line":213,"in_reply_to":"9fb8cfa7_f24cba77","updated":"2019-06-05 13:45:36.000000000","message":"Well, we\u0027ve moderately made an API contract for the data in node.reserved to be representative, even if we\u0027ve changed all of the internals with regards to locking. The plus side with this is that we then let the module decide what to do, and in the database case nothing happens, where as with an etcd case it the default class would just get leveraged.","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"}],"ironic/tests/unit/conductor/test_manager.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"36d3b2fab689b38939089f158a7f55ae13b0c1f2","unresolved":false,"context_lines":[{"line_number":8193,"context_line":"        self.node.refresh()"},{"line_number":8194,"context_line":"        mock_off_cond.assert_called_once_with()"},{"line_number":8195,"context_line":"        # mock_mapped.assert_called_once_with(self.node.uuid, \u0027fake-hardware\u0027,"},{"line_number":8196,"context_line":"        #                                    \u0027\u0027)"},{"line_number":8197,"context_line":""},{"line_number":8198,"context_line":"        # assert node was released"},{"line_number":8199,"context_line":"        self.assertIsNone(self.node.reservation)"}],"source_content_type":"text/x-python","patch_set":20,"id":"ffb9cba7_d1ca684c","line":8196,"updated":"2019-04-29 04:04:16.000000000","message":"nit: remove?","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"03def507e4ad023edbd7286681ddac1eaec12047","unresolved":false,"context_lines":[{"line_number":8193,"context_line":"        self.node.refresh()"},{"line_number":8194,"context_line":"        mock_off_cond.assert_called_once_with()"},{"line_number":8195,"context_line":"        # mock_mapped.assert_called_once_with(self.node.uuid, \u0027fake-hardware\u0027,"},{"line_number":8196,"context_line":"        #                                    \u0027\u0027)"},{"line_number":8197,"context_line":""},{"line_number":8198,"context_line":"        # assert node was released"},{"line_number":8199,"context_line":"        self.assertIsNone(self.node.reservation)"}],"source_content_type":"text/x-python","patch_set":20,"id":"bfb3d3c7_2e35d646","line":8196,"in_reply_to":"ffb9cba7_d1ca684c","updated":"2019-05-22 18:58:09.000000000","message":"Done","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"}],"ironic/tests/unit/objects/test_node.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"e5c75bba02f8156a0c3b3a8bed2aa268d5cdca7c","unresolved":false,"context_lines":[{"line_number":351,"context_line":"    # have a second locking provider in-tree that is _NOT_"},{"line_number":352,"context_line":"    # the database as we presently don\u0027t want to double"},{"line_number":353,"context_line":"    # poll the database for lock status, and for datbase"},{"line_number":354,"context_line":"    # instances, the update node action is a pass."},{"line_number":355,"context_line":"    # @mock.patch.object(locking.database.OtherLocking,"},{"line_number":356,"context_line":"    #                    \u0027get_lock_owner\u0027, autospec\u003dTrue)"},{"line_number":357,"context_line":"    # def test_refresh_shows_lock(self, mock_lock_factory):"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_ed682a88","line":354,"updated":"2019-06-06 09:22:25.000000000","message":"Can we create a fake provider for tests only? Also, maybe use test skipping instead of commenting out?","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1e32917fd7133f375594552a0cc84d18bb2c999e","unresolved":false,"context_lines":[{"line_number":351,"context_line":"    # have a second locking provider in-tree that is _NOT_"},{"line_number":352,"context_line":"    # the database as we presently don\u0027t want to double"},{"line_number":353,"context_line":"    # poll the database for lock status, and for datbase"},{"line_number":354,"context_line":"    # instances, the update node action is a pass."},{"line_number":355,"context_line":"    # @mock.patch.object(locking.database.OtherLocking,"},{"line_number":356,"context_line":"    #                    \u0027get_lock_owner\u0027, autospec\u003dTrue)"},{"line_number":357,"context_line":"    # def test_refresh_shows_lock(self, mock_lock_factory):"}],"source_content_type":"text/x-python","patch_set":23,"id":"3fa7e38b_f75bf4f5","line":354,"in_reply_to":"9fb8cfa7_ed682a88","updated":"2019-12-23 23:34:28.000000000","message":"we could create a noop locking interface that always returns \"Not locked, lock all you want!\"\n\nAlthough, I\u0027m afraid someone would actually do that.......","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"}],"releasenotes/notes/locking-provider-5d585307ab161f0f.yaml":[{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"365eef31e0a1f1f5967601c5226b3458c7116a04","unresolved":false,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Adds a database locking interface and makes the overall lock"},{"line_number":5,"context_line":"    acquisition and release process work through the a locking interface"},{"line_number":6,"context_line":"    to enable additional interfaces to be created and facilitiate locking"},{"line_number":7,"context_line":"    using alternative means. This underlying locking change should be"},{"line_number":8,"context_line":"    transparent to users."}],"source_content_type":"text/x-yaml","patch_set":18,"id":"9fdfeff1_26f98b7f","line":5,"range":{"start_line":5,"start_character":49,"end_line":5,"end_character":54},"updated":"2019-02-07 18:22:14.000000000","message":"either the or a","commit_id":"468d436bd3c7e82888b60748d073ea09588908e0"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d8c0d6bc2c72efd851816d11056a389ba063364f","unresolved":false,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Adds a database locking interface and makes the overall lock"},{"line_number":5,"context_line":"    acquisition and release process work through the a locking interface"},{"line_number":6,"context_line":"    to enable additional interfaces to be created and facilitiate locking"},{"line_number":7,"context_line":"    using alternative means. This underlying locking change should be"},{"line_number":8,"context_line":"    transparent to users."}],"source_content_type":"text/x-yaml","patch_set":18,"id":"5fc1f717_a892029e","line":5,"range":{"start_line":5,"start_character":49,"end_line":5,"end_character":54},"in_reply_to":"9fdfeff1_26f98b7f","updated":"2019-03-22 21:41:40.000000000","message":"Done","commit_id":"468d436bd3c7e82888b60748d073ea09588908e0"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"36d3b2fab689b38939089f158a7f55ae13b0c1f2","unresolved":false,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"    An individual node can be queried for lock status via locking"},{"line_number":17,"context_line":"    interface method ``get_lock_owner``. A list of locks can be obtained"},{"line_number":18,"context_line":"    using the locking interface ``get_locks_for_conductors`` method."}],"source_content_type":"text/x-yaml","patch_set":20,"id":"ffb9cba7_11032075","line":18,"updated":"2019-04-29 04:04:16.000000000","message":"It\u0027s not a user-visible feature, so it should be here or in \"other\"","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"fdbf07d0250241e0613f7abcc7c4e7fee7c9cc5a","unresolved":false,"context_lines":[{"line_number":13,"context_line":"    tasks to query with current lock state. The result however, could often"},{"line_number":14,"context_line":"    be out of date with long running perodic tasks."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"    An individual node can be queried for lock status via locking"},{"line_number":17,"context_line":"    interface method ``get_lock_owner``. A list of locks can be obtained"},{"line_number":18,"context_line":"    using the locking interface ``get_locks_for_conductors`` method."}],"source_content_type":"text/x-yaml","patch_set":20,"id":"3fce034c_b61b5319","line":18,"range":{"start_line":16,"start_character":4,"end_line":18,"end_character":68},"updated":"2019-04-16 11:39:48.000000000","message":"nit: maybe move this up to features ?","commit_id":"33868395587db7b7ca1bb7f3c121066d5d10085d"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"e5c75bba02f8156a0c3b3a8bed2aa268d5cdca7c","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Adds a database locking interface and makes the overall lock"},{"line_number":5,"context_line":"    acquisition and release process work through a locking interface"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"9fb8cfa7_cd6d2699","line":2,"updated":"2019-06-06 09:22:25.000000000","message":"Should it be \"other\"? It\u0027s not really a feature anybody can use yet..","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"66fbf6afa4d20314831e5b70fef125f3139ff037","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Adds a database locking interface and makes the overall lock"},{"line_number":5,"context_line":"    acquisition and release process work through a locking interface"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"3fa7e38b_c044d41b","line":2,"in_reply_to":"9fb8cfa7_cd6d2699","updated":"2020-02-07 01:05:25.000000000","message":"With a fake interface, I guess they could...","commit_id":"2af1f3fe435877df0a647b768e3de240df24dc86"}]}