Use correct CLI entrypoint #146

charlesbluca · 2022-10-25T17:11:37Z

Closes #145
Closes #144
Fixes dask/distributed#7176

Checklist

Used a personal fork of the feedstock to propose changes
Bumped the build number (if the version is unchanged)
Reset the build number to 0 (if the version changed)
Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
Ensured the license file is being packaged.

conda-forge-linter · 2022-10-25T17:11:42Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

charlesbluca · 2022-10-25T17:11:50Z

@conda-forge-admin, please rerender

github-actions · 2022-10-25T17:12:58Z

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/dask-core-feedstock/actions/runs/3322902694.

douglasdavis · 2022-10-25T17:49:38Z

Perhaps we should use dask.__main__:main as the entry point, because that's the one used in the pip(setuptools) installation.

The dask.__main__:main entry point was created to allow for the Click-not-installed error message. Is Click always installed when dask is installed via conda-forge?

charlesbluca · 2022-10-25T17:51:17Z

recipe/meta.yaml

+    - dask docs --help
+    - dask info --help
+    - dask info versions --help


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

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

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

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

charlesbluca · 2022-10-25T17:55:03Z

The dask.main:main entry point was created to allow for the Click-not-installed error message.

Was going to ask about this, as it seems like this error handling is implemented in cli.py as well, so wasn't sure if dask.__main__ was redundant now?

Is Click always installed when dask is installed via conda-forge?

It's not, we would need to make changes to the core dependencies if we wanted Dask's CLI to work out of the box after installing dask-core, see #146 (comment)

jakirkham · 2022-10-25T17:56:36Z

cc @jrbourbeau

douglasdavis · 2022-10-25T17:57:38Z

so wasn't sure if dask.__main__ was redundant now?

dask.__main__ isn't redundant because we want to allow folks to use $ python -m dask. The error message in dask.__main__ is there for folks who try to run $ dask or $ python -m dask, while the error message in dask.cli is there for folks who try to import dask.cli.

charlesbluca · 2022-10-25T18:08:16Z

Thanks for the context @douglasdavis 😄 in that case, using dask.__main__:main as the entrypoint seems to make the most sense here.

Some follow up questions:

how do we want to handle the click requirement since it's not specified in either dask-core or dask? We could just add it to the runtime requirements, though I notice it doesn't seem to be specified anywhere in the setup.py of dask-core
do we want to install the entrypoints as part of dask-core, dask, or both? My preference would be dask-core only, as that's where the CLI code resides and I can imagine situations where a user would want the CLI from dask-core without installing distributed
do we know why these distributed feedstock tests seemed to pass despite the fact that the distributed CLI wasn't working locally?

douglasdavis · 2022-10-25T18:22:00Z

how do we want to handle the click requirement since it's not specified in either dask-core or dask? We could just add it to the runtime requirements, though I notice it doesn't seem to be specified anywhere in the setup.py of dask-core

In the CLI PR we decided not to make Click a hard dependency. It seems that it may be easiest to just make it a dependency. ~~I think an alternative is we can make it a dependency of dask-feedstock and not dask-core-feedstock~~. Nevermind, distributed already depends on Click and dask-feedstock depends on distributed!

do we want to install the entrypoints as part of dask-core, dask, or both? My preference would be dask-core only, as that's where the CLI code resides and I can imagine situations where a user would want the CLI from dask-core without installing distributed

It looks like I shouldn't have opened conda-forge/dask-feedstock#200 - based on what @jrbourbeau and @jakirkham were doing I think the the entry point should be part of dask-core-feedstock?

douglasdavis · 2022-10-25T18:23:52Z

3. do we know why these distributed feedstock tests seemed to pass despite the fact that the distributed CLI wasn't working locally?

The only thing I can think of is maybe the command still returned exit code 0

jakirkham · 2022-10-25T18:26:49Z

It was doing that at the time. Maybe because of these?

Don't see that behavior with recent packages (that dropped those)

$ dask scheduler ; echo $?
Usage: dask [OPTIONS] COMMAND [ARGS]...
Try 'dask -h' for help.

Error: No such command 'scheduler'.
2

jrbourbeau

Thanks all for digging in here!

Folks seem to think dask = dask.cli:run_cli is the right thing to do here. I'm inclined to remove the dask docs --help tests for the time being (we have some coverage still as those are included as test in dask-feedstock) to see if the entypoint update fixes things. Adding click as a dask-core dependency seems like a relevant, but separate, conversation. Thoughts?

jakirkham · 2022-10-25T18:39:38Z

As noted above, think if we are adding an entry point here (especially given the issues we have had around it), we should have a test here as well. In terms of click we can add it to test/requires if it is controversial to make it a hard requirement

douglasdavis · 2022-10-25T18:41:42Z

@jrbourbeau I agree with your inclination (kick the can down the road on the Click dep w.r.t this PR, but at this point I would say for the upcoming release I think it's a good idea to just have a Click dep).

I'll also add that I think dask.__main__:main is the better entry point (consistency with dask's setup.cfg and it keeps $ dask and $ python -m dask identical)

charlesbluca · 2022-10-25T18:49:14Z

It was doing that at the time. Maybe because of these?

This is a passing run triggered by the removal of those entrypoints, so think it might be something else.

I'm generally in favor of @jakirkham's suggestion to add click to the test requirements for the time being so we at least have some security that the entrypoints are working properly, and @douglasdavis's suggestion to do with dask.__main__:main as the entrypoint for sake of consistency. Not sure if there have been internal conversations leaning towards dask.cli:run_cli though @jrbourbeau 🙂

jakirkham · 2022-10-25T18:53:55Z

@conda-forge-admin, please re-render

jakirkham · 2022-10-25T18:54:45Z

(oops missed re-rendering was already done above 🤦‍♂️)

github-actions · 2022-10-25T18:55:01Z

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/dask-core-feedstock/actions/runs/3323505831.

jakirkham

LGTM. Thanks all! 🙏

jrbourbeau · 2022-10-25T19:23:29Z

Just tried out the new build and dask scheduler is working as expected 🎉 thanks again @charlesbluca @douglasdavis @jakirkham!

jakirkham · 2022-10-25T19:36:08Z

Nice! Also working for me 🎉

Think we will want to add these changes to the dask-core nightly as well

Am looking into making click a hard requirement ( dask/dask#9595 )

jakirkham · 2022-10-28T22:17:13Z

click became a hard requirement in the recent release ( #147 )

Dropping click from test/requires in PR ( #148 )

Fix CLI entrypoint

5246336

charlesbluca requested review from jakirkham, jcrist, jrbourbeau, martindurant, mrocklin, pitrou, shoyer, sinhrks and TomAugspurger as code owners October 25, 2022 17:11

charlesbluca mentioned this pull request Oct 25, 2022

Resolving new CLI bug #145

Closed

1 task

Add CLI tests

8f09ec7

charlesbluca commented Oct 25, 2022

View reviewed changes

This was referenced Oct 25, 2022

dask scheduler not recognized as a valid command, despite suggestion from FutureWarning dask/distributed#7176

Closed

Drop dask entry point #144

Closed

douglasdavis mentioned this pull request Oct 25, 2022

update entry point conda-forge/dask-feedstock#200

Closed

jrbourbeau reviewed Oct 25, 2022

View reviewed changes

Update entrypoint, add click to test requirements

b5c558a

jakirkham approved these changes Oct 25, 2022

View reviewed changes

jakirkham requested a review from jrbourbeau October 25, 2022 19:02

jrbourbeau approved these changes Oct 25, 2022

View reviewed changes

jrbourbeau merged commit 0c9c6bb into conda-forge:main Oct 25, 2022

jakirkham mentioned this pull request Oct 25, 2022

Require Click 7.0+ in Dask dask/dask#9595

Merged

3 tasks

charlesbluca mentioned this pull request Oct 26, 2022

Update nightly recipe with CLI entrypoint dask/dask#9600

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use correct CLI entrypoint #146

Use correct CLI entrypoint #146

charlesbluca commented Oct 25, 2022 •

edited by jakirkham

conda-forge-linter commented Oct 25, 2022

charlesbluca commented Oct 25, 2022

github-actions bot commented Oct 25, 2022

douglasdavis commented Oct 25, 2022 •

edited

charlesbluca Oct 25, 2022

jakirkham Oct 25, 2022

jrbourbeau Oct 25, 2022

jakirkham Oct 25, 2022 •

edited

charlesbluca commented Oct 25, 2022

jakirkham commented Oct 25, 2022

douglasdavis commented Oct 25, 2022

charlesbluca commented Oct 25, 2022 •

edited

douglasdavis commented Oct 25, 2022 •

edited

douglasdavis commented Oct 25, 2022

jakirkham commented Oct 25, 2022

jrbourbeau left a comment

jakirkham commented Oct 25, 2022

douglasdavis commented Oct 25, 2022 •

edited

charlesbluca commented Oct 25, 2022

jakirkham commented Oct 25, 2022

jakirkham commented Oct 25, 2022

github-actions bot commented Oct 25, 2022

jakirkham left a comment

jrbourbeau commented Oct 25, 2022

jakirkham commented Oct 25, 2022

jakirkham commented Oct 28, 2022

Use correct CLI entrypoint #146

Use correct CLI entrypoint #146

Conversation

charlesbluca commented Oct 25, 2022 • edited by jakirkham

conda-forge-linter commented Oct 25, 2022

charlesbluca commented Oct 25, 2022

github-actions bot commented Oct 25, 2022

douglasdavis commented Oct 25, 2022 • edited

charlesbluca Oct 25, 2022

Choose a reason for hiding this comment

jakirkham Oct 25, 2022

Choose a reason for hiding this comment

jrbourbeau Oct 25, 2022

Choose a reason for hiding this comment

jakirkham Oct 25, 2022 • edited

Choose a reason for hiding this comment

charlesbluca commented Oct 25, 2022

jakirkham commented Oct 25, 2022

douglasdavis commented Oct 25, 2022

charlesbluca commented Oct 25, 2022 • edited

douglasdavis commented Oct 25, 2022 • edited

douglasdavis commented Oct 25, 2022

jakirkham commented Oct 25, 2022

jrbourbeau left a comment

Choose a reason for hiding this comment

jakirkham commented Oct 25, 2022

douglasdavis commented Oct 25, 2022 • edited

charlesbluca commented Oct 25, 2022

jakirkham commented Oct 25, 2022

jakirkham commented Oct 25, 2022

github-actions bot commented Oct 25, 2022

jakirkham left a comment

Choose a reason for hiding this comment

jrbourbeau commented Oct 25, 2022

jakirkham commented Oct 25, 2022

jakirkham commented Oct 28, 2022

charlesbluca commented Oct 25, 2022 •

edited by jakirkham

douglasdavis commented Oct 25, 2022 •

edited

jakirkham Oct 25, 2022 •

edited

charlesbluca commented Oct 25, 2022 •

edited

douglasdavis commented Oct 25, 2022 •

edited

douglasdavis commented Oct 25, 2022 •

edited