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

Improve user interface for the dask dashboard #78

Merged
merged 14 commits into from
Nov 27, 2024
Merged

Conversation

mandresm
Copy link
Contributor

@mandresm mandresm commented Nov 26, 2024

Thank you for contributing! 🎉

Summary of the most important changes

This pull request introduces:

  • a new module for managing the Dask cluster and integrates it with the existing codebase and adds a function to set the dashboard link for the Dask cluster and updating the cmorizer module to use this new function.
  • Reports the pymorize ssh-tunnel command that you need to run in your laptop

New module for Dask cluster management:

  • src/pymorize/cluster.py: Added a new module containing the set_dashboard_link function to manage the Dask cluster's dashboard link, including handling errors when the cluster is not launched from JupyterHub.

Integration with existing codebase:

  • src/pymorize/cmorizer.py: Imported the set_dashboard_link function from the new cluster module and used it in the _post_init_create_dask_cluster method to set the dashboard link for the SLURMCluster. [1] [2] in this PR.

Checklist

  • I have tested the changes in this PR.
  • I have updated the documentation.
  • [ ] If I have made a new step which requires new arguments, these are added to the validate.py file
    so that the user can see documentation for the arguments.

@mandresm mandresm marked this pull request as ready for review November 26, 2024 17:50
@mandresm
Copy link
Contributor Author

For some reason I cannot explore right now, the tests are broken. Other than that, I think is ready to be reviewed.

pgierz
pgierz previously approved these changes Nov 27, 2024
Copy link
Member

@pgierz pgierz left a comment

Choose a reason for hiding this comment

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

Looks good. I will merge this once I figure out what is going on with the tests.

src/pymorize/cluster.py Outdated Show resolved Hide resolved
setup.py Show resolved Hide resolved
@pgierz pgierz self-requested a review November 27, 2024 08:48
pgierz
pgierz previously approved these changes Nov 27, 2024
src/pymorize/cmorizer.py Outdated Show resolved Hide resolved
@pgierz pgierz merged commit d1bea94 into main Nov 27, 2024
4 checks passed
@mandresm mandresm deleted the fix/dask_dashboard branch November 27, 2024 09:28
@mandresm
Copy link
Contributor Author

Thanks @pgierz for the fixes!!!

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