Page MenuHomePhabricator

RFC: Add support for specifying Dask resources
ClosedPublic

Authored by natekupp on Aug 20 2019, 11:32 PM.

Details

Reviewers
schrockn
alangenfeld
Group Reviewers
Restricted Project
Commits
R1:d0a7e0e0f805: RFC: Add support for specifying Dask resources
Summary

Proposal for programmatic specification of Dask resources. (tests will fail here because the test Dask cluster doesn't provide a "CPU" resource).

Dask resources are abstract and can be anything you want; e.g. {"CPU": 12} is fine, but so is {"foo_bar": 2}; https://distributed.dask.org/en/latest/resources.html - and are specified at worker creation and task instantiation via client.submit()

Worth in the future doing this through the config system, but something like this would at least unblock the folks who are requesting this feature in #general

Test Plan

unit

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

natekupp created this revision.Aug 20 2019, 11:32 PM
natekupp retitled this revision from Add support for specifying Dask resources to RFC: Add support for specifying Dask resources.Aug 21 2019, 4:21 PM
natekupp edited the summary of this revision. (Show Details)
natekupp added a reviewer: Restricted Project.
alangenfeld added a subscriber: alangenfeld.EditedAug 21 2019, 9:10 PM

some other options to consider:

  • namespace all dask stuff in one object

{'dask': {'resource_requirements': {...}}}

  • use a name spacing scheme ala k8s

{'dask.engine/resource_requirements': {...}}

agree on deciding on some sort of namespacing scheme here. i bias towards {'dask': {'resource_requirements': {...}}} option because it is inline with our current config system. but it does say something the software people like (e.g. vscode and k8s) using the in-key namespacing scheme

schrockn requested changes to this revision.Aug 22 2019, 3:13 AM
This revision now requires changes to proceed.Aug 22 2019, 3:13 AM
natekupp updated this revision to Diff 3934.Aug 22 2019, 4:33 PM
natekupp edited the summary of this revision. (Show Details)

up

This revision was not accepted when it landed; it landed in state Needs Review.Aug 22 2019, 4:55 PM
This revision was automatically updated to reflect the committed changes.