feat: support both grpc package name versions by Acohen-work · Pull Request #100 · ansys/pycfx

Acohen-work · 2026-05-21T20:07:22Z

A low-to-no compromise version of this change set is proposed here where the package namespace will be determined by the version of CFX being run.

…nsys/pycfx into feat/change_grpc_package_names

Co-authored-by: pyansys-ci-bot <92810346+pyansys-ci-bot@users.noreply.github.com>

codecov-commenter · 2026-05-21T22:17:13Z

Codecov Report

❌ Patch coverage is 81.18812% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.48%. Comparing base (d1e08d0) to head (130d6d9).

Files with missing lines	Patch %	Lines
src/ansys/cfx/core/launcher/launcher.py	28.57%	5 Missing ⚠️
src/ansys/cfx/core/services/settings.py	87.09%	4 Missing ⚠️
src/ansys/cfx/core/services/health_check.py	75.00%	3 Missing ⚠️
src/ansys/cfx/core/services/batch_ops.py	86.66%	2 Missing ⚠️
src/ansys/cfx/core/services/engine_eval.py	83.33%	2 Missing ⚠️
src/ansys/cfx/core/services/events.py	80.00%	1 Missing ⚠️
...ys/cfx/core/streaming_services/events_streaming.py	85.71%	1 Missing ⚠️
src/ansys/cfx/core/utils/cfx_version.py	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #100      +/-   ##
==========================================
- Coverage   81.67%   81.48%   -0.20%     
==========================================
  Files          42       42              
  Lines        3563     3607      +44     
==========================================
+ Hits         2910     2939      +29     
- Misses        653      668      +15

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

anshforkel · 2026-05-22T09:41:18Z



-def create_launcher(cfx_launch_mode: LaunchMode = None, **kwargs):
+def create_launcher(version, cfx_launch_mode: LaunchMode = None, **kwargs):


I'm concerned that users will be required to specify the Ansys release version, which seems redundant in most cases. Basically, a user would need duplicate specification of the Ansys release: its location (which implies the release) and then again the release version.

I understand we cannot query the engine at this point, because the API version v0/v1 must be decided before connecting to the engine. However, there are other means, e.g., the AWP_ROOTxxx environment variable provides the release and the path also has the version in it. I'd prefer to provide default logic, possibly with an optional(!) argument allowing the user to override the API version.

anshforkel · 2026-05-22T09:42:34Z

        self._metadata = metadata

-    def execute(self, request: batch_ops_pb2.ExecuteRequest) -> batch_ops_pb2.ExecuteResponse:
+    def execute(self, request: "batch_ops_pb2.ExecuteRequest") -> "batch_ops_pb2.ExecuteResponse":


Can you please elaborate on this change? I want to learn.

yes, the symbol is not defined until the function is called, so this is a sort of forward-declaration

anshforkel · 2026-05-22T09:45:30Z

+    # start-timeout is intentionally provided to be 0s for the connection to fail
    with pytest.raises(Exception):
-        pycfx.PreProcessing.from_install(start_timeout=1)
+        pycfx.PreProcessing.from_install(start_timeout=0)


Did you encounter issues with the 1 [s] timeout?

yes, they started up in less than 1s :)

anshforkel · 2026-05-22T09:46:05Z

-    assert len(param_state) == 1
-    param_state_file_path = param_state[0].replace("\\", "/")
+    assert len(param_state) == 51
+    param_state_file_path = param_state.replace("\\", "/")


Did the test fail before?

yes, and this will likely need to be changed to checking for string content rather than length

swilson-work · 2026-05-22T09:54:56Z

-    assert len(param_state) == 1
-    param_state_file_path = param_state[0].replace("\\", "/")
+    assert len(param_state) == 51
+    param_state_file_path = param_state.replace("\\", "/")


This needs investigation, as it suggests that something has changed since the test was last run. In any case, if get_state() is now returning a string and not a list, we should not be testing the length of the path as the full path could vary e.g. across platforms or due to a configuration change.

swilson-work · 2026-05-22T09:56:40Z

            BatchInterceptor(),
        )
-        self.__stub = EngineEvalGrpcModule.EngineEvalStub(intercept_channel)
+        print(f"EngineEvalService initialized with version {version}")


@Acohen-work This needs removing or logging as debug information.

Copilot

Pull request overview

This PR aims to support both legacy (ansys.api.cfx.v0) and newer (ansys.api.cfx.v1) gRPC package namespaces by selecting the proto/stub module based on the CFX version being used.

Changes:

Route multiple gRPC service imports (events, batch ops, engine eval, health check, settings) to v0 vs v1 based on a CFXVersion threshold.
Thread a CFX version value through launcher/connection/session paths (including connection properties) to enable version-based routing.
Update tests and codegen plumbing to pass/accept version information and handle changed behavior.

Reviewed changes

Copilot reviewed 20 out of 21 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
tests/test_session.py	Passes `version` through `from_connection` kwargs.
tests/test_pre_settings_mesh.py	Updates expectations for mesh file path state/value (currently brittle).
tests/test_launcher.py	Makes connection-failure tests use `start_timeout=0`.
src/ansys/cfx/core/utils/cfx_version.py	Accepts `v`/`V`-prefixed version strings when parsing.
src/ansys/cfx/core/streaming_services/events_streaming.py	Selects events proto module per version and stores it on the manager.
src/ansys/cfx/core/session.py	Propagates version to created services and requires version when creating sessions from server info files.
src/ansys/cfx/core/session_utilities.py	Adds optional `version` parameter to `from_connection` (docs incomplete).
src/ansys/cfx/core/services/settings.py	Chooses settings protos/stubs per version and updates request construction accordingly.
src/ansys/cfx/core/services/health_check.py	Switches health-check protos/stubs per version (currently includes stdout prints).
src/ansys/cfx/core/services/events.py	Switches events stub module per version (contains leftover commented debug).
src/ansys/cfx/core/services/engine_eval.py	Switches engine-eval protos/stubs per version (currently includes stdout prints).
src/ansys/cfx/core/services/batch_ops.py	Switches batch-ops protos/stubs per version and threads them through ops.
src/ansys/cfx/core/launcher/standalone_launcher.py	Passes version into session creation and adjusts WNUA filtering logic (currently unsafe for `None`).
src/ansys/cfx/core/launcher/launcher.py	Changes launcher factory signature and threads version through launch/connect entry points.
src/ansys/cfx/core/launcher/container_launcher.py	Adjusts container image tag handling and passes version into `CFXConnection` (currently unsafe for `None`/enum).
src/ansys/cfx/core/launcher/cfx_container.py	Ensures Docker tags are `v`-prefixed (non-idiomatic check).
src/ansys/cfx/core/cfx_connection.py	Makes `version` a required constructor argument and stores it in connection properties.
Makefile	Passes `CFX_IMAGE_TAG` to codegen as `--ansys-version`.
doc/changelog.d/100.added.md	Adds changelog entry for dual gRPC package support.
coverage.svg	Updates displayed coverage percent (currently inconsistent within SVG).
codegen/allapigen.py	Plumbs `ansys_version` through to `launch_cfx(product_version=...)`.

Comments suppressed due to low confidence (1)

src/ansys/cfx/core/launcher/launcher.py:264

connect_to_cfx() defaults version to None but passes it into CFXConnection, and downstream services do version >= CFXVersion.v271, which will raise for None. Make version required here or implement auto-detection/fallback before constructing CFXConnection.

    version: Optional[CFXVersion] = None,
) -> Union[PreProcessing, Solver, PostProcessing]:
    """Connect to an existing CFX server instance.

    Parameters

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


+        cfx_version = CFXVersion(self._cfx_version)
        filter_wnua = (
-            is_windows() and self._cfx_version and CFXVersion(self._cfx_version) < CFXVersion.v261
+            is_windows() and self._cfx_version and cfx_version < CFXVersion.v261
        ) and os.getenv("PYCFX_SHOW_ALL_WNUA_MESSAGES", "unset") != "1"


            BatchInterceptor(),
        )
-        self._stub = HealthCheckGrpcModule.HealthStub(intercept_channel)
+        print(f"HealthCheckService initialized with version {version}")


+    ):
        """Initialize an instance of the ``EventsService`` class."""
+
+        # print(f"EventsService initialized with version {version}")


            image_tag = os.getenv("CFX_IMAGE_TAG", "v26.1.0")
        if not image_name:
            image_name = os.getenv("CFX_IMAGE_NAME", "ghcr.io/ansys/pycfx")
+        if image_tag.startswith("v") == False:


anshforkel · 2026-05-22T11:05:36Z

        self.container_dict["cfx_cmd"] = self.mode.value[0].get_cmd_name()
        if self.product_version:
-            self.container_dict["image_tag"] = f"v{self.product_version}"
+            self.container_dict["image_tag"] = f"{self.product_version}"


@Acohen-work It seems the semantic of product_version has changed, i.e. whether it includes the leading 'v' or not. Please can you elaborate?

+def create_launcher(version, cfx_launch_mode: LaunchMode = None, **kwargs):
    """Use a factory function to create a launcher for supported launch modes.

    Parameters


        )
-        self.__stub = EngineEvalGrpcModule.EngineEvalStub(intercept_channel)
+        print(f"EngineEvalService initialized with version {version}")
+        if version >= CFXVersion.v271:


    param_state = pypre.setup.mesh["MeshFromDefFile"].file_path.get_state()
-    assert len(param_state) == 1
-    param_state_file_path = param_state[0].replace("\\", "/")
+    assert len(param_state) == 51
+    param_state_file_path = param_state.replace("\\", "/")
    assert param_state_file_path.endswith("data/StaticMixer.def")


    param_value = pypre.setup.mesh["MeshFromDefFile"].file_path()
-    assert len(param_value) == 1
-    param_value_file_path = param_value[0].replace("\\", "/")
+    assert len(param_value) == 51
+    param_value_file_path = param_value.replace("\\", "/")


            Whether to connect CFX's gRPC server in insecure mode without TLS.
            This mode is not recommended. For more information on the implications
            and usage of insecure mode, see the CFX documentation.
+        version :  CFXVersion, default: None


swilson-work · 2026-05-22T09:57:05Z

            BatchInterceptor(),
        )
-        self._stub = HealthCheckGrpcModule.HealthStub(intercept_channel)
+        print(f"HealthCheckService initialized with version {version}")


@Acohen-work Please remove

anshforkel · 2026-05-22T11:21:29Z

+            from ansys.api.cfx.v1 import engine_eval_pb2_grpc as EngineEvalGrpcModule
+        else:
+            from ansys.api.cfx.v0 import engine_eval_pb2 as EngineEvalProtoModule
+            from ansys.api.cfx.v0 import engine_eval_pb2_grpc as EngineEvalGrpcModule


Instead of repeating the logic which Ansys release uses which API version, I suggest to implement that logic in a single location. The logic for determining the API version might become more involved in the future. Therefore it would be beneficial to encapsulate this, i.e. provide an API or a global variable defining the API version (as opposed to the Ansys release version).

Since the imports are different for each service, the most centralized version of this is to look at the engine version. It's only done once per service already.

feat: support both grpc package name versions

27f2176

Acohen-work requested a review from a team as a code owner May 21, 2026 20:07

github-actions Bot added testing Anything related to testing enhancement New features or code improvements labels May 21, 2026

pyansys-ci-bot and others added 4 commits May 21, 2026 20:08

chore: adding changelog file 100.added.md [dependabot-skip]

cc8965f

pre commit

9b15696

Merge branch 'feat/change_grpc_package_names' of https://github.com/a…

4e3f67d

…nsys/pycfx into feat/change_grpc_package_names

add version to api gen in cicd

67aa871

github-actions Bot added the maintenance Package and maintenance related label May 21, 2026

Acohen-work added 2 commits May 21, 2026 16:36

zizmor, pre commit and pass version correctly in allapigen

5117778

typo

bd7e3dc

Copilot started work on behalf of Acohen-work May 21, 2026 20:54 View session

Copilot stopped work on behalf of Acohen-work due to an error May 21, 2026 20:57
The session was cancelled by the user.

Acohen-work and others added 6 commits May 21, 2026 16:58

pass image tag as version

e1acc99

extra 'v'

e7b297f

allow CFXVersion input to start with v

0565ac7

correct image tag

880dee7

tests: update coverage badge value decrease to 81 percent

f85aa9c

Co-authored-by: pyansys-ci-bot <92810346+pyansys-ci-bot@users.noreply.github.com>

Merge branch 'main' into feat/change_grpc_package_names

130d6d9

anshforkel reviewed May 22, 2026

View reviewed changes

anshforkel requested review from Copilot and swilson-work May 22, 2026 09:48

Copilot started reviewing on behalf of anshforkel May 22, 2026 09:48 View session

anshforkel requested a review from mchen-work May 22, 2026 09:49

swilson-work reviewed May 22, 2026

View reviewed changes

Copilot AI reviewed May 22, 2026

View reviewed changes

swilson-work reviewed May 22, 2026

View reviewed changes

anshforkel reviewed May 22, 2026

View reviewed changes



		def create_launcher(cfx_launch_mode: LaunchMode = None, **kwargs):
		def create_launcher(version, cfx_launch_mode: LaunchMode = None, **kwargs):

Uh oh!

Conversation

Acohen-work commented May 21, 2026

Uh oh!

codecov-commenter commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

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

Projects

Milestone

Development

Uh oh!

6 participants

codecov-commenter commented May 21, 2026 •

edited

Loading