ray: Abstraction violations in command_runner interface

It seems we have a few new abstraction violations for the CommandRunnerInterface.

8f0f7371a055257b4c704bad49edccbfbfc01831 (#10097) introduced run_cp_up and run_cp_down. dc378a80b77685cc45c2b53e2f484ed9c6d8c44c (#9515) introduced run_init

cc @AmeerHajAli

@richardliaw @krfricke these abstraction violations are blocking @AmeerHajAli 's work proxying the cloud providers.

Can we either (1) revert the features that depend on this or (2) otherwise make the autoscaler / tune etc strictly use only the CommandRunnerInterface, without attempting to peek behind the scenes at the implementation class?

About this issue

Original URL
State: closed
Created 4 years ago
Comments: 16 (16 by maintainers)

Most upvoted comments

@AmeerHajAli SGTM, @wuisawesome I don’t imagine changing this (run_init) abstraction would cause many merge conflicts as it would be adding a 2-4 lines to command_runner.py and removing 2 from updater.py.

ijrsvt on Sep 10, 2020

@richardliaw I see, I can remove the external calls to run_cp_<up/down> and make run_init part of the command runner interface.

Are you all ok with that? @ericl @richardliaw @ijrsvt @wuisawesome ?

AmeerHajAli on Sep 10, 2020