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.

Sign up for GitHub

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

Jump to bottom

Use correct CLI entrypoint #146

Merged
merged 3 commits into from
Oct 25, 2022
Merged

Use correct CLI entrypoint #146

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5246336
Fix CLI entrypoint
charlesbluca Oct 25, 2022
8f09ec7
Add CLI tests
charlesbluca Oct 25, 2022
b5c558a
Update entrypoint, add click to test requirements
charlesbluca Oct 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions recipe/meta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ source:
sha256: f277a3b300ecc1d0be232a339f346dff686e0becdb0843c0aaf2ae943d5492d3

build:
number: 1
number: 2
noarch: python
script: {{ PYTHON }} -m pip install . -vv
entry_points:
- dask = dask.cli:cli
- dask = dask.__main__:main

requirements:
host:
Expand All @@ -35,8 +35,12 @@ test:
- dask
commands:
- pip check
- dask docs --help
- dask info --help
- dask info versions --help
Comment on lines +38 to +40
Copy link
Member Author

Choose a reason for hiding this comment

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

Tests are failing because the CLI requires click; are we okay with adding click as a core dependency for the Dask conda packages? Or are these tests better suited for the dask metapackage

Copy link
Member

Choose a reason for hiding this comment

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

Yes think that is reasonable

We should then also added it to Dask's setup.py and the nightly package

Would reuse Distributed click >=6.6 requirement

Copy link
Member

Choose a reason for hiding this comment

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

I initially suggested adding click as a required dependency of dask (pip) / dask-core (conda) but there was some push back against that. I personally don't have strong thoughts one way or the other. I agree we should have these. We currently have these types of checks over in the dask-feedstock due to the current places that click is installed

Copy link
Member

@jakirkham jakirkham Oct 25, 2022 •
edited

Choose a reason for hiding this comment

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

Yeah think we still want some kind of check in this feedstock to make sure the entry point added here works

We could just add click to test/requires as is done with pip below

That said, I think not including click as a requirement in Dask itself while adding entry points is a subpar user experience. Think we should bring this up again and point back to this issue (and its resolution) as a reason to change it

requires:
- pip
- click

about:
home: https://github.com/dask/dask/
Expand Down