docs: add Python async gRPC client guide under gRPC remote execution - #1653
docs: add Python async gRPC client guide under gRPC remote execution#1653tmckayus wants to merge 5 commits into
Conversation
Document remote execution vs explicit gRPC clients, add quick-start and streaming examples, and clarify integrated-client environment variables. Also clarify some existing docs on API and server behavior.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR reorganizes gRPC documentation around integrated remote execution and explicit Python async clients. It adds client guides, API references, streaming examples, TLS guidance, updated server capacity details, and Python client stream behavior documentation. ChangesgRPC documentation and client guidance
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
python/cuopt/cuopt/grpc/linear_programming/grpc_client.pyx (1)
576-580: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winKeep a timed-out incumbent stream registered.
join_incumbent_stream()removes the job from_incumbent_threadsbeforethread.join(timeout). If the timeout expires, the thread remains alive butdelete()no longer sees it and can delete server state while_poll_incumbents()is still running.Keep the entry until the thread finishes.
Proposed fix
- thread = self._incumbent_threads.pop(job_id, None) + thread = self._incumbent_threads.get(job_id) if thread is not None: thread.join(timeout) + if thread.is_alive(): + return + self._incumbent_threads.pop(job_id, None)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/cuopt/cuopt/grpc/linear_programming/grpc_client.pyx` around lines 576 - 580, Update join_incumbent_stream() so _incumbent_threads retains the job entry while thread.join(timeout) returns with the thread still alive; remove the entry only after the thread has finished. Ensure delete() can still observe the active incumbent-stream thread, while preserving the existing error retrieval from _incumbent_thread_errors.
🧹 Nitpick comments (1)
python/cuopt/cuopt/grpc/linear_programming/grpc_client.pyx (1)
534-538: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a runtime warning and removal version for the deprecated callback.
The docstring marks
callbackas deprecated, but the method does not emitDeprecationWarningand does not state a removal version. Add both, or remove the deprecation wording until the removal policy is defined.As per coding guidelines, a public Python API signature change must emit
DeprecationWarningwith a removal version before the old signature is broken.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/cuopt/cuopt/grpc/linear_programming/grpc_client.pyx` around lines 534 - 538, Update the public method containing the deprecated callback documentation to emit a DeprecationWarning whenever the plain callback argument is used, and specify the planned removal version in its deprecation documentation. Keep the callback behavior unchanged until removal; if no removal version can be established, remove the deprecation wording instead.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/cuopt/source/cuopt-grpc/api.rst`:
- Around line 104-107: Expand the Errors section in the API documentation to
specify each RPC’s status contract: CheckStatus uses Status::OK with
job_status=NOT_FOUND for unknown jobs; GetResult uses transport NOT_FOUND for
unknown jobs, UNAVAILABLE when results are not ready, and Status::OK with
status=ERROR_SOLVE_FAILED for failed jobs; DeleteResult and CancelJob use
Status::OK with outcomes reported in response fields. Keep the existing general
transport-versus-response-field distinction and proto reference.
- Around line 100-103: Update the “Problem types” documentation in the SubmitJob
section to identify the supported wire categories as LP/QP or MILP. Explicitly
state that QP is submitted through lp_request using the SolveLPRequest payload,
with quadratic fields in OptimizationProblem, while preserving the existing
routing availability note.
In `@docs/cuopt/source/cuopt-grpc/examples.rst`:
- Around line 9-18: Revise the opening remote-execution statement in the
examples documentation to limit the “run unchanged” claim to the integrated
Python, C API, and cuopt_cli examples. Exclude the separately documented Python
async client, which requires explicit Client(host, port) configuration and does
not use CUOPT_REMOTE_* variables.
In `@docs/cuopt/source/cuopt-grpc/examples/incumbent_stream_demo.py`:
- Around line 11-17: Update the default port used by the incumbent_stream_demo
example and its surrounding server/client commands to 5001, matching
cuopt_grpc_server and the linked guides; ensure running the script without
--port connects to the documented server port consistently.
- Around line 31-55: Add explicit type annotations to
IncumbentPrinter.get_solution, build_problem, and main, using Any for
extension-provided types that cannot be named; annotate main’s optional argv and
return values appropriately. Add meaningful docstrings to build_problem and main
documenting parameters, returns, and raises, and document get_solution as needed
for the public callback API while preserving its behavior.
- Around line 45-52: Update main() so every non-COMPLETED result from
client.wait() calls client.delete(job_id) and joins the incumbent-stream thread
before returning. Add pytest coverage for main() using a fake Client that
verifies cleanup for both COMPLETED and non-completed statuses, while retaining
test_mip_incumbent_stream as integration coverage.
In `@docs/cuopt/source/cuopt-grpc/python-async-client.rst`:
- Around line 54-67: Wrap the job lifecycle in all four examples with
try/finally so delete() executes on every exit path, including timeout, status,
transport, callback, result, and stream-join failures. In
docs/cuopt/source/cuopt-grpc/python-async-client.rst (54-67),
docs/cuopt/source/cuopt-grpc/python-async-client-examples.rst (17-34 and 76-86),
and docs/cuopt/source/cuopt-grpc/examples/incumbent_stream_demo.py (69-86),
explicitly join any log streams before cleanup while keeping delete() in the
finally block so it still runs if joining raises.
In `@docs/cuopt/source/cuopt-grpc/quick-start.rst`:
- Line 74: Update the quick-start server instructions to use the same default
port as the incumbent_stream_demo.py example, or explicitly instruct users to
pass --port 5001 when running that example; ensure the documented commands are
consistent so the examples connect without additional troubleshooting.
- Around line 48-52: Update the quick-start installation instructions around the
GPU server/client selector and the subsequent cuopt_grpc_server verification
command to state that the server-binary check runs only on the GPU server using
the C/libcuopt bundle; keep Python-only client guidance from implying that
command is available, and verify the documented examples and instructions
execute correctly.
- Around line 145-169: Update the async gRPC example around Client.submit,
Client.wait, and Client.result to avoid assert-based runtime validation and wrap
all post-submission operations in try/finally. Validate that wait returns
JobStatus.COMPLETED, allow failures or result errors to propagate, and always
call client.delete(job_id) whenever submission succeeds, including every exit
path.
In `@python/cuopt/cuopt/grpc/linear_programming/grpc_client.pyx`:
- Around line 287-291: Update the docstring for the job-waiting method
containing this documentation to describe that non-None timeouts are converted
with int(timeout), including the resulting indefinite wait for values such as
0.5, and that positive timeouts poll every second and raise GrpcError when they
expire instead of returning JobStatus; alternatively, validate timeout values to
prevent this behavior and document the enforced contract.
---
Outside diff comments:
In `@python/cuopt/cuopt/grpc/linear_programming/grpc_client.pyx`:
- Around line 576-580: Update join_incumbent_stream() so _incumbent_threads
retains the job entry while thread.join(timeout) returns with the thread still
alive; remove the entry only after the thread has finished. Ensure delete() can
still observe the active incumbent-stream thread, while preserving the existing
error retrieval from _incumbent_thread_errors.
---
Nitpick comments:
In `@python/cuopt/cuopt/grpc/linear_programming/grpc_client.pyx`:
- Around line 534-538: Update the public method containing the deprecated
callback documentation to emit a DeprecationWarning whenever the plain callback
argument is used, and specify the planned removal version in its deprecation
documentation. Keep the callback behavior unchanged until removal; if no removal
version can be established, remove the deprecation wording instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c0d71359-29d7-4f47-bc73-f2eee2d3d38c
📒 Files selected for processing (15)
docs/cuopt/source/_static/large-rubric.cssdocs/cuopt/source/conf.pydocs/cuopt/source/cuopt-grpc/advanced.rstdocs/cuopt/source/cuopt-grpc/api.rstdocs/cuopt/source/cuopt-grpc/examples.rstdocs/cuopt/source/cuopt-grpc/examples/incumbent_stream_demo.pydocs/cuopt/source/cuopt-grpc/grpc-server-architecture.mddocs/cuopt/source/cuopt-grpc/index.rstdocs/cuopt/source/cuopt-grpc/python-async-client-api.rstdocs/cuopt/source/cuopt-grpc/python-async-client-examples.rstdocs/cuopt/source/cuopt-grpc/python-async-client.rstdocs/cuopt/source/cuopt-grpc/quick-start.rstdocs/cuopt/source/cuopt-python/index.rstdocs/cuopt/source/introduction.rstpython/cuopt/cuopt/grpc/linear_programming/grpc_client.pyx
CI Test Summary✅ All 22 test job(s) passed. (1 skipped) |
includes one code fix on log streaming
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
python/cuopt/cuopt/grpc/linear_programming/grpc_client.pyx (3)
421-433: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHonor
Falseduring log backfill.
start_log_streamdocumentsFalseas an early-stop signal. The fallback path calls_call_log_callback(...)but ignores its return value, so it continues invoking the callback after the callback returnsFalse.Stop the backfill loop when
_call_log_callback(...) is False.Suggested fix
for line in bulk: state["lines"].append(line) - _call_log_callback(state["callback"], line, True) + if _call_log_callback(state["callback"], line, True) is False: + break🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/cuopt/cuopt/grpc/linear_programming/grpc_client.pyx` around lines 421 - 433, Update the log backfill loop in the method documenting start_log_stream behavior to inspect the return value of _call_log_callback(...). Stop iterating immediately when it returns False, while preserving the existing backfill and callback behavior for other return values.
308-322: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winCancel the job before joining the active incumbent stream.
delete()joinsjoin_incumbent_stream(job_id)before it calls the server’sdelete_job. The join has no timeout here, and_poll_incumbents()normally exits only after completion or cancellation. Deleting a running job with an active incumbent stream can therefore block indefinitely and never reach server cleanup. A stored stream error can also raise before deletion.Cancel queued or processing jobs before joining. Ensure the server deletion still runs when joining reports a stream error.
Suggested ordering
if job_id in self._incumbent_threads: + if self.status(job_id) in ( + JobStatus.QUEUED, + JobStatus.PROCESSING, + ): + self.cancel(job_id) self.join_incumbent_stream(job_id)Also applies to: 590-599
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/cuopt/cuopt/grpc/linear_programming/grpc_client.pyx` around lines 308 - 322, Update delete() to cancel queued or processing jobs before calling join_incumbent_stream(job_id), allowing _poll_incumbents() to exit before the join. Preserve any stream error while ensuring the server delete_job operation always runs, then propagate the stored error after deletion if appropriate.
549-559: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winCancel the job before joining an active incumbent stream.
Client.delete()joins the stream before deleting the job and never requests cancellation. An active stream can hang indefinitely. Cancel the job before joining it, and add tests for active-stream deletion, callback cancellation,DeprecationWarning, timed joins, and stored worker errors.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/cuopt/cuopt/grpc/linear_programming/grpc_client.pyx` around lines 549 - 559, Update Client.delete() to request job cancellation before joining any active incumbent stream, preventing an indefinite wait; preserve deletion after the stream exits. Add coverage for active-stream deletion, callback-triggered cancellation, DeprecationWarning behavior in start_incumbent_stream(), timed joins, and propagation of stored worker errors.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@python/cuopt/cuopt/grpc/linear_programming/grpc_client.pyx`:
- Around line 421-433: Update the log backfill loop in the method documenting
start_log_stream behavior to inspect the return value of
_call_log_callback(...). Stop iterating immediately when it returns False, while
preserving the existing backfill and callback behavior for other return values.
- Around line 308-322: Update delete() to cancel queued or processing jobs
before calling join_incumbent_stream(job_id), allowing _poll_incumbents() to
exit before the join. Preserve any stream error while ensuring the server
delete_job operation always runs, then propagate the stored error after deletion
if appropriate.
- Around line 549-559: Update Client.delete() to request job cancellation before
joining any active incumbent stream, preventing an indefinite wait; preserve
deletion after the stream exits. Add coverage for active-stream deletion,
callback-triggered cancellation, DeprecationWarning behavior in
start_incumbent_stream(), timed joins, and propagation of stored worker errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c5a91ae3-44b9-4959-a97a-19c399186b71
📒 Files selected for processing (7)
docs/cuopt/source/cuopt-grpc/api.rstdocs/cuopt/source/cuopt-grpc/examples.rstdocs/cuopt/source/cuopt-grpc/examples/incumbent_stream_demo.pydocs/cuopt/source/cuopt-grpc/python-async-client-examples.rstdocs/cuopt/source/cuopt-grpc/python-async-client.rstdocs/cuopt/source/cuopt-grpc/quick-start.rstpython/cuopt/cuopt/grpc/linear_programming/grpc_client.pyx
🚧 Files skipped from review as they are similar to previous changes (6)
- docs/cuopt/source/cuopt-grpc/python-async-client-examples.rst
- docs/cuopt/source/cuopt-grpc/python-async-client.rst
- docs/cuopt/source/cuopt-grpc/quick-start.rst
- docs/cuopt/source/cuopt-grpc/api.rst
- docs/cuopt/source/cuopt-grpc/examples/incumbent_stream_demo.py
- docs/cuopt/source/cuopt-grpc/examples.rst
| Limitations and Scope | ||
| ===================== | ||
|
|
||
| * **Problem types** — **LP**, **MILP**, and **QP** are supported on the gRPC remote path. **Routing** (VRP, TSP, PDP) is **not** supported yet; use the :doc:`REST self-hosted server <../cuopt-server/index>` for remote routing until a future release adds routing over ``CuOptRemoteService``. |
There was a problem hiding this comment.
Something I noticed: MIP and MILP still seem to be used interchangeably throughout the docs? Is that on purpose?
There was a problem hiding this comment.
normalized on MIP, and in #1655 normalized references in the other docs as well. Thanks!
cwilkinson76
left a comment
There was a problem hiding this comment.
Only a few minor edits
Additionally, change remote_lp_demo.py to maxmimize so the problem is more interesting, and also align remote_lp_demo.mps to match (the mps file previously was slightly different than the demo script anyway. Now they solve the same problem). Normalize "MILP" references to "MIP" in any file already touched by this PR. Follow up with a separate PR for the other docs.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/cuopt/source/cuopt-grpc/examples/incumbent_stream_demo.py`:
- Around line 24-30: Update IncumbentPrinter with a meaningful class docstring,
and annotate get_solution’s solution, solution_cost, solution_bound, and
user_data parameters using Any where cuOpt types are unavailable, with a None
return annotation. Add a method docstring documenting all parameters, the lack
of a return value, and any applicable raises behavior.
- Around line 44-55: Add pytest coverage under python/cuopt/cuopt/tests for the
incumbent stream lifecycle in incumbent_stream_demo, using a fake Client to
cover completed, non-completed, and wait() exception paths. Assert that
join_incumbent_stream occurs before delete(job_id), and point to existing
equivalent coverage instead if it is already present elsewhere.
- Around line 44-55: Update the cleanup around start_incumbent_stream,
join_incumbent_stream, and delete so that once the incumbent stream starts
successfully, join_incumbent_stream(job_id) always runs before
client.delete(job_id), including when wait raises or returns a non-completed
status. Preserve deletion in the final cleanup and avoid joining if stream
startup fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: dcb9314e-cebf-4a25-82d8-416403dca2c2
⛔ Files ignored due to path filters (1)
docs/cuopt/source/cuopt-grpc/examples/remote_lp_demo.mpsis excluded by!**/*.mps
📒 Files selected for processing (11)
docs/cuopt/source/cuopt-grpc/advanced.rstdocs/cuopt/source/cuopt-grpc/api.rstdocs/cuopt/source/cuopt-grpc/examples.rstdocs/cuopt/source/cuopt-grpc/examples/incumbent_stream_demo.pydocs/cuopt/source/cuopt-grpc/examples/remote_lp_demo.pydocs/cuopt/source/cuopt-grpc/index.rstdocs/cuopt/source/cuopt-grpc/python-async-client-api.rstdocs/cuopt/source/cuopt-grpc/python-async-client-examples.rstdocs/cuopt/source/cuopt-grpc/python-async-client.rstdocs/cuopt/source/cuopt-grpc/quick-start.rstdocs/cuopt/source/introduction.rst
🚧 Files skipped from review as they are similar to previous changes (7)
- docs/cuopt/source/cuopt-grpc/python-async-client-api.rst
- docs/cuopt/source/cuopt-grpc/python-async-client.rst
- docs/cuopt/source/cuopt-grpc/index.rst
- docs/cuopt/source/cuopt-grpc/examples.rst
- docs/cuopt/source/cuopt-grpc/api.rst
- docs/cuopt/source/cuopt-grpc/advanced.rst
- docs/cuopt/source/cuopt-grpc/quick-start.rst
|
Normalized MILP references in these files to MIP, and submitted another PR to normalize the other references in documents outside of this PR |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
python/cuopt/cuopt/grpc/linear_programming/grpc_client.pyx (2)
553-563: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftCancel the job before joining the incumbent stream.
delete()joins beforedelete_job(), while_poll_incumbents()waits forjob_complete. This can block deletion of a running job. The implementation also does not callcancel()as its docstring states. Cancel first, ensuredelete_job()runs infinally, then re-raise stream errors. Add pytest coverage for running jobs and failed stream threads.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/cuopt/cuopt/grpc/linear_programming/grpc_client.pyx` around lines 553 - 563, Update join_incumbent_stream to cancel the job before waiting on its incumbent-stream thread, matching the documented behavior and preventing deletion from blocking on a running job. Ensure delete_job is always invoked in a finally block, then re-raise any stored stream-thread exception after cleanup; preserve timeout handling for still-running threads. Add pytest coverage for running jobs and failed stream threads.Source: Coding guidelines
273-281: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDescribe
status()as a synchronous status check.
status()calls synchronousCheckStatus, so it can block on transport or server I/O. It does not wait for the job to reach a terminal state. Replace “without blocking” with “without waiting for the job to reach a terminal state.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/cuopt/cuopt/grpc/linear_programming/grpc_client.pyx` around lines 273 - 281, Update the docstring for the status() method to describe it as a synchronous status check that may block on transport or server I/O, while clarifying that it does not wait for the job to reach a terminal state. Replace the “without blocking” wording accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@python/cuopt/cuopt/grpc/linear_programming/grpc_client.pyx`:
- Around line 553-563: Update join_incumbent_stream to cancel the job before
waiting on its incumbent-stream thread, matching the documented behavior and
preventing deletion from blocking on a running job. Ensure delete_job is always
invoked in a finally block, then re-raise any stored stream-thread exception
after cleanup; preserve timeout handling for still-running threads. Add pytest
coverage for running jobs and failed stream threads.
- Around line 273-281: Update the docstring for the status() method to describe
it as a synchronous status check that may block on transport or server I/O,
while clarifying that it does not wait for the job to reach a terminal state.
Replace the “without blocking” wording accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2af4cc5b-a970-48c4-b8ab-2ad3ab5ac55a
📒 Files selected for processing (1)
python/cuopt/cuopt/grpc/linear_programming/grpc_client.pyx
|
since 1654 merged, I need to check that the items fixed in that PR are correctly documented now |
We refer to both MILP and MIP in the documentation, so we decided to normalize around MIP everywhere. Left a few references changed separately in #1653 Authors: - Trevor McKay (https://github.com/tmckayus) Approvers: - Ramakrishna Prabhu (https://github.com/ramakrishnap-nv) URL: #1655
Document remote execution vs explicit gRPC clients, add quick-start and streaming examples, and clarify integrated-client environment variables. Also clarify some existing docs on API and server behavior.