Skip to content

Change 'create_grpc_client_channel' to 'create_grpc_device_channel' - #13

Merged
alexdubois-ni merged 1 commit into
mainfrom
users/adubois/changeCreateGrpcClientChannelToBeRestrictedToGrpcDevice
Aug 24, 2026
Merged

alexdubois-ni merged 1 commit into
mainfrom
users/adubois/changeCreateGrpcClientChannelToBeRestrictedToGrpcDevice

Conversation

@alexdubois-ni

@alexdubois-ni alexdubois-ni commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

What does this Pull Request accomplish?

Change the public API 'create_grpc_client_channel' to 'create_grpc_device_channel'

Why should this Pull Request be merged?

Originally, this code was based off existing code that allows us to configure any service for usage with NI-TLS. However, there is no need for that in the public github repo for Python as the only known NI-TLS usage we intend to have today that needs Python access and NI-TLS integration is gRPC device.

Obviously, this is subject to change in the future, but it is easier to make our interface more restrictive at first and then extend it later to be more flexible as opposed to the other way around.

I want to ensure we have the least code and complexity to maintain for now, so I am opting to simplify this a bit, and make the interface to be clearer for users. But, we have every ability to make this more generic in the future if the need arises and have most of the re-usable code ready and in-place for extending.

Furthermore, the terminology from gRPC is that a channel is essentially a client, so having "client channel" in the name is rather redundant as it stands today. I'd rather make this specifically mention grpc_device.

Note: I am not worried about backwards compatibility. We are only NOW beginning to bubble this into nimi-python and eventually DAQmx Python, so I'm trying to make this change now as opposed to later.

What testing has been done?

Unit-tests.

@ni-github-admins

ni-github-admins commented Aug 24, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@alexdubois-ni
alexdubois-ni force-pushed the users/adubois/changeCreateGrpcClientChannelToBeRestrictedToGrpcDevice branch from 2475090 to 5b384ff Compare August 24, 2026 21:37
@github-actions

Copy link
Copy Markdown

Test Results

 12 files  ±0   12 suites  ±0   17s ⏱️ -1s
 66 tests ±0   66 ✅ ±0  0 💤 ±0  0 ❌ ±0 
792 runs  ±0  792 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 5b384ff. ± Comparison against base commit b329f64.

This pull request removes 1 and adds 1 tests. Note that renamed tests count towards both.
tests.unit.test_grpc_channel ‑ test_service_name_and_address_are_forwarded
tests.unit.test_grpc_channel ‑ test_configuration_is_read_for_the_grpc_device_service

@alexdubois-ni alexdubois-ni changed the title Users/adubois/change create grpc client channel to be restricted to grpc device Change 'create_grpc_client_channel' to 'create_grpc_device_channel' Aug 24, 2026
@alexdubois-ni
alexdubois-ni marked this pull request as ready for review August 24, 2026 21:47
@alexdubois-ni
alexdubois-ni requested a balanced review from Copilot August 24, 2026 21:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Renames and narrows the public gRPC channel factory specifically for the NI gRPC Device Server.

Changes:

  • Replaces create_grpc_client_channel with create_grpc_device_channel.
  • Fixes the NI-TLS service name to ni-grpc-device and centralizes it.
  • Updates exports, documentation, CI, and unit tests.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/nitlsconfig/grpc_channel.py Renames and restricts the channel factory.
src/nitlsconfig/service.py Defines the shared device service name.
src/nitlsconfig/audit.py Uses the shared service name for auditing.
src/nitlsconfig/__init__.py Updates public and lazy exports.
tests/unit/test_grpc_channel.py Updates factory and fixed-service tests.
tests/unit/test_grpc_channel_tls.py Updates TLS integration tests.
tests/unit/test_grpc_channel_real.py Updates real-channel tests.
tests/unit/test_nitlsconfig.py Updates configuration integration tests.
README.md Documents the renamed API.
.github/workflows/run_unit_tests.yml Updates optional-dependency verification.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@alexdubois-ni
alexdubois-ni requested a review from ckoellin August 24, 2026 21:51
@alexdubois-ni
alexdubois-ni merged commit 64a21d5 into main Aug 24, 2026
31 checks passed
@alexdubois-ni
alexdubois-ni deleted the users/adubois/changeCreateGrpcClientChannelToBeRestrictedToGrpcDevice branch August 24, 2026 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants