-
Notifications
You must be signed in to change notification settings - Fork 23
feat(server): expose the server debug trace functionality #2650
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
❌ 5 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
except: | ||
warnings.warn(traceback.format_exc()) | ||
|
||
def start_debug(self, folder_path: str | Path): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should it really be at the server API? Wouldn't it be better in core, close to load plugin...
Also, I would create a context manager (similar design to licensing context manager)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I'll put tests but I wanted to validate the API first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put it at the Server level because it is a specific server you ask for its debug info.
I can make a context manager and also expose it at the ansys.dpf.core level as a global command which would take the global server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes sense here, as it is for a given server.
Just a question, I remember we had an issue in the past that was fixed for data sources when the client was in a OS (say Windows) and the server was another (say Linux), that there was some path adaptation. I think that was fixed. Is this affected by that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes sense here, as it is for a given server.
Just a question, I remember we had an issue in the past that was fixed for data sources when the client was in a OS (say Windows) and the server was another (say Linux), that there was some path adaptation. I think that was fixed. Is this affected by that?
yes you are right, I am now adding the same logic here.
@cbellot000 @rafacanton while testing I found a weird behavior: The test starts a server, sets a folder for the debug trace, then runs a forward operator. |
Some tests with 'continue-on-error: true' have failed:
|
from pathlib import Path | ||
|
||
server_instance = start_local_server(config=server_config) | ||
server_instance.start_debug(tmp_path / Path("DEBUG_TEST_")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You miss a "/" or "" at the end
@PProfizi The rule is that each new DPF server and thread has its own |
No description provided.