distributed

distributed copied to clipboard

This transitions existing CLI tooling to use the new dask CLI tool from dask/dask#9283

• Moves tests to use dask foo instead of dask-foo
• Removes some deprecated options and tests associated with those depcrecations.
• Adds a warning to the dask-foo flavor single executables
• Updates all occurrences in the documentation of the single executables

Unit Test Results

See test report for an extended history of previous test failures. This is useful for diagnosing flaky tests.

       15 files  ±  0         15 suites  ±0   6h 14m 26s :stopwatch: - 9m 10s   3 143 tests  - 10    3 052 :heavy_check_mark:  - 17    85 :zzz: +1  6 :x: +6  23 251 runs   - 84  22 332 :heavy_check_mark:  - 98  913 :zzz: +8  6 :x: +6 

For more details on these failures, see this check.

Results for commit 2cbf0b4f. ± Comparison against base commit 044225ae.

:recycle: This comment has been updated with latest results.

Two requests:

1. Verify that the previous commands still work (seems like tests pass, so we should be good)
2. Add a simple test showing that the new ones do the same thing?

Closing in favor of #6738

I'm not sure that the job submission PR is ideal. I would leave this one open. I suspect that it'll be easier to merge.

On Tue, Jul 19, 2022 at 1:24 PM Doug Davis @.***> wrote:

Closed #6735 https://github.com/dask/distributed/pull/6735.

— Reply to this email directly, view it on GitHub https://github.com/dask/distributed/pull/6735#event-7022998257, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACKZTEO5GGGDIE5K44CNTTVU3XGTANCNFSM53YKLJEA . You are receiving this because you commented.Message ID: @.***>

We should probably use this as an opportunity to enforce a couple of deprecations. For instance, the following arguments are no longer supported. it would be great if the new CLI would not even show them.

• --reconnect
• --nprocs
• --bokeh
• `--bokeh-port

This should be ready to go upon reversing the dependence on my douglasdavis/dask@new-cli branch (I'll leave it as draft until that is reversed), see my comment at the partner dask/dask PR

@jrbourbeau no worries! fixed the conflict and merged main in both https://github.com/dask/dask/pull/9283 and here (PS the merge conflict really had to do with the temporary overriding of the dask version requirement to the branch in dask/dask#9283)

Thanks for the review @jrbourbeau! I've incorporated suggestions/responded to your comments-- happy to keep iterating.

Looks like test_deprecated_single_executable is hitting a 30 second time out sometimes. Should we adjust a timeout argument?

Pushed an update which resolves the test_deprecated_single_executable issue

https://github.com/dask/distributed/pull/6735#issuecomment-1219449460

We should probably use this as an opportunity to enforce a couple of deprecations. For instance, the following arguments are no longer supported. it would be great if the new CLI would not even show them.

• --reconnect
• --nprocs
• --bokeh
• --bokeh-port

Note that this became a breaking change with no entry in the changelog and entirely unreflected in the PR title.

Thanks for reporting @consideRatio. I'm not sure I follow as I thought we had already been emitting warnings about these options (for example, this sort of thing). Which option(s) change was problematic for you?

Its nothing problematic about making the breaking change for me, but its a plus if its made a bit more visible via the changelog, PR title, PR description.

https://github.com/dask/helm-chart/issues/338

I didnt see warnings in this case due to the nature of how dask-scheduler was used :/

I should have included something in the PR about removing the deprecations, apologies for that. It makes sense that automation tools would indeed make FutureWarnings harder to catch

Yeah, that's a fair point. See https://github.com/dask/distributed/pull/7178 for an update to the changelog