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

Better Dask integration #23

Merged
merged 13 commits into from Jul 7, 2021
Merged

Better Dask integration #23

merged 13 commits into from Jul 7, 2021

Conversation

ethangreen-dev
Copy link
Collaborator

@ethangreen-dev ethangreen-dev commented Jul 7, 2021

This PR contains numerous changes that (hopefully) improve the behavior of Client <--> Compute <--> Redis interactions.

Changes

  • Delegate instances are no longer constructed once on API start. Instead, they're dynamically created and destroyed based on Flask request sessions.
  • Changed DaskDelegate to no longer depend on internal state to track running jobs. Instead, the Dask Scheduler is queried for the state and, if completed, the results are grabbed from Redis.
  • Dask Future instances are no longer needed to be kept around to ensure that their corresponding job is run on a Worker.
  • Added Delegate#job_complete function to support new workflow.
  • Moved Flask instantiation out of __init__.py and into api.py.
  • General cleanup of the project: Removed dead code and source files, added documentation, comments, etc.

Closes #18

+ Added Delegate#job_complete function to support new methods
  of task tracking in the DaskDelegate.

~ Changed job creation to fire-and-forget. This ensures that Dask
  will not depend on the actual status of the Future to run.
~ Changed results retrieval to be entirely redis-dependent.
~ Updated DaskDelegate#job_status to use TaskState#state values.

- Disabled LocalCluster support due to changes in how delegates
  are instantiated in the API.
- Removed dependence on local future maps -- this no longer works
  due to API changes.
@ethangreen-dev ethangreen-dev added the enhancement New feature or request label Jul 7, 2021
@ethangreen-dev ethangreen-dev added this to the v1.0.0b milestone Jul 7, 2021
@ethangreen-dev ethangreen-dev self-assigned this Jul 7, 2021
@ethangreen-dev ethangreen-dev added this to In Progress in Release v0.9 via automation Jul 7, 2021
@ethangreen-dev ethangreen-dev marked this pull request as ready for review July 7, 2021 21:04
@ethangreen-dev ethangreen-dev moved this from In Progress to Awaiting Review in Release v0.9 Jul 7, 2021
:param port: The port to listen on.
:type port: int

:param delegate: The type of delegate to be used to run simulation jobs. Defaults
Copy link
Collaborator

@mdip226 mdip226 Jul 7, 2021

Choose a reason for hiding this comment

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

:param delegate_type:
:type delegate_type: type
or something like that, maybe?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for catching that -- fixed!

~ Changed start_api() docstring to reference `delegate_type`, not
  `delegate`.
@mdip226 mdip226 merged commit 0df360c into dev Jul 7, 2021
Release v0.9 automation moved this from Awaiting Review to Done Jul 7, 2021
@ethangreen-dev ethangreen-dev added the ​✔️​ Issue / PR has been resolved. label Jul 7, 2021
@ethangreen-dev ethangreen-dev linked an issue Jul 7, 2021 that may be closed by this pull request
@ethangreen-dev ethangreen-dev deleted the feature/better-dask-integration branch October 15, 2021 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ​✔️​ Issue / PR has been resolved.
Projects
No open projects
Release v0.9
  
Done
Development

Successfully merging this pull request may close these issues.

Add remote Dask cluster compute support
2 participants