-
Notifications
You must be signed in to change notification settings - Fork 13
CHORE: Ansys version warning vs grpc #1294
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
# Conflicts: # src/pyedb/grpc/database/layout/layout.py
# Conflicts: # src/pyedb/configuration/cfg_setup.py
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1294 +/- ##
==========================================
- Coverage 75.79% 75.76% -0.03%
==========================================
Files 163 163
Lines 23908 23921 +13
==========================================
+ Hits 18120 18123 +3
- Misses 5788 5798 +10 🚀 New features to boost your workflow:
|
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.
LGTM, I left two minor comments
warnings.warn("Your ANSYS AEDT version is eligible to gRPC version", UserWarning) | ||
warnings.warn("gRPC is not enabled by default, however you might consider using this version", UserWarning) | ||
warnings.warn("Please check online documentation for more information", UserWarning) | ||
warnings.warn("gRPC is offering better stability and compatibility with Linux", UserWarning) | ||
warnings.warn("Note: current DotNet PyEDB will be deprecated starting release 2026.1", UserWarning) | ||
warnings.warn("New feature will only be implemented in gRPC version", UserWarning) |
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.
warnings.warn("Your ANSYS AEDT version is eligible to gRPC version", UserWarning) | |
warnings.warn("gRPC is not enabled by default, however you might consider using this version", UserWarning) | |
warnings.warn("Please check online documentation for more information", UserWarning) | |
warnings.warn("gRPC is offering better stability and compatibility with Linux", UserWarning) | |
warnings.warn("Note: current DotNet PyEDB will be deprecated starting release 2026.1", UserWarning) | |
warnings.warn("New feature will only be implemented in gRPC version", UserWarning) | |
warnings.warn("Your ANSYS AEDT version is eligible to gRPC version", UserWarning) | |
warnings.warn("gRPC is not enabled by default, however you might consider using this version", UserWarning) | |
warnings.warn("Please check online documentation for more information", UserWarning) | |
warnings.warn("gRPC is offering better stability and compatibility with Linux", UserWarning) | |
warnings.warn("Note: current DotNet PyEDB will be deprecated starting release 2026.1", UserWarning) | |
warnings.warn("New feature will only be implemented in gRPC version", UserWarning) |
Can you shorten the message and provide the link to the gRPC documentation ?
if grpc: | ||
warnings.warn( | ||
f"gRPC flag was enabled however your ANSYS AEDT version {edbversion} is not compatible", UserWarning | ||
) | ||
warnings.warn( | ||
f"Disabling gRPC, you might consider upgrading your ANSYS version for better experience", UserWarning | ||
) | ||
grpc = False |
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.
Any reason to switch grpc value instead of directly raising an error ?
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 prefer breaking and warn user ? ok for me too
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 would avoid changing the input provided by the user yeah. I think that, by default, grpc
is equal to False
so if it's True
then the user should strongly be notified about this limitation. Otherwise you might endup with people missing the warning and thinking that gRPC didn't change anything compared to the previous approach
This PR is adding deprecation warning with respect to PyEDB DotNet vs gRPC.