Skip to content

Properly reset dask.config scheduler and shuffle when closing the client#2475

Merged
mrocklin merged 1 commit intodask:masterfrom
gsakkis:master
Jan 28, 2019
Merged

Properly reset dask.config scheduler and shuffle when closing the client#2475
mrocklin merged 1 commit intodask:masterfrom
gsakkis:master

Conversation

@gsakkis
Copy link
Contributor

@gsakkis gsakkis commented Jan 22, 2019

Fixes #2473

with self._set_config:
pass
if self.get == dask.config.get('get', None):
del dask.config.config['get']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the user had specified something prior to starting the scheduler, like

dask.config.set(scheduler='threads')
with Client():
    pass
assert dask.config.get('scheduler') == 'threads'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment with shuffle.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tested it, the assertion passes; the with self._set_config restores the previous settings at exit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, you're right. That's a nice solution.

@mrocklin
Copy link
Member

This looks good to me. It looks like there is a test failure though. You might have to slightly modify that test.

@gsakkis
Copy link
Contributor Author

gsakkis commented Jan 23, 2019

I fixed the related test, the others that fail randomly are probably irrelevant (test_io_loop_periodic_callbacks on python 3.5 at one run, test_log_tasks_during_restart on 3.7 at another, with no code diff between them).

@mrocklin
Copy link
Member

Thanks @gsakkis !

Merging.

@mrocklin mrocklin merged commit b08622b into dask:master Jan 28, 2019
@mrocklin
Copy link
Member

Also, I notice that this is your first code contribution to this repository. Welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants