refactor: avoid deleting server-info file #5242
Conversation
….com/ansys/pyfluent into refactor/couple_issues_launch_fluent
….com/ansys/pyfluent into refactor/couple_issues_launch_fluent
…ournal_file condition
….com/ansys/pyfluent into refactor/couple_issues_launch_fluent
Noted merge it
Up to standards ✅🟢 Issues
|
Thanks @seanpearsonuk for pointing this. I have updated the PR description. Kindly have a look |
Thanks @mainakk that file is missed due to some conflicts. I have updated it, kindly have a look. |
| if ( | ||
| remove_server_info_file | ||
| and cleanup_on_exit | ||
| and host_server_info_file.exists() |
There was a problem hiding this comment.
unlink() checks exists() - no need to duplicate the call.
There was a problem hiding this comment.
@seanpearsonuk I think we should check for existence before calling unlink.
Python 3.14.5 (tags/v3.14.5:5607950, May 10 2026, 10:43:50) [MSC v.1944 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
Ctrl click to launch VS Code Native REPL
>>> from pathlib import Path
>>> Path("xyz.abc").unlink()
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
Path("xyz.abc").unlink()
~~~~~~~~~~~~~~~~~~~~~~^^
File "C:\Users\mkundu\AppData\Local\Programs\Python\Python314\Lib\pathlib\__init__.py", line 1042, in unlink
os.unlink(self)
~~~~~~~~~^^^^^^
FileNotFoundError: [WinError 2] The system cannot find the file specified: 'xyz.abc'
>>> Path("xyz.abc").exists()
False
There was a problem hiding this comment.
@mkundu1 @seanpearsonuk file_path.unlink(missing_ok=True)
This would be the correct approach ?
There was a problem hiding this comment.
Thanks and Noted.
| ) as tmp_file: | ||
| false_path = Path(tmp_file.name) | ||
| tmp_file.write(b"cleanup_false_test") |
There was a problem hiding this comment.
tmp_file already has the same API. The way to reason when you see code like this is that it would be surprising if Python were forcing you to write this code. The same thought process applies for exists + unlink - it's simply that certain patterns stick out as being surprising.
Noting that there are more of these elsewhere in this module.
| and host_server_info_file.exists() | ||
| ): | ||
| host_server_info_file.unlink() | ||
| if remove_server_info_file and cleanup_on_exit: |
There was a problem hiding this comment.
This logic is wrong. These two variables (one is from the config dict) are saying exactly the same thing (one needs to be deprecated). This logic overlooks that fact.
IF either is False AND either is True -> raise (cannot proceed on contradictory input)
ELSE IF at least one is True -> unlink()
Make sure to fail early.
| @@ -379,6 +379,4 @@ def __call__( | |||
| raise LaunchFluentError(self._launch_cmd) from ex | |||
| finally: | |||
| if self.argvals.get("cleanup_on_exit", True): | |||
There was a problem hiding this comment.
Just a comment on what is presumably the pre-existing status here. I find this odd. If cleanup_on_exit is an optional argument, it's intuitive to think that if I don't pass that argument then nothing gets cleaned up, but it's actually the other way around. As such, it feels like an optional argument like preserve_info_file: bool (defaulting to False) makes more sense. Then if I want to preserve the file, I explicitly request it. We have it the other way round where I might ignore cleanup_on_exit because I don't want to do that but then it just happens anyway.
| @@ -928,205 +801,183 @@ def test_idle_timeout(monkeypatch): | |||
| ) | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Do we know the history of this class? Why are PyFluent tests checking the mechanics of Python's documented file handling functionality?
We should just delete this - and anything else with a similar smell.
| Additional command-line arguments for Fluent, formatted as they would be on the command line. | ||
| env : dict[str, str], optional | ||
| A mapping for modifying environment variables in Fluent. Defaults to ``None``. | ||
| cleanup_on_exit : bool, optional |
There was a problem hiding this comment.
How shall I call the launcher now if I don't want to shut down the Fluent process on session.exit()?
There was a problem hiding this comment.
@mainakk If I understood properly your point then we can preserve the server info file and avoid cleanup on session.exit() , call the launcher with
session = pyfluent.launch_fluent(preserve_info_file = True)
session.exit()
Kindly let me know, if I misunderstood your question or any further query or any change needed.
There was a problem hiding this comment.
@mayankansys is responding to my comment above but that comment does not request any changes. If we agree to follow up on that idea, we would create a separate issue.
- Remove _cleanup_on_exit_to_preserve_info_file_converter function - Revert start_fluent_container() to use cleanup_on_exit parameter (default True) - Update DockerLauncher and StandaloneLauncher to use cleanup_on_exit - Remove deprecation decorators from launcher classes - Simplify parameter logic: cleanup_on_exit directly maps to remove_server_info_file - Keep code style fixes from Seanpearsonuk review (unlink(missing_ok=True)) - Supports issue #5145: preserve server-info file when cleanup_on_exit=False
5 participants
Context
When using
launch_fluent(..., cleanup_on_exit=False), the server-info file was being deleted during cleanup or workflow execution even though the user explicitly requested to avoid cleanup. This caused avoidable issues when that file was still needed for triaging or debugging, particularly for troubleshooting client-server connection failures related to port setup. The server-info file is essential for understanding how a Fluent process was initialized and should be preserved when cleanup is disabled.Change Summary
This change refactors the cleanup logic to respect the
cleanup_on_exit=Falseparameter and avoid deleting the server-info file when requested. The refactoring updates:Rationale
Preserving the server-info file when
cleanup_on_exit=Falseis critical for debugging and issue triaging. This file contains port configuration and connection metadata that becomes inaccessible once deleted, making it impossible to diagnose connection failures after they occur. When Fluent is manually exited after being launched withcleanup_on_exit=False, the Fluent process itself will handle cleanup. PyFluent should not preemptively delete resources the user explicitly opted to preserve.Impact
cleanup_on_exitparameter consistently