Avoiding having both "start_instance" and "ip" set. (#2912)

Adding tests

Co-authored-by: Camille <[email protected]>

src/ansys/mapdl/core/launcher.py

@@ -221,7 +221,7 @@ def port_in_use(port: Union[int, str], host: str = LOCALHOST) -> bool:
 
 
 def port_in_use_using_socket(port: Union[int, str], host: str) -> bool:
-    """Returns True when a port is in use at the given host using socket librry.
+    """Returns True when a port is in use at the given host using socket library.
 
     Must actually "bind" the address.  Just checking if we can create
     a socket is insufficient as it's possible to run into permission
@@ -283,7 +283,6 @@ def launch_grpc(
     ram: Optional[int] = None,
     run_location: str = None,
     port: int = MAPDL_DEFAULT_PORT,
-    ip: str = LOCALHOST,
     additional_switches: str = "",
     override: bool = True,
     timeout: int = 20,
@@ -595,7 +594,9 @@ def launch_grpc(
 
     env_vars = update_env_vars(add_env_vars, replace_env_vars)
 
-    LOG.info(f"Running in {ip}:{port} the following command: '{command}'")
+    LOG.info(
+        f"Running a local instance at port {port} the following command: '{command}'"
+    )
 
     LOG.debug("MAPDL starting in background.")
     process = subprocess.Popen(
@@ -1161,13 +1162,14 @@ def launch_mapdl(
 
     ip : bool, optional
         Used only when ``start_instance`` is ``False``. If provided,
-        it will force ``start_instance`` to be ``False``.
+        and ``start_instance`` (or its correspondent environment variable
+        ``PYMAPDL_START_INSTANCE``) is ``True`` then, an exception is raised.
         Specify the IP address of the MAPDL instance to connect to.
         You can also provide a hostname as an alternative to an IP address.
         Defaults to ``'127.0.0.1'``. You can also override the
         default behavior of this keyword argument with the
-        environment variable ``PYMAPDL_IP=<IP>``.
-        This argument has priority over the environment variable.
+        environment variable ``PYMAPDL_IP=<IP>``. If this environment variable
+        is empty, it is as it is not set.
 
     clear_on_connect : bool, optional
         Defaults to ``True``, giving you a fresh environment when
@@ -1470,8 +1472,23 @@ def launch_mapdl(
         ms_ = ", ".join([f"'{each}'" for each in kwargs.keys()])
         raise ValueError(f"The following arguments are not recognized: {ms_}")
 
-    if ip is None:
-        ip = os.environ.get("PYMAPDL_IP", None)
+    # Getting IP from env var
+    ip_env_var = os.environ.get("PYMAPDL_IP", "")
+    if ip_env_var != "":
+        ip = ip_env_var
+        LOG.debug(f"An IP ({ip}) has been set using 'PYMAPDL_IP' env var.")
+
+    ip = None if ip == "" else ip  # Making sure the variable is not empty
+
+    # Getting "start_instance" using "True" as default.
+    if (ip is not None) and (start_instance is None):
+        # An IP has been supplied. By default, 'start_instance' is equal
+        # false, unless it is set through the env vars.
+        start_instance = get_start_instance(start_instance=False)
+    else:
+        start_instance = get_start_instance(start_instance=start_instance)
+
+    LOG.debug("Using 'start_instance' equal to %s.", start_instance)
 
     if ip is None:
         if ON_WSL:
@@ -1481,8 +1498,9 @@ def launch_mapdl(
                     f"On WSL: Using the following IP address for the Windows OS host: {ip}"
                 )
             else:
-                LOG.debug(
-                    "PyMAPDL could not find the IP address of the Windows host machine."
+                raise MapdlDidNotStart(
+                    "You seems to be working from WSL.\n"
+                    "Unfortunately, PyMAPDL could not find the IP address of the Windows host machine."
                 )
 
         if not ip:
@@ -1496,10 +1514,17 @@ def launch_mapdl(
             "Because 'PYMAPDL_IP' is not None, an attempt is made to connect to"
             " a remote session ('START_INSTANCE' is set to 'False')."
         )
-        if not ON_WSL:
-            start_instance = False
-        else:
+        if ON_WSL:
             LOG.debug("On WSL: Allowing 'start_instance' and 'ip' arguments together.")
+        else:
+            if start_instance is True:
+                raise ValueError(
+                    "When providing a value for the argument 'ip', the argument "
+                    "'start_instance' cannot be 'True'.\n"
+                    "Make sure the corresponding environment variables are not setting "
+                    "those argument values.\n"
+                    "For more information visit https://github.com/ansys/pymapdl/issues/2910"
+                )
 
         ip = socket.gethostbyname(ip)  # Converting ip or hostname to ip
 
@@ -1530,10 +1555,6 @@ def launch_mapdl(
 
     version = _verify_version(version)  # return a int version or none
 
-    # Getting "start_instance" using "True" as default.
-    start_instance = get_start_instance(start_instance=start_instance)
-    LOG.debug("Using 'start_instance' equal to %s.", start_instance)
-
     if start_instance:
         # special handling when building the gallery outside of CI. This
         # creates an instance of mapdl the first time.
@@ -1745,7 +1766,6 @@ def launch_mapdl(
 
             port, actual_run_location, process = launch_grpc(
                 port=port,
-                ip=ip,
                 add_env_vars=add_env_vars,
                 replace_env_vars=replace_env_vars,
                 **start_parm,

tests/test_launcher.py

@@ -38,6 +38,7 @@
     PortAlreadyInUseByAnMAPDLInstance,
 )
 from ansys.mapdl.core.launcher import (
+    LOCALHOST,
     _check_license_argument,
     _force_smp_student_version,
     _is_ubuntu,
@@ -603,3 +604,71 @@ def test_launcher_start_instance(monkeypatch, start_instance):
         monkeypatch.delenv("PYMAPDL_START_INSTANCE")
     options = launch_mapdl(start_instance=start_instance, _debug_no_launch=True)
     assert start_instance == options["start_instance"]
+
+
+@pytest.mark.parametrize("start_instance", [None, True, False])
+@pytest.mark.parametrize("start_instance_envvar", [None, True, False])
+@pytest.mark.parametrize("ip", [None, "", "123.1.1.1"])
+@pytest.mark.parametrize("ip_envvar", [None, "", "123.1.1.1"])
+def test_ip_and_start_instance(
+    monkeypatch, start_instance, start_instance_envvar, ip, ip_envvar
+):
+    # start_instance=False
+    # start_instance_envvar=True
+    # ip=""
+    # ip_envvar="123.1.1.1"
+
+    # For more information, visit https://github.com/ansys/pymapdl/issues/2910
+    if "PYMAPDL_START_INSTANCE" in os.environ:
+        monkeypatch.delenv("PYMAPDL_START_INSTANCE")
+
+    if start_instance_envvar is not None:
+        monkeypatch.setenv("PYMAPDL_START_INSTANCE", str(start_instance_envvar))
+    if ip_envvar is not None:
+        monkeypatch.setenv("PYMAPDL_IP", str(ip_envvar))
+
+    start_instance_is_true = start_instance_envvar is True or (
+        start_instance_envvar is None and (start_instance is True)
+    )
+
+    ip_is_true = bool(ip_envvar) or (
+        (ip_envvar is None or ip_envvar == "") and bool(ip)
+    )
+
+    exceptions = start_instance_envvar is None and start_instance is None and ip_is_true
+
+    if (start_instance_is_true and ip_is_true) and not exceptions:
+        with pytest.raises(
+            ValueError,
+            match="When providing a value for the argument 'ip', the argument ",
+        ):
+            options = launch_mapdl(
+                start_instance=start_instance, ip=ip, _debug_no_launch=True
+            )
+
+        return  # Exit
+
+    options = launch_mapdl(start_instance=start_instance, ip=ip, _debug_no_launch=True)
+
+    if start_instance_envvar is True:
+        assert options["start_instance"] is True
+    elif start_instance_envvar is False:
+        assert options["start_instance"] is False
+    else:
+        if start_instance is None:
+            if ip_envvar or bool(ip):
+                assert not options["start_instance"]
+            else:
+                assert options["start_instance"]
+        elif start_instance is True:
+            assert options["start_instance"]
+        else:
+            assert not options["start_instance"]
+
+    if ip_envvar:
+        assert options["ip"] == ip_envvar
+    else:
+        if ip:
+            assert options["ip"] == ip
+        else:
+            assert options["ip"] in (LOCALHOST, "0.0.0.0")