Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add client_kwargs option and fix datasets_from_delayed #103

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

bolliger32
Copy link
Collaborator

  • closes #xxxx

  • tests added / passed

  • docs reflect changes

  • passes flake8 rhg_compute_tools tests docs

  • entry in HISTORY.rst

  • client_kwargs`` option in rhg_compute_tools.kubernetes.get_clusterallows you to pass kwargs to thedask.distributed.Client` instantiation command

  • Fixes a typo in rhg_compute_tools.xarray.datasets_from_delayed related to client_kwargs that was preventing this function from executing.

Not sure whats going on with that failed test at the moment.

@delgadom
Copy link
Member

delgadom commented Jan 5, 2022

I think this might be the culprit:
https://github.com/RhodiumGroup/rhg_compute_tools/blob/client-kwargs/tests/test_rhg_compute_tools.py#L35-L36

def monkeypatch_cluster(func):
    def inner(monkeypatch, mem, cpu, scale):
        ...
        def mock_dask_Client(cluster):
            return cluster  # this gets invoked by `client.gather()` which isn't a method on LocalGateway
        ...

Not sure what the best way around this is if you're avoiding creating a proper dask client, but if not... you could just pass back dd.Client(cluster)

@delgadom
Copy link
Member

delgadom commented Jan 5, 2022

unless you mean.... not sure why this is popping up now... in which case I agree with you 🤷🏼‍♂️

@bolliger32
Copy link
Collaborator Author

yeah that's what i was wondering. I feel like there was a reason we couldn't create an actual client here...but can't remember what it was. worth a try at least, i suppose

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