Web hosted client (api key based access)

[atomic]

Description

This PR introduces a new CuOptServiceWebHostedClient for connecting to web-hosted cuOpt services (like NVIDIA's API endpoints) with endpoint URL support and authentication mechanisms.

Issue

#479

Tested with NVIDIA's Build.com CuOpt model

Test script (excluded from the MR as it is an adhoc script): https://gist.github.com/atomic/9c761bd4d64d62bc039ecf88fe28fca0#file-test_cuopt_nvidia_build_com-py

export CUOPT_API_KEY=... python3 file-test_cuopt_nvidia_build_com-py.py

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[rgsl888prabhu]

/ok to test e595eab

[rgsl888prabhu]

@CodeRabbit review and summarize status

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get PR status and details
gh pr view 480 --json state,title,isDraft,reviews,statusCheckRollup,mergeable,mergeStateStatus,additions,deletions,changedFiles

# Get list of files changed in the PR
gh pr view 480 --json files --jq '.files[] | "\(.path) +\(.additions) -\(.deletions)"'

Length of output: 5220

---

🏁 Script executed:

#!/bin/bash
# Get the new web hosted client implementation
cat python/cuopt_self_hosted/cuopt_sh_client/cuopt_web_hosted_client.py

echo -e "\n\n=== INIT FILE ==="
cat python/cuopt_self_hosted/cuopt_sh_client/__init__.py

echo -e "\n\n=== MODIFIED SELF HOST CLIENT ==="
cat python/cuopt_self_hosted/cuopt_sh_client/cuopt_self_host_client.py

echo -e "\n\n=== TEST FILE ==="
cat python/cuopt_self_hosted/tests/test_web_hosted_client.py

Length of output: 61283

---

@rgsl888prabhu I'll review the PR and provide a status summary.

📊 PR Status Summary

Current State: OPEN (Merge Blocked - Waiting for CI checks)

CI Checks Status:

• ✅ Passing: changed-files, other-checks, Branch Checker, CodeRabbit, Label Checker
• 🔄 In Progress: check-style, multiple wheel builds (Python 3.10-3.13, amd64/arm64)
• Reviews: None submitted yet

Changes: 4 files, +527/-10 lines

• New: cuopt_web_hosted_client.py (251 lines)
• New: test_web_hosted_client.py (232 lines)
• Modified: cuopt_self_host_client.py (+40/-10)
• Modified: __init__.py (+4)

---

🔍 Code Review

✅ Strengths

1. Excellent Architecture

   • Clean inheritance from CuOptServiceSelfHostClient
   • Factory pattern with create_client() for easy client selection
   • Proper separation of concerns between web-hosted and self-hosted clients

2. Comprehensive Testing

   • 232 lines of well-structured unit tests
   • Tests cover URL parsing, authentication, error handling, and factory function
   • Good use of mocking and parametrization

3. Authentication & Security

   • Proper Bearer token implementation for API keys
   • Environment variable support (CUOPT_API_KEY)
   • Self-signed certificate handling
   • Specific error handling for 401/403 responses

4. Documentation

   • Detailed docstrings with parameter descriptions
   • Clear usage examples in docstrings
   • Good inline comments explaining design decisions

5. Backward Compatibility

   • Parent class refactored to support extensibility via _make_http_request()
   • Existing functionality preserved

🔧 Suggestions for Improvement

1. Error Handling Enhancement (Line 186-192, cuopt_web_hosted_client.py)

   # Current: Uses ValueError for auth errors
   if response.status_code == 401:
       raise ValueError("Authentication failed. Please check your API key.")
   
   # Consider: More specific exception types
   if response.status_code == 401:
       raise AuthenticationError("Authentication failed. Please check your API key.")
   elif response.status_code == 403:
       raise PermissionError("Access forbidden. Please check your permissions.")

2. Header Merging Pattern (Line 174-178)
   The current implementation replaces headers entirely. Consider a more defensive merge:

   def _make_http_request(self, method: str, url: str, **kwargs):
       headers = kwargs.get("headers", {}).copy()  # Copy to avoid mutations
       headers.update(self._get_auth_headers())
       kwargs["headers"] = headers
       
       response = requests.request(method, url, **kwargs)
       # ... rest of the method

3. URL Construction Clarity (Lines 140-149)
   The comment mentions a "curl example" - consider adding a reference to documentation or examples for clarity.

4. Test Coverage Enhancement
   While unit tests are comprehensive, consider adding integration tests that verify:

   • get_optimized_routes() works with the web-hosted client
   • get_LP_solve() works with authentication headers
   • URL construction is correct for all inherited methods

5. Type Hints
   Consider adding return type hints to methods like _get_auth_headers():

   def _get_auth_headers(self) -> Dict[str, str]:

📝 Minor Observations

1. Line 49: The log warning for missing protocol is good, but consider making the default protocol configurable if http-only endpoints exist.

2. Line 28: mime_type import is correct, but verify it's exported properly in the parent module's __init__.py (it is ✅).

3. Factory Function: The create_client() error message when API key is provided without endpoint is excellent - very clear guidance for users.

---

🎯 Overall Assessment

Code Quality: ⭐⭐⭐⭐½ (4.5/5)

• Well-designed, tested, and documented
• Follows Python best practices
• Minor improvements suggested above

Readiness:

• ⏳ Waiting for: CI checks to complete (check-style and wheel builds)
• ✅ Code review: Looks good with minor suggestions
• ✅ Tests: Comprehensive coverage
• ✅ Documentation: Well documented

Recommendation: Once CI checks pass (check-style and wheel builds), this PR should be ready to merge pending maintainer review of the architectural approach.

---

Would you like me to elaborate on any of the suggestions or run additional analysis on specific aspects of the code?

[anandhkb]

Gentle ping @atomic

[github-actions]

🔔 Hi @anandhkb @atomic, this pull request has had no activity for 7 days. Please update or let us know if it can be closed. Thank you!

If this is an "epic" issue, then please add the "epic" label to this issue.
If it is a PR and not ready for review, then please convert this to draft.
If you just want to switch off this notification, then use the "skip inactivity reminder" label.