Skip to content

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

Open
wants to merge 79 commits into
base: main
Choose a base branch
from

Conversation

svandenb-dev
Copy link
Collaborator

This PR is adding deprecation warning with respect to PyEDB DotNet vs gRPC.

# Conflicts:
#	src/pyedb/grpc/database/layout/layout.py
# Conflicts:
#	src/pyedb/configuration/cfg_setup.py
@github-actions github-actions bot added the testing Anything related to testing label Jun 17, 2025
@codecov-commenter
Copy link

codecov-commenter commented Jun 17, 2025

Codecov Report

Attention: Patch coverage is 23.07692% with 10 lines in your changes missing coverage. Please review.

Project coverage is 75.76%. Comparing base (970c119) to head (310730d).

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:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@svandenb-dev svandenb-dev requested a review from gkorompi June 20, 2025 06:58
Copy link
Collaborator

@SMoraisAnsys SMoraisAnsys left a 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

Comment on lines +120 to +125
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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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 ?

Comment on lines +127 to +134
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
Copy link
Collaborator

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 ?

Copy link
Collaborator Author

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

Copy link
Collaborator

@SMoraisAnsys SMoraisAnsys Jun 20, 2025

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Anything related to testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants