do not log an error on unset variable delete #3652
Conversation
If a Variable is never set or accessed no entries are made in the tracking attributes of VariableExtension. Therefore there is no need to raise or log an error when the variable is deleted.
|
I've added a test here but it passes even without this change. An error is logged by the scheduler but I'm not sure how to turn this into a failure in the test. |
Try |
distributed/tests/test_variable.py
Outdated
| def test_delete_unset_variable(client): | ||
| x = Variable() | ||
| assert x.client is client | ||
| x.delete() |
There was a problem hiding this comment.
I encourage you to use gen_cluster whenever possible when testing. This is for a few reasons:
- If we need to debug we can enter a breakpoint anywhere.
gen_clustersets up everything to run in the main thread - It's much faster than the
clientfixture, which while more comfortable because it uses the synchronous syntax, also sets up some processes - It will force you to learn some async things if you don't know them already
https://distributed.dask.org/en/latest/develop.html#writing-tests
There was a problem hiding this comment.
I modified the test to use gen_cluster and captured_logger, thanks for the suggestion.
I needed to trigger an action on the client after the delete to ensure the error was logged. I used c.close for this, was curious is there is a better way to do this.
|
Friendly ping @jjhelmus |
Capture the log in the test_delete_unset_variable test to check that no KeyError was emitted.
Co-Authored-By: Matthew Rocklin <mrocklin@gmail.com>
Co-Authored-By: Matthew Rocklin <mrocklin@gmail.com>
|
Ah, it looks like the delete method isn't asynchronous. It just fires of a message and returns immediately. Hrm, that kind of mismatch makes using these things a bit rough. Probably not something to resolve now, but something to keep in mind. |
|
Thanks @jjhelmus |
If a Variable is never set or accessed no entries are made in the
tracking attributes of VariableExtension. Therefore there is no need to
raise or log an error when the variable is deleted.
closes #3492