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

Adding conda environment file for development dependencies #488

Merged
merged 30 commits into from
Mar 26, 2022

Conversation

rigzba21
Copy link
Contributor

Hi all! Just adding a conda environment file to help with the local development setup. Thanks!

dask-gateway-dev.yaml Outdated Show resolved Hide resolved
@martindurant
Copy link
Member

@consideRatio @jacobtomlinson - the docs build here is falling now, with permission error on git push to gh-pages. I assume this is because of the conversation about keys?

@consideRatio
Copy link
Collaborator

@martindurant the failure is because we don't have a if condition to not try publish the docs from within a PR. There will not be credentials available for pushing to gh-pages within a PR triggered github workflow, and shouldn't be. We have just made a mistake of not letting the publishing be conditional on pushes to the main branch.

This is fixed in #495, see c04ea59.

Copy link
Collaborator

@consideRatio consideRatio left a comment

Choose a reason for hiding this comment

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

I think this could be very helpful, but at the same time, I'm very afraid of the maintenance as this seem to declare things that are declared in multiple locations.

I love the comments like dask-gateway/continuous_integration/install.sh which help me assume that what is below relates to dependencies listed there for example.

I think what I'd like to see as part of this PR, is even more clarity with such comments that are also made even more explicit - that "please update ... as well if you update anything below here". Along with such comments, a comment in those files that if they update something, those updates should also be made here.

With comments like that, it is just a matter of some chore work that I think is worth doing to provide a quicker setup to work with this project. But on the other hand, if such comments are left out, I think we will end up with something that gets outdated and confusing over time.

Requested changes

  • For all dependencies declared in multiple places, add comments in those places to ensure updates done at one place is also done at the other.

@rigzba21
Copy link
Contributor Author

@consideRatio thank you for the recommendation! I've added comments in the environment file for where dependencies are defined and which component they're used for.

dask-gateway-dev.yaml Outdated Show resolved Hide resolved
dask-gateway-dev.yaml Outdated Show resolved Hide resolved
Minor doc fix!

The [install instructions for dask-gateway on Kubernetes](https://gateway.dask.org/install-kube.html#shutting-everything-down) use an unsupported flag (`--purge`) in the `helm` command for cleanup and uninstalling work. When run, this throws an error at the poor user.

This PR updates the docs to remove the `--purge` flag. The behavior from `--purge` is the default behavior in the current version of `helm`.
dask-gateway-dev.yaml Outdated Show resolved Hide resolved
dask-gateway-dev.yaml Outdated Show resolved Hide resolved
dask-gateway-dev.yaml Outdated Show resolved Hide resolved
dask-gateway-dev.yaml Outdated Show resolved Hide resolved
dask-gateway-dev.yaml Outdated Show resolved Hide resolved
dask-gateway-dev.yaml Outdated Show resolved Hide resolved
dask-gateway-server/setup.py Outdated Show resolved Hide resolved
dask-gateway/setup.py Outdated Show resolved Hide resolved
docs/source/develop.rst Outdated Show resolved Hide resolved
docs/source/develop.rst Outdated Show resolved Hide resolved
@rigzba21
Copy link
Contributor Author

@consideRatio thank you for the help pointing me in the right direction to where all of these dependencies are located!

@consideRatio consideRatio merged commit 0d0dec1 into dask:main Mar 26, 2022
@rigzba21 rigzba21 deleted the dev-conda-env branch March 26, 2022 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants