Skip to content

Conversation

@subhnayak
Copy link
Contributor

Description

Please provide a brief description of the changes made in this pull request.

Issue linked

Please mention the issue number or describe the problem this pull request addresses.

Checklist

  • I have tested my changes locally.
  • I have added necessary documentation or updated existing documentation.
  • I have followed the coding style guidelines of this project.
  • I have added appropriate unit tests.
  • I have reviewed my changes before submitting this pull request.
  • I have linked the issue or issues that are solved to the PR if any.
  • I have assigned this PR to myself.
  • I have made sure that the title of my PR follows Conventional commits style (e.g. feat: wrap with feature edges)

@github-actions github-actions bot added maintenance Package and maintenance related documentation Improvements or additions to documentation labels Nov 28, 2025
@subhnayak subhnayak requested a review from Copilot December 10, 2025 07:09
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the PyPrimeMesh package to version 0.8.2, introducing secure gRPC communication capabilities with mutual TLS support and improving connection security between client and server.

Key Changes:

  • Added secure gRPC communication with mutual TLS (mTLS) support using client and server certificates
  • Introduced ConnectionType enum to distinguish between secure and insecure gRPC connections
  • Updated default remote version from '251-sp2' to 'latest'

Reviewed changes

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

Show a summary per file
File Description
pyproject.toml Version bump from 0.8.1 to 0.8.2
src/ansys/meshing/prime/internals/config.py Added ConnectionType enum for connection type management
src/ansys/meshing/prime/internals/launcher.py Enhanced launch_prime and launch_server_process functions with certificate directory parameters and secure connection support
src/ansys/meshing/prime/internals/client.py Updated Client class to handle secure/insecure connections with certificate validation
src/ansys/meshing/prime/internals/grpc_communicator.py Added get_secure_channel function for TLS channel creation and removed unused server_command method
src/ansys/meshing/prime/params/primestructs.py Added extra blank comment line
docker/linux/Dockerfile Added COPY instruction for certificates directory
docker/build_docker_linux.py Enhanced Docker image build script with version tagging and error handling
doc/source/getting_started/index.rst Added documentation for gRPC connection types and secure certificate-based connections
doc/styles/config/vocabularies/ANSYS/accept.txt Added 'client_certs_dir' to accepted vocabulary
.github/workflows/ci_cd.yml Updated Docker image tag from 25.1.2 to 25.1.4
doc/changelog.d/1188.documentation.md Added changelog entry

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

Comment on lines +50 to +54
tls_client_files : list
List of paths to the TLS files. The list should contain:
- client certificate file path
- client key file path
- CA certificate file path
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The parameter documentation refers to 'tls_client_files' but the actual parameter is named 'client_certs_dir'. Update the parameter name in the docstring to match the function signature.

Suggested change
tls_client_files : list
List of paths to the TLS files. The list should contain:
- client certificate file path
- client key file path
- CA certificate file path
client_certs_dir : str
Path to the directory containing the TLS certificate files. The directory should contain:
- client.crt (client certificate file)
- client.key (client key file)
- ca.crt (CA certificate file)

Copilot uses AI. Check for mistakes.
Comment on lines +50 to +51
GRPC_SECURE = (1,)
GRPC_INSECURE = (2,)
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The enum values use single-element tuples (1,) and (2,) instead of simple integers. This is unnecessary and unconventional. Change to GRPC_SECURE = 1 and GRPC_INSECURE = 2 for clarity.

Suggested change
GRPC_SECURE = (1,)
GRPC_INSECURE = (2,)
GRPC_SECURE = 1
GRPC_INSECURE = 2

Copilot uses AI. Check for mistakes.
Comment on lines +113 to +114
"Warning: Ignoring client certificate \
directory for insecure connections"
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The string continuation with backslash creates a multi-line string literal with embedded whitespace. Use implicit string concatenation with parentheses or a single-line string for cleaner code.

Suggested change
"Warning: Ignoring client certificate \
directory for insecure connections"
(
"Warning: Ignoring client certificate "
"directory for insecure connections"
)

Copilot uses AI. Check for mistakes.
- Ensure that ca.crt file is the same for the client and the server. You should not modify the
file names in the client_certs_dir and server_certs_dir respectively.

- The path of input the files must be the same for server and client and should be on the shared network.
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

Grammatical error: 'path of input the files' should be 'path of the input files' or 'path to the input files'.

Suggested change
- The path of input the files must be the same for server and client and should be on the shared network.
- The path to the input files must be the same for server and client and should be on the shared network.

Copilot uses AI. Check for mistakes.
if ip == defaults.ip():
port = utils.get_available_local_port(port)

channel = None
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The variable channel is initialized to None but never assigned a different value before being passed to the Client constructor at line 336. Consider removing this unused initialization or documenting its intended purpose.

Copilot uses AI. Check for mistakes.
@github-actions github-actions bot added the testing Anything related to testing label Dec 12, 2025
@subhnayak subhnayak merged commit 6aa97ad into release/0.8 Dec 15, 2025
21 checks passed
@subhnayak subhnayak deleted the branch/0.8.2 branch December 15, 2025 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation maintenance Package and maintenance related testing Anything related to testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants