Skip to content

Conversation

@bkeryan
Copy link
Collaborator

@bkeryan bkeryan commented Sep 20, 2023

What does this Pull Request accomplish?

Add a session_management_client property to the MeasurementService object, which lazily constructs a SessionManagementClient and caches it.

Rework initialization code:

  • DiscoveryClient: lazily create stub and allow injecting a GrpcChannelPool.
  • SessionManagementClient: lazily create stub, allow injecting a DiscoveryClient and GrpcChannelPool, and automatically resolve service.
  • MeasurementService:
    • Lazily create DiscoveryClient.
    • Defer GrpcService creation until host_service() is called.
    • Update close_service() to clear lazily created objects, in order to allow reusing the MeasurementService object.
    • Deprecate grpc_service property because it leaks implementation details.
    • Add service_location property as a replacement for grpc_service that uses public types.

The examples will be updated in AB#2527580.

Why should this Pull Request be merged?

Provide a simple, official API for creating the session management client, to replace the create_session_management_client function in _helpers.py.

Prepare to inject a DiscoveryClient and GrpcChannelPool into the Reservation classes to support improved session management APIs.

What testing has been done?

Ran poetry run pytest -v
Manually tested nidcpower_source_dc_voltage and sample_measurement with debug logging (-vv) enabled.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 20, 2023

Test Results

     12 files  ±0       12 suites  ±0   2m 0s ⏱️ -6s
   198 tests ±0     169 ✔️ ±0    29 💤 ±0  0 ±0 
2 364 runs  ±0  2 016 ✔️ ±0  348 💤 ±0  0 ±0 

Results for commit fd3c031. ± Comparison against base commit d83e46f.

♻️ This comment has been updated with latest results.

@bkeryan bkeryan merged commit ceda75a into main Sep 20, 2023
@bkeryan bkeryan deleted the users/bkeryan/client-constructors branch September 20, 2023 21:13
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.

3 participants