feat: database module improvements (#3320)

* feat: adding ip argument

* feat: adding port start option

* refactor: 'run_as_routine' to make it use more mapdl attributes.\nDeleting 'WithinBeginLevel' context

* chore: adding changelog file 3320.miscellaneous.md

* fix: test

* fix: test

* fix: test

* feat: creating '_enter_routine' and moving out code. Refactoring

* fix: missing import

* feat: allowing routines starting with /.

---------

Co-authored-by: pyansys-ci-bot <[email protected]>

doc/changelog.d/3320.miscellaneous.md

@@ -0,0 +1 @@
+feat: database module improvements

src/ansys/mapdl/core/database/database.py

@@ -25,6 +25,7 @@
 from functools import wraps
 import os
 import time
+from typing import Optional
 from warnings import warn
 import weakref
 
@@ -38,24 +39,7 @@
 MINIMUM_MAPDL_VERSION = "21.1"
 FAILING_DATABASE_MAPDL = ["24.1", "24.2"]
 
-
-class WithinBeginLevel:
-    """Context manager to run MAPDL within the being level."""
-
-    def __init__(self, mapdl):
-        """Initialize this context manager."""
-        self._mapdl = mapdl
-
-    def __enter__(self):
-        """Enter the begin level and cache the current routine."""
-        self._mapdl._cache_routine()
-        if "BEGIN" not in self._mapdl._cached_routine.upper():
-            self._mapdl.finish()
-
-    def __exit__(self, *args, **kwargs):
-        """Exit the begin level and reload the previous routine."""
-        if "BEGIN" not in self._mapdl._cached_routine.upper():
-            self._mapdl._resume_routine()
+DEFAULT_DB_PORT = 50055
 
 
 def check_mapdl_db_is_alive(function):
@@ -160,7 +144,7 @@ def _mapdl(self):
         """Return the weakly referenced instance of mapdl."""
         return self._mapdl_weakref()
 
-    def _start(self) -> int:
+    def _start(self, port: Optional[int] = None) -> int:
         """
         Lower level start of the database server.
 
@@ -170,38 +154,39 @@ def _start(self) -> int:
             Port of the database server.
 
         """
-        self._mapdl._log.debug("Starting MAPDL server")
-
         # database server must be run from the "BEGIN" level
+        self._mapdl._log.debug("Starting MAPDL server")
         self._mapdl._cache_routine()
-        with WithinBeginLevel(self._mapdl):
-            self._mapdl.run("/DBS,SERVER,START")
+        with self._mapdl.run_as_routine("Begin level"):
+            self._mapdl.run(f"/DBS,SERVER,START,{port}")
 
-        # Scan the DBServer.info file to get the Port Number
+        if not port:
+            # We do not know which port has started with
+            # Scan the DBServer.info file to get the Port Number
 
-        # Default is 50055
-        # wait for start
-        tstart = time.time()
-        timeout = 1
-        status = self._mapdl._download_as_raw("DBServer.info").decode()
-        while status == "":  # pragma: no cover
+            self._mapdl._log.debug("Downloading 'DBServer' file.")
+            tstart = time.time()
+            timeout = 1
             status = self._mapdl._download_as_raw("DBServer.info").decode()
-            time.sleep(0.05)
-            if time.time() - tstart > timeout:
-                raise TimeoutError(
-                    f"Unable to start database server in {timeout} second(s)"
+            while status == "":  # pragma: no cover
+                status = self._mapdl._download_as_raw("DBServer.info").decode()
+                time.sleep(0.05)
+                if time.time() - tstart > timeout:
+                    raise TimeoutError(
+                        f"Unable to start database server in {timeout} second(s). DBServer not received."
+                    )
+
+            self._mapdl._log.debug("Downloading 'DBServer' file.")
+            try:
+                # expected of the form 'Port : 50055'
+                port = int(status.split(":")[1])
+            except Exception as e:  # pragma: no cover
+                self._mapdl._log.error(
+                    "Unable to read port number from '%s' due to\n%s",
+                    status,
+                    str(e),
                 )
-
-        try:
-            # expected of the form 'Port : 50055'
-            port = int(status.split(":")[1])
-        except Exception as e:  # pragma: no cover
-            self._mapdl._log.error(
-                "Unable to read port number from '%s' due to\n%s",
-                status,
-                str(e),
-            )
-            port = 50055
+                port = DEFAULT_DB_PORT
 
         self._mapdl._log.debug("MAPDL database server started on port %d", port)
         return port
@@ -211,7 +196,7 @@ def active(self) -> bool:
         """Return if the database server is active."""
         return "NOT" not in self._status()
 
-    def start(self, timeout=10):
+    def start(self, port: Optional[int] = None, timeout: int = 10):
         """
         Start the gRPC MAPDL database server.
 
@@ -262,19 +247,27 @@ def start(self, timeout=10):
         self._mapdl._log.debug("MAPDL DB server running: %s", str(is_running))
         if is_running:
             return
-        db_port = self._start()
-
-        self._ip = self._mapdl._ip
 
         # permit overriding db_port via env var for CI
-        if "PYMAPDL_DB_PORT" in os.environ:
-            db_port_str = os.environ.get("PYMAPDL_DB_PORT")
-            try:
-                db_port = int(db_port_str)
-            except ValueError:  # pragma: no cover
-                raise ValueError(
-                    f"Invalid port '{db_port_str}' specified in the env var PYMAPDL_DB_PORT"
+        if not port:
+            if (
+                "PYMAPDL_DB_PORT" in os.environ
+                and os.environ.get("PYMAPDL_DB_PORT").isdigit()
+            ):
+                db_port_str = int(os.environ.get("PYMAPDL_DB_PORT"))
+                self._mapdl._log.debug(
+                    f"Setting DB port from 'PYMAPDL_DB_PORT' env var: {db_port_str}"
+                )
+            else:
+                self._mapdl._log.debug(
+                    f"Setting default DB port ('{DEFAULT_DB_PORT}') because no port was input or the env var 'PYMAPDL_DB_PORT' is not correctly set."
                 )
+                port = DEFAULT_DB_PORT
+
+        db_port = self._start(port=port)
+
+        if not self._ip:
+            self._ip = self._mapdl.ip
 
         self._server = {"ip": self._ip, "port": db_port}
         self._channel_str = f"{self._ip}:{db_port}"
@@ -295,13 +288,13 @@ def start(self, timeout=10):
 
         if not self._state._matured:  # pragma: no cover
             raise MapdlConnectionError(
-                "Unable to establish connection to MAPDL database server"
+                f"Unable to establish connection to MAPDL database server {self._channel_str}"
             )
         self._mapdl._log.debug("Established connection to MAPDL database server")
 
     def _stop(self):
         """Stop the MAPDL database service."""
-        with WithinBeginLevel(self._mapdl):
+        with self._mapdl.run_as_routine("Begin level"):
             return self._mapdl.run("/DBS,SERVER,STOP")
 
     def stop(self):
@@ -338,7 +331,7 @@ def _status(self):
          DB Server is NOT currently running ..
         """
         # Need to use the health check here
-        with WithinBeginLevel(self._mapdl):
+        with self._mapdl.run_as_routine("Begin level"):
             return self._mapdl.run("/DBS,SERVER,STATUS")
 
     def load(self, fname, progress_bar=False):

src/ansys/mapdl/core/mapdl_core.py

@@ -1443,29 +1443,27 @@ class _RetainRoutine:
 
         def __init__(self, parent, routine):
             self._parent = weakref.ref(parent)
-
-            # check the routine is valid since we're muting the output
-            check_valid_routine(routine)
             self._requested_routine = routine
 
         def __enter__(self):
             """Store the current routine and enter the requested routine."""
-            self._cached_routine = self._parent().parameters.routine
-            self._parent()._log.debug("Caching routine %s", self._cached_routine)
-            if self._requested_routine.lower() != self._cached_routine.lower():
-                self._enter_routine(self._requested_routine)
+            self._parent()._cache_routine()
+            self._parent()._log.debug(f"Caching routine {self._cached_routine}")
+
+            if (
+                self._requested_routine.lower().strip()
+                != self._cached_routine.lower().strip()
+            ):
+                self._parent()._enter_routine(self._requested_routine)
 
         def __exit__(self, *args):
             """Restore the original routine."""
-            self._parent()._log.debug("Restoring routine %s", self._cached_routine)
-            self._enter_routine(self._cached_routine)
+            self._parent()._log.debug(f"Restoring routine '{self._cached_routine}'")
+            self._parent()._resume_routine()
 
-        def _enter_routine(self, routine):
-            """Enter a routine."""
-            if routine.lower() == "begin level":
-                self._parent().finish(mute=True)
-            else:
-                self._parent().run(f"/{routine}", mute=True)
+        @property
+        def _cached_routine(self):
+            return self._parent()._cached_routine
 
     def run_as_routine(self, routine):
         """
@@ -1707,17 +1705,26 @@ def open_gui(self, include_result=None, inplace=None):  # pragma: no cover
         # restore remove tmp state
         self._remove_tmp = remove_tmp
 
+    def _enter_routine(self, routine):
+        # check the routine is valid since we're muting the output
+        check_valid_routine(routine)
+
+        if routine.lower() in ["begin level", "finish"]:
+            self.finish(mute=True)
+        else:
+            if not routine.startswith("/"):
+                routine = f"/{routine}"
+
+            self.run(f"{routine}", mute=True)
+
     def _cache_routine(self):
         """Cache the current routine."""
         self._cached_routine = self.parameters.routine
 
     def _resume_routine(self):
         """Resume the cached routine."""
         if self._cached_routine is not None:
-            if "BEGIN" not in self._cached_routine:
-                self.run(f"/{self._cached_routine}", mute=True)
-            else:
-                self.finish(mute=True)
+            self._enter_routine(self._cached_routine)
             self._cached_routine = None
 
     def _launch(self, *args, **kwargs):  # pragma: no cover

src/ansys/mapdl/core/misc.py

@@ -111,6 +111,9 @@ def check_valid_routine(routine):
         Raised when a routine is invalid.
 
     """
+    if routine.lower().startswith("/"):
+        routine = routine[1:]
+
     if routine.lower().startswith("begin"):
         return True
     if not hasattr(ROUTINES, routine.upper()):

tests/test_misc.py

@@ -372,12 +372,17 @@ def myotherfun2(self):
         assert myclass.myotherfun2 is None
 
 
-def test_check_valid_routine():
-    assert check_valid_routine("prep7")
-    assert check_valid_routine("PREP7")
-    assert check_valid_routine("begin level")
+@pytest.mark.parametrize(
+    "routine", ["prep7", "PREP7", "/PREP7", "begin level", "BEGIN LEVEL"]
+)
+def test_check_valid_routine(routine):
+    assert check_valid_routine(routine)
+
+
+@pytest.mark.parametrize("routine", ["invalid", "invalid routine", "prep78"])
+def test_check_valid_routine_invalid(routine):
     with pytest.raises(ValueError, match="Invalid routine"):
-        check_valid_routine("invalid")
+        check_valid_routine(routine)
 
 
 @requires("local")