Repository: modin-project/modin
Pull request: FIX-#2225: update parameters of ray APIs for cloud features
Signed-off-by: Anatoly Myachev <anatoly.myachev@intel.com>

<!--
Thank you for your contribution! 
Please review the contributing docs: https://modin.readthedocs.io/en/latest/contributing.html
if you have questions about contributing.
-->

## What do these changes do?

<!-- Please give a short brief about these changes. -->

- [ ] commit message follows format outlined [here](https://modin.readthedocs.io/en/latest/contributing.html)
- [ ] passes `flake8 modin`
- [ ] passes `black --check modin`
- [ ] signed commit with `git commit -s` <!-- you can amend your commit with a signature via `git commit -amend -s` -->
- [x] Resolves #2225 <!-- issue must be created for each patch -->
- [ ] tests added and passing


Problem statement:
Update ray API used for cloud features after pin ray>=1.0.0
### System information
- **OS Platform and Distribution (e.g., Linux Ubuntu 16.04)**:
- **Modin version** (`modin.__version__`): `0.8.1.1`
- **Python version**:
- **Code we can use to reproduce**:

<!--
You can obtain the Modin version with

python -c "import modin; print(modin.__version__)"
-->

### Describe the problem
<!-- Describe the problem clearly here. -->

```shell
...
modin.experimental.cloud.base.CannotSpawnCluster: cause: create_or_update_cluste
r() got an unexpected keyword argument 'log_old_style'
Cannot spawn cluster
Cannot destroy cluster:
Traceback (most recent call last):
  File "modin/modin/experimental/cloud/rayscale.py", line 2
20, in __do_destroy
    teardown_cluster(
TypeError: teardown_cluster() got an unexpected keyword argument 'log_old_style'
...
```

### Source code / logs
<!-- Include any logs or source code that would be helpful to diagnose the problem. If including tracebacks, please include the full traceback. Large logs and files should be attached. Try to provide a reproducible test case that is the bare minimum necessary to generate the problem. -->

Hints from issue discussion:
Also add unit tests for the problem.

Commit message: FIX-#2225: use pytest.fixture to create mock objects

Signed-off-by: Anatoly Myachev <anatoly.myachev@intel.com>

Patch to review:
diff --git a/modin/experimental/cloud/rayscale.py b/modin/experimental/cloud/rayscale.py
index 84d80165a84..6530ac365ec 100644
--- a/modin/experimental/cloud/rayscale.py
+++ b/modin/experimental/cloud/rayscale.py
@@ -200,9 +200,8 @@ def __do_spawn(self):
                 yes=True,
                 override_cluster_name=None,
                 no_config_cache=False,
-                log_old_style=False,
-                log_color="auto",
-                verbose=1,
+                redirect_command_output=False,
+                use_login_shells=True,
             )
             # need to re-load the config, as create_or_update_cluster() modifies it
             with open(self.config_file) as inp:
@@ -223,9 +222,6 @@ def __do_destroy(self):
                 workers_only=False,
                 override_cluster_name=None,
                 keep_min_workers=0,
-                log_old_style=False,
-                log_color="auto",
-                verbose=1,
             )
             self.ready = False
             self.config = None
diff --git a/modin/experimental/cloud/test/test_cloud.py b/modin/experimental/cloud/test/test_cloud.py
index f35e08f5fbd..ec179a9b694 100644
--- a/modin/experimental/cloud/test/test_cloud.py
+++ b/modin/experimental/cloud/test/test_cloud.py
@@ -14,8 +14,111 @@
 import unittest.mock as mock
 import pytest
 from collections import namedtuple
+from inspect import signature
 from modin.experimental.cloud.rayscale import RayCluster
 from modin.experimental.cloud.cluster import Provider
+from ray.autoscaler.commands import (
+    create_or_update_cluster,
+    teardown_cluster,
+    get_head_node_ip,
+    _bootstrap_config,
+)
+
+
+@pytest.fixture
+def make_bootstrap_config_mock():
+    sig_bootstrap_config = signature(_bootstrap_config)
+
+    def bootstrap_config_mock(*args, **kwargs):
+        sig_bootstrap_config.bind(*args, **kwargs)
+        config = args[0]
+        config["auth"]["ssh_user"] = "modin"
+        config["auth"]["ssh_private_key"] = "X" * 20
+        return config
+
+    return bootstrap_config_mock
+
+
+@pytest.fixture
+def make_get_head_node_ip_mock():
+    sig_get_head_node_ip = signature(get_head_node_ip)
+
+    def get_head_node_ip_mock(*args, **kwargs):
+        sig_get_head_node_ip.bind(*args, **kwargs)
+        return "127.0.0.1"
+
+    return get_head_node_ip_mock
+
+
+@pytest.fixture
+def make_teardown_cluster_mock():
+    sig_teardown_cluster = signature(teardown_cluster)
+
+    def teardown_cluster_mock(*args, **kwargs):
+        sig_teardown_cluster.bind(*args, **kwargs)
+
+    return teardown_cluster_mock
+
+
+@pytest.fixture
+def make_create_or_update_cluster_mock():
+    sig_create_or_update_cluster = signature(create_or_update_cluster)
+
+    def create_or_update_cluster_mock(*args, **kwargs):
+        sig_create_or_update_cluster.bind(*args, **kwargs)
+
+    return create_or_update_cluster_mock
+
+
+def test__bootstrap_config(make_bootstrap_config_mock):
+    with mock.patch(
+        "modin.experimental.cloud.rayscale._bootstrap_config",
+        make_bootstrap_config_mock,
+    ):
+        RayCluster(Provider(name="aws"))
+
+
+def test_get_head_node_ip(make_bootstrap_config_mock, make_get_head_node_ip_mock):
+    with mock.patch(
+        "modin.experimental.cloud.rayscale._bootstrap_config",
+        make_bootstrap_config_mock,
+    ):
+        ray_cluster = RayCluster(Provider(name="aws"))
+
+    with mock.patch(
+        "modin.experimental.cloud.rayscale.get_head_node_ip", make_get_head_node_ip_mock
+    ):
+        ray_cluster.ready = True
+        ray_cluster._get_connection_details()
+
+
+def test_teardown_cluster(make_bootstrap_config_mock, make_teardown_cluster_mock):
+    with mock.patch(
+        "modin.experimental.cloud.rayscale._bootstrap_config",
+        make_bootstrap_config_mock,
+    ):
+        ray_cluster = RayCluster(Provider(name="aws"))
+
+    with mock.patch(
+        "modin.experimental.cloud.rayscale.teardown_cluster", make_teardown_cluster_mock
+    ):
+        ray_cluster._destroy(wait=True)
+
+
+def test_create_or_update_cluster(
+    make_bootstrap_config_mock, make_create_or_update_cluster_mock
+):
+    with mock.patch(
+        "modin.experimental.cloud.rayscale._bootstrap_config",
+        make_bootstrap_config_mock,
+    ):
+        ray_cluster = RayCluster(Provider(name="aws"))
+
+    with mock.patch(
+        "modin.experimental.cloud.rayscale.create_or_update_cluster",
+        make_create_or_update_cluster_mock,
+    ):
+        ray_cluster._spawn(wait=True)
 
 
 @pytest.mark.parametrize(
@@ -29,9 +132,10 @@
         """
     ],
 )
-def test_update_conda_requirements(setup_commands_source):
+def test_update_conda_requirements(setup_commands_source, make_bootstrap_config_mock):
     with mock.patch(
-        "modin.experimental.cloud.rayscale._bootstrap_config", lambda config: config
+        "modin.experimental.cloud.rayscale._bootstrap_config",
+        make_bootstrap_config_mock,
     ):
         ray_cluster = RayCluster(
             Provider(name="aws"), add_conda_packages=["scikit-learn>=0.23"]


Code review comments:
diff_hunk: @@ -14,8 +14,111 @@
 import unittest.mock as mock
 import pytest
 from collections import namedtuple
+from inspect import signature
 from modin.experimental.cloud.rayscale import RayCluster
 from modin.experimental.cloud.cluster import Provider
+from ray.autoscaler.commands import (
+    create_or_update_cluster,
+    teardown_cluster,
+    get_head_node_ip,
+    _bootstrap_config,
+)
+
+
+@pytest.fixture
+def make_bootstrap_config_mock():
+    sig_bootstrap_config = signature(_bootstrap_config)
+
+    def bootstrap_config_mock(*args, **kwargs):
+        sig_bootstrap_config.bind(*args, **kwargs)
+        config = args[0]
+        config["auth"]["ssh_user"] = "modin"
+        config["auth"]["ssh_private_key"] = "X" * 20
+        return config
+
+    return bootstrap_config_mock
+
+
+@pytest.fixture
+def make_get_head_node_ip_mock():
+    sig_get_head_node_ip = signature(get_head_node_ip)
+
+    def get_head_node_ip_mock(*args, **kwargs):
+        sig_get_head_node_ip.bind(*args, **kwargs)
+        return "127.0.0.1"
+
+    return get_head_node_ip_mock
+
+
+@pytest.fixture
+def make_teardown_cluster_mock():
+    sig_teardown_cluster = signature(teardown_cluster)
+
+    def teardown_cluster_mock(*args, **kwargs):
+        sig_teardown_cluster.bind(*args, **kwargs)
+
+    return teardown_cluster_mock
+
+
+@pytest.fixture
+def make_create_or_update_cluster_mock():
+    sig_create_or_update_cluster = signature(create_or_update_cluster)
+
+    def create_or_update_cluster_mock(*args, **kwargs):
+        sig_create_or_update_cluster.bind(*args, **kwargs)
+
+    return create_or_update_cluster_mock
+
+
+def test__bootstrap_config(make_bootstrap_config_mock):
+    with mock.patch(
+        "modin.experimental.cloud.rayscale._bootstrap_config",
+        make_bootstrap_config_mock,
+    ):
+        RayCluster(Provider(name="aws"))
+
+
+def test_get_head_node_ip(make_bootstrap_config_mock, make_get_head_node_ip_mock):
+    with mock.patch(
+        "modin.experimental.cloud.rayscale._bootstrap_config",
+        make_bootstrap_config_mock,
+    ):
+        ray_cluster = RayCluster(Provider(name="aws"))
+
+    with mock.patch(
+        "modin.experimental.cloud.rayscale.get_head_node_ip", make_get_head_node_ip_mock
+    ):
+        ray_cluster.ready = True
+        ray_cluster._get_connection_details()
path: modin/experimental/cloud/test/test_cloud.py
text: @user1:
```suggestion
        details = ray_cluster._get_connection_details()
        assert details.address == "127.0.0.1"
```
?

diff_hunk: @@ -14,8 +14,111 @@
 import unittest.mock as mock
 import pytest
 from collections import namedtuple
+from inspect import signature
 from modin.experimental.cloud.rayscale import RayCluster
 from modin.experimental.cloud.cluster import Provider
+from ray.autoscaler.commands import (
+    create_or_update_cluster,
+    teardown_cluster,
+    get_head_node_ip,
+    _bootstrap_config,
+)
+
+
+@pytest.fixture
+def make_bootstrap_config_mock():
+    sig_bootstrap_config = signature(_bootstrap_config)
+
+    def bootstrap_config_mock(*args, **kwargs):
+        sig_bootstrap_config.bind(*args, **kwargs)
+        config = args[0]
+        config["auth"]["ssh_user"] = "modin"
+        config["auth"]["ssh_private_key"] = "X" * 20
+        return config
+
+    return bootstrap_config_mock
+
+
+@pytest.fixture
+def make_get_head_node_ip_mock():
+    sig_get_head_node_ip = signature(get_head_node_ip)
+
+    def get_head_node_ip_mock(*args, **kwargs):
+        sig_get_head_node_ip.bind(*args, **kwargs)
+        return "127.0.0.1"
+
+    return get_head_node_ip_mock
+
+
+@pytest.fixture
+def make_teardown_cluster_mock():
+    sig_teardown_cluster = signature(teardown_cluster)
+
+    def teardown_cluster_mock(*args, **kwargs):
+        sig_teardown_cluster.bind(*args, **kwargs)
+
+    return teardown_cluster_mock
path: modin/experimental/cloud/test/test_cloud.py
text: @user1:
```suggestion
    return lambda *args, **kw: signature(teardown_cluster).bind(*args, **kw)
```
this could look even shorter, though I'm not sure if it's readable enough

diff_hunk: @@ -14,8 +14,111 @@
 import unittest.mock as mock
 import pytest
 from collections import namedtuple
+from inspect import signature
 from modin.experimental.cloud.rayscale import RayCluster
 from modin.experimental.cloud.cluster import Provider
+from ray.autoscaler.commands import (
+    create_or_update_cluster,
+    teardown_cluster,
+    get_head_node_ip,
+    _bootstrap_config,
+)
+
+
+@pytest.fixture
+def make_bootstrap_config_mock():
+    sig_bootstrap_config = signature(_bootstrap_config)
+
+    def bootstrap_config_mock(*args, **kwargs):
+        sig_bootstrap_config.bind(*args, **kwargs)
+        config = args[0]
path: modin/experimental/cloud/test/test_cloud.py
text: @user1:
```suggestion
    def bootstrap_config_mock(config, *args, **kwargs):
        signature(_bootstrap_config).bind(config, *args, **kwargs)
```
a little bit shorter :)

diff_hunk: @@ -14,8 +14,111 @@
 import unittest.mock as mock
 import pytest
 from collections import namedtuple
+from inspect import signature
 from modin.experimental.cloud.rayscale import RayCluster
 from modin.experimental.cloud.cluster import Provider
+from ray.autoscaler.commands import (
+    create_or_update_cluster,
+    teardown_cluster,
+    get_head_node_ip,
+    _bootstrap_config,
+)
+
+
+@pytest.fixture
+def make_bootstrap_config_mock():
+    sig_bootstrap_config = signature(_bootstrap_config)
+
+    def bootstrap_config_mock(*args, **kwargs):
+        sig_bootstrap_config.bind(*args, **kwargs)
+        config = args[0]
+        config["auth"]["ssh_user"] = "modin"
+        config["auth"]["ssh_private_key"] = "X" * 20
+        return config
+
+    return bootstrap_config_mock
+
+
+@pytest.fixture
+def make_get_head_node_ip_mock():
+    sig_get_head_node_ip = signature(get_head_node_ip)
+
+    def get_head_node_ip_mock(*args, **kwargs):
+        sig_get_head_node_ip.bind(*args, **kwargs)
+        return "127.0.0.1"
+
+    return get_head_node_ip_mock
+
+
+@pytest.fixture
+def make_teardown_cluster_mock():
+    sig_teardown_cluster = signature(teardown_cluster)
+
+    def teardown_cluster_mock(*args, **kwargs):
+        sig_teardown_cluster.bind(*args, **kwargs)
+
+    return teardown_cluster_mock
+
+
+@pytest.fixture
+def make_create_or_update_cluster_mock():
+    sig_create_or_update_cluster = signature(create_or_update_cluster)
+
+    def create_or_update_cluster_mock(*args, **kwargs):
+        sig_create_or_update_cluster.bind(*args, **kwargs)
+
+    return create_or_update_cluster_mock
+
+
+def test__bootstrap_config(make_bootstrap_config_mock):
+    with mock.patch(
+        "modin.experimental.cloud.rayscale._bootstrap_config",
+        make_bootstrap_config_mock,
+    ):
+        RayCluster(Provider(name="aws"))
+
+
+def test_get_head_node_ip(make_bootstrap_config_mock, make_get_head_node_ip_mock):
+    with mock.patch(
+        "modin.experimental.cloud.rayscale._bootstrap_config",
+        make_bootstrap_config_mock,
+    ):
+        ray_cluster = RayCluster(Provider(name="aws"))
path: modin/experimental/cloud/test/test_cloud.py
text: @user1:
this looks like it should be a fixture (using another fixture), as it's repeated quite a while in tests