support launching pywb on linux by franknli · Pull Request #209 · ansys/pyworkbench

franknli · 2025-04-24T22:05:35Z

No description provided.

…nux' into fli/launch_wb_on_linux

asawas · 2025-04-25T14:43:27Z

+            # 'wmi' cannot be imported
+            self._wmi = None
+
+        if not self._wmi:


this line will be run also on Windows when it fails to load the wmi.
I think we should raise an exception after line 54 to clearly state the issue.
The current message at line 64 is confusing.

Good point. Adjusted the code so it branches based on platform. @asawas

asawas · 2025-04-25T14:46:09Z

+        ):
+            raise Exception("Invalid ANSYS version: " + version)
+
+        if host and not self._wmi:


The host could be the "localhost" or "127.0.0.1" which should be valid on Linux, right?

@asawas User is required to not specify the host argument if launching wb on local machine. So we are good here.

The parameter description does not clearly state that. Users may make such mistake since using "localhost" or "127.0.0.1" is a common practice to specify the local machine.

we could include the host value in the error message for easier troubleshooting.

Yes, that makes sense. Added host value in the message for failed connection to the host.

…nux' into fli/launch_wb_on_linux

PipKat · 2025-05-08T17:16:12Z


 ## Connect to a remote Workbench server

 Execute the steps below to link PyWorkbench with a remote session:


Suggested change

Execute the steps below to link PyWorkbench with a remote session:

Execute the following steps to link PyWorkbench with a remote session:

Google dev doc style guide says not to use placement locators like above and below but rather use preceding and following.

PipKat · 2025-05-08T17:17:37Z

+workbench.download_file("*.out")
 ```

 Use the ``{download_project_archive()}`` function to save, archive, and download the current Workbench project from the server to the client:


Should the brackets be in the function tagged entity? I've never seen this before.

PipKat · 2025-05-08T17:19:18Z

+
+            self._wmi = wmi
+            if not self._wmi:
+                raise Exception("Required Python module WMI does not exist or not loading properly")


Suggested change

raise Exception("Required Python module WMI does not exist or not loading properly")

raise Exception("Required Python module WMI does not exist or is not loading properly.")

Messages should be complete sentences and concluding punctuation.

PipKat · 2025-05-08T17:20:30Z

+                self._libc = None
+            if not self._libc:
+                raise Exception(
+                    "Required Python module ctypes does not exist or not loading libc.so.6 properly"


Suggested change

"Required Python module ctypes does not exist or not loading libc.so.6 properly"

"Required Python module ctypes do not exist or 'libc.so.6' is not loading properly."

PipKat · 2025-05-08T17:21:20Z

+        Parameters
+        ----------
+        version : str
+            Workbench version to launch. It must be a 3-digit version that is "242" or later.


Suggested change

Workbench version to launch. It must be a 3-digit version that is "242" or later.

Workbench version to launch. It must be a three-digit version, such as ``242`` or later.

PipKat · 2025-05-08T17:21:32Z

+        version : str
+            Workbench version to launch. It must be a 3-digit version that is "242" or later.
+        show_gui : bool, default: True
+            Weather to launch Workbench in UI mode.


Suggested change

Weather to launch Workbench in UI mode.

Whether to launch Workbench in UI mode.

PipKat · 2025-05-08T17:21:51Z

+        show_gui : bool, default: True
+            Weather to launch Workbench in UI mode.
+        server_workdir : str, default: None
+            Path to a writable directory on the server computer. The default is ``None``,


Suggested change

Path to a writable directory on the server computer. The default is ``None``,

Path to a writable directory on the server. The default is ``None``,

I've not seen any other Ansys library use "server computer" but only "server" instead.

PipKat · 2025-05-08T17:22:55Z

+            Server computer's name or IP address. The default is ``None`` for launching on the
+            local computer.


Suggested change

Server computer's name or IP address. The default is ``None`` for launching on the

local computer.

Name or IP address of the server. The default is ``None``, which launches

Workbench on the local computer.

Assuming it is launching Workbench but perhaps this and subsequent changes should be PyWorkbench?

PipKat · 2025-05-08T17:24:54Z

+            User's login name on the server computer. The default is ``None`` for launching on
+            the local computer.


Suggested change

User's login name on the server computer. The default is ``None`` for launching on

the local computer.

User's login name on the server. The default is ``None``, which launches

Workbench on the local computer.

PipKat · 2025-05-08T17:25:12Z

+            User's password on the server computer. The default is ``None`` for launching on
+            the local computer.


Suggested change

User's password on the server computer. The default is ``None`` for launching on

the local computer.

User's password on the server. The default is ``None``, which launches

Workbench on the local computer.

PipKat · 2025-05-08T17:25:46Z

+            If wmi service is not available for remote launching
+            If the host is given but username or password is missing.


Suggested change

If wmi service is not available for remote launching

If the host is given but username or password is missing.

If the wmi service is not available for remote launching.

If the host is given but the username or password is missing.

PipKat · 2025-05-08T17:26:03Z

+            raise Exception("Invalid ANSYS version: " + version)
+
+        if host and not self._wmi:
+            raise Exception("Launching PyWorkbench on a remote machine from Linux is not supported")


Suggested change

raise Exception("Launching PyWorkbench on a remote machine from Linux is not supported")

raise Exception("Launching PyWorkbench on a remote machine from Linux is not supported.")

PipKat · 2025-05-08T17:27:43Z

+            or version[0] not in ["2", "3"]
+            or version[2] not in ["1", "2"]
+        ):
+            raise Exception("Invalid ANSYS version: " + version)


Suggested change

raise Exception("Invalid ANSYS version: " + version)

raise Exception("Invalid Ansys version: " + version)

PipKat · 2025-05-08T17:29:38Z

+                    raise Exception(
+                        "Launching PyWorkbench on a remote machine failed. "
+                        "Make sure that the remote host is a Windows machine and that "
+                        "correct credential is used."


Suggested change

"correct credential is used."

"the correct credential is used."

Adding the article "the" to improve readability.

PipKat · 2025-05-08T17:30:11Z

+                self._process_id = process.pid
+                self._process = process
+        if successful:
+            logging.info("Workbench launched on the host with process id: " + str(self._process_id))


Suggested change

logging.info("Workbench launched on the host with process id: " + str(self._process_id))

logging.info("Workbench launched on the host with process ID: " + str(self._process_id))

PipKat · 2025-05-08T17:31:29Z

+        if not port or int(port) <= 0:
+            logging.error("Failed to retrieve the port used by Workbench service.")
+            return 0
+        logging.info("Workbench service uses port " + port)


Concluding period or add a colon after port (and before + port)?

PipKat · 2025-05-08T17:31:57Z

+            if value:
+                value = value.decode("utf-8")
+        else:
+            raise Exception("unexpected code path")


Suggested change

raise Exception("unexpected code path")

raise Exception("Code path is unexpected.")

PipKat · 2025-05-08T17:33:49Z

+                pass
+
+        """
+        # terminate related processes bottom-up


Suggested change

# terminate related processes bottom-up

# terminate related processes from the bottom up

PipKat

Performed doc review, leaving some minor comments.

asawas · 2025-04-25T14:53:16Z

+            raise Exception("Ansys installation is not found.")
+
+        args = []
+        if platform.system() == "Windows":


I think we should abstract the host machine to get rid of such if/else conditions, and self._wmi checks.
Probably, having three classes: Local Windows, Local Linux, and Remote Windows.

asawas · 2025-04-25T14:54:02Z

+        command_line = " ".join(args)
+
+        successful = False
+        if self._wmi:


code can be simpler via host machine abstraction

asawas · 2025-04-25T14:56:06Z

+        self._process_id = -1
+        self._process = None
+
+    def __collect_process_tree(self):


This is a Windows only method. Abstracting the host machine will move it to the appropriate class.

asawas · 2025-05-08T17:56:03Z

            or version[2] not in ["1", "2"]
        ):
-            raise Exception("Invalid ANSYS version: " + version)
+            raise Exception("Invalid Ansys version: " + version)


This version validation logic does not enforce the version to be >= 242

asawas · 2025-05-08T18:01:18Z

+        ):
+            raise Exception("Invalid ANSYS version: " + version)
+
+        if host and not self._wmi:


The parameter description does not clearly state that. Users may make such mistake since using "localhost" or "127.0.0.1" is a common practice to specify the local machine.

asawas · 2025-05-08T18:03:18Z

+        ):
+            raise Exception("Invalid ANSYS version: " + version)
+
+        if host and not self._wmi:


we could include the host value in the error message for easier troubleshooting.

asawas · 2025-05-08T18:08:10Z

+                    p.Terminate()
+                except Exception:
+                    pass
+        """


Remove the commented code.

also the unused method __collect_process_tree should be removed

support launching pywb on linux

1aa3729

franknli requested a review from a team as a code owner April 24, 2025 22:05

franknli requested a review from asawas April 24, 2025 22:05

pyansys-ci-bot and others added 7 commits April 24, 2025 22:06

chore: adding changelog file 209.miscellaneous.md [dependabot-skip]

af9f530

fix code style

779faef

Merge remote-tracking branch 'refs/remotes/origin/fli/launch_wb_on_li…

db9f44b

…nux' into fli/launch_wb_on_linux

fix code style

291afdb

fix code style

dcaafbc

code style

9b327c6

code style

2ee5c92

asawas reviewed Apr 25, 2025

View reviewed changes

franknli added 4 commits April 29, 2025 14:52

change per code review

392f722

fix launcher for Linux

9d9669f

fix code style

3fe3677

update doc

ed7b186

franknli requested a review from PipKat as a code owner May 7, 2025 17:40

github-actions Bot added the documentation Improvements or additions to documentation label May 7, 2025

pyansys-ci-bot and others added 3 commits May 7, 2025 17:41

chore: adding changelog file 209.documentation.md [dependabot-skip]

7742f0b

fix cheatsheet

7d0204c

Merge remote-tracking branch 'refs/remotes/origin/fli/launch_wb_on_li…

34c8877

…nux' into fli/launch_wb_on_linux

PipKat reviewed May 8, 2025

View reviewed changes

PipKat approved these changes May 8, 2025

View reviewed changes

franknli added 2 commits May 8, 2025 18:46

change per doc review

0fb7063

fix code style

ad8e335

asawas reviewed May 8, 2025

View reviewed changes

franknli added 6 commits May 8, 2025 21:55

fix code style

a8d1129

change per code review

b8ebe73

fix code style

aa2d93a

disable cheatsheet for now

b812b44

update per code review

295bcd2

update summary doc

bcf05bb

asawas approved these changes May 14, 2025

View reviewed changes

change per code review

ff881be

asawas approved these changes May 14, 2025

View reviewed changes

franknli added 2 commits May 14, 2025 22:43

merge main

e778889

update pywb version used in unit tests

f80c9bc

github-actions Bot added the testing Anything related to tests label May 15, 2025

franknli merged commit 3818a1a into main May 15, 2025

franknli deleted the fli/launch_wb_on_linux branch May 15, 2025 03:10


		## Connect to a remote Workbench server

		Execute the steps below to link PyWorkbench with a remote session:

	Execute the steps below to link PyWorkbench with a remote session:
	Execute the following steps to link PyWorkbench with a remote session:

	raise Exception("Required Python module WMI does not exist or not loading properly")
	raise Exception("Required Python module WMI does not exist or is not loading properly.")

	"Required Python module ctypes does not exist or not loading libc.so.6 properly"
	"Required Python module ctypes do not exist or 'libc.so.6' is not loading properly."

	Workbench version to launch. It must be a 3-digit version that is "242" or later.
	Workbench version to launch. It must be a three-digit version, such as ``242`` or later.

	Weather to launch Workbench in UI mode.
	Whether to launch Workbench in UI mode.

	Path to a writable directory on the server computer. The default is ``None``,
	Path to a writable directory on the server. The default is ``None``,

		Server computer's name or IP address. The default is ``None`` for launching on the
		local computer.

		User's login name on the server computer. The default is ``None`` for launching on
		the local computer.

		User's password on the server computer. The default is ``None`` for launching on
		the local computer.

		If wmi service is not available for remote launching
		If the host is given but username or password is missing.

	raise Exception("Launching PyWorkbench on a remote machine from Linux is not supported")
	raise Exception("Launching PyWorkbench on a remote machine from Linux is not supported.")

	raise Exception("Invalid ANSYS version: " + version)
	raise Exception("Invalid Ansys version: " + version)

	"correct credential is used."
	"the correct credential is used."

	logging.info("Workbench launched on the host with process id: " + str(self._process_id))
	logging.info("Workbench launched on the host with process ID: " + str(self._process_id))

	raise Exception("unexpected code path")
	raise Exception("Code path is unexpected.")

	# terminate related processes bottom-up
	# terminate related processes from the bottom up

Uh oh!

Conversation

franknli commented Apr 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PipKat May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PipKat May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PipKat left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

PipKat May 8, 2025 •

edited

Loading

PipKat May 8, 2025 •

edited

Loading