Skip to content

Commit fc8dea3

Browse files
atomicclaude
andcommitted
Clean up endpoint validation and fix formatting
Remove suspicious hostname pattern validation that was overly heuristic-based. Simplify invalid URL detection to rely on urlparse hostname check. Fix Black/isort/flake8 formatting issues. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Tony Salim <tsalim@nvidia.com>
1 parent 4b46fa6 commit fc8dea3

2 files changed

Lines changed: 147 additions & 114 deletions

File tree

python/cuopt_self_hosted/cuopt_sh_client/cuopt_web_hosted_client.py

Lines changed: 71 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@
1717
import os
1818
import warnings
1919
from typing import Dict, Optional, Union
20-
from urllib.parse import urlparse, urljoin
20+
from urllib.parse import urljoin, urlparse
2121

2222
import requests
2323

24-
from .cuopt_self_host_client import CuOptServiceSelfHostClient, mime_type
24+
from .cuopt_self_host_client import CuOptServiceSelfHostClient
2525

2626
log = logging.getLogger(__name__)
2727

@@ -30,11 +30,11 @@ class CuOptServiceWebHostedClient(CuOptServiceSelfHostClient):
3030
"""
3131
Web-hosted version of the CuOptServiceClient that supports endpoint URLs
3232
and authentication mechanisms for cloud-hosted services.
33-
33+
3434
This client is specifically designed for web-hosted cuOpt services and
3535
requires an endpoint URL. For self-hosted services with ip/port parameters,
3636
use CuOptServiceSelfHostClient instead.
37-
37+
3838
Parameters
3939
----------
4040
endpoint : str
@@ -43,8 +43,9 @@ class CuOptServiceWebHostedClient(CuOptServiceSelfHostClient):
4343
- "https://inference.nvidia.com/cuopt"
4444
- "http://my-cuopt-service.com:8080/api"
4545
api_key : str, optional
46-
API key for authentication. Will be sent as Bearer token in Authorization header.
47-
Can also be set via CUOPT_API_KEY environment variable.
46+
API key for authentication. Will be sent as
47+
Bearer token in Authorization header. Can also
48+
be set via CUOPT_API_KEY environment variable.
4849
base_path : str, optional
4950
Base path to append to the endpoint if not included in endpoint URL.
5051
Defaults to "/cuopt" if not specified in endpoint.
@@ -54,116 +55,130 @@ class CuOptServiceWebHostedClient(CuOptServiceSelfHostClient):
5455
Additional parameters passed to parent CuOptServiceSelfHostClient
5556
(excluding ip, port, use_https which are determined from endpoint)
5657
"""
57-
58+
5859
def __init__(
5960
self,
60-
endpoint: str,
61+
endpoint: Optional[str] = None,
6162
api_key: Optional[str] = None,
6263
base_path: Optional[str] = None,
6364
self_signed_cert: str = "",
64-
**kwargs
65+
**kwargs,
6566
):
6667
if not endpoint:
67-
raise ValueError("endpoint parameter is required for CuOptServiceWebHostedClient")
68-
68+
raise ValueError(
69+
"endpoint parameter is required"
70+
" for CuOptServiceWebHostedClient"
71+
)
72+
6973
# Handle authentication from environment variables
7074
self.api_key = api_key or os.getenv("CUOPT_API_KEY")
71-
75+
7276
# Parse endpoint URL
7377
self._parsed_endpoint = self._parse_endpoint_url(endpoint, base_path)
74-
78+
7579
# Extract connection parameters from endpoint
7680
ip = self._parsed_endpoint["host"]
77-
port = str(self._parsed_endpoint["port"]) if self._parsed_endpoint["port"] else ""
81+
port = (
82+
str(self._parsed_endpoint["port"])
83+
if self._parsed_endpoint["port"]
84+
else ""
85+
)
7886
use_https = self._parsed_endpoint["scheme"] == "https"
7987
self._base_path = self._parsed_endpoint["path"]
80-
88+
8189
# Initialize parent class with extracted parameters
8290
super().__init__(
8391
ip=ip,
8492
port=port,
8593
use_https=use_https,
8694
self_signed_cert=self_signed_cert,
87-
**kwargs
95+
**kwargs,
8896
)
89-
97+
9098
# Override URL construction with endpoint-based URLs
9199
self._construct_endpoint_urls()
92-
93-
def _parse_endpoint_url(self, endpoint: str, base_path: Optional[str] = None) -> Dict[str, Union[str, int, None]]:
100+
101+
def _parse_endpoint_url(
102+
self, endpoint: str, base_path: Optional[str] = None
103+
) -> Dict[str, Union[str, int, None]]:
94104
"""
95105
Parse endpoint URL and extract components.
96-
106+
97107
Parameters
98108
----------
99109
endpoint : str
100110
Full endpoint URL
101111
base_path : str, optional
102112
Base path to use if not included in endpoint
103-
113+
104114
Returns
105115
-------
106116
dict
107117
Parsed URL components
108118
"""
109119
# Add protocol if missing
110120
if not endpoint.startswith(("http://", "https://")):
111-
log.warning(f"No protocol specified in endpoint '{endpoint}', assuming https://")
121+
warnings.warn(
122+
"No protocol specified in endpoint"
123+
f" '{endpoint}', assuming https://",
124+
UserWarning,
125+
)
112126
endpoint = f"https://{endpoint}"
113-
127+
114128
parsed = urlparse(endpoint)
115-
129+
116130
if not parsed.hostname:
117131
raise ValueError(f"Invalid endpoint URL: {endpoint}")
118-
132+
119133
# Determine base path
120134
path = parsed.path.rstrip("/")
121135
if not path and base_path:
122136
path = base_path.rstrip("/")
123137
elif not path:
124138
path = "/cuopt"
125-
139+
126140
return {
127141
"scheme": parsed.scheme,
128142
"host": parsed.hostname,
129143
"port": parsed.port,
130144
"path": path,
131-
"full_url": f"{parsed.scheme}://{parsed.netloc}{path}"
145+
"full_url": f"{parsed.scheme}://{parsed.netloc}{path}",
132146
}
133-
147+
134148
def _construct_endpoint_urls(self):
135149
"""Construct service URLs from parsed endpoint.
136-
150+
137151
For web-hosted cuOpt services (like NVIDIA's API), the endpoint
138152
URL is typically the complete service endpoint that handles all
139153
operations. Unlike self-hosted services that have separate paths
140154
for /request, /log, /solution, web-hosted APIs often use a single
141155
endpoint for all operations.
142156
"""
143157
base_url = self._parsed_endpoint["full_url"]
144-
158+
145159
# For web-hosted services, use the provided endpoint directly
146160
# This matches the curl example which POSTs directly to the endpoint
147161
self.request_url = base_url
148-
162+
149163
# Log and solution URLs may still follow traditional patterns
150164
# but many web-hosted APIs use the same endpoint for all operations
151165
self.log_url = urljoin(base_url + "/", "log")
152166
self.solution_url = urljoin(base_url + "/", "solution")
153-
167+
154168
def _get_auth_headers(self) -> Dict[str, str]:
155169
"""Get authentication headers."""
156170
headers = {}
157-
171+
158172
if self.api_key:
159173
headers["Authorization"] = f"Bearer {self.api_key}"
160-
174+
161175
return headers
162-
176+
163177
def _make_http_request(self, method: str, url: str, **kwargs):
164178
"""
165-
Override parent method to add authentication headers and handle auth errors.
166-
179+
Override parent method to add authentication
180+
headers and handle auth errors.
181+
167182
Parameters
168183
----------
169184
method : str
@@ -172,7 +187,7 @@ def _make_http_request(self, method: str, url: str, **kwargs):
172187
Request URL
173188
**kwargs
174189
Additional arguments passed to requests.request()
175-
190+
176191
Returns
177192
-------
178193
requests.Response
@@ -182,62 +197,63 @@ def _make_http_request(self, method: str, url: str, **kwargs):
182197
headers = kwargs.get("headers", {})
183198
headers.update(self._get_auth_headers())
184199
kwargs["headers"] = headers
185-
200+
186201
# Make request
187202
response = requests.request(method, url, **kwargs)
188-
203+
189204
# Handle authentication errors
190205
if response.status_code == 401:
191-
raise ValueError("Authentication failed. Please check your API key.")
206+
raise ValueError(
207+
"Authentication failed. Please check your API key."
208+
)
192209
elif response.status_code == 403:
193-
raise ValueError("Access forbidden. Please check your permissions.")
194-
210+
raise ValueError(
211+
"Access forbidden. Please check your permissions."
212+
)
213+
195214
return response
196215

197216

198217
def create_client(
199-
endpoint: Optional[str] = None,
200-
api_key: Optional[str] = None,
201-
**kwargs
218+
endpoint: Optional[str] = None, api_key: Optional[str] = None, **kwargs
202219
) -> Union[CuOptServiceWebHostedClient, CuOptServiceSelfHostClient]:
203220
"""
204221
Factory function to create appropriate client based on parameters.
205-
222+
206223
Creates CuOptServiceWebHostedClient if endpoint is provided, otherwise
207224
creates CuOptServiceSelfHostClient for legacy ip/port usage.
208-
225+
209226
Parameters
210227
----------
211228
endpoint : str, optional
212229
Full endpoint URL. If provided, creates a web-hosted client.
213230
Required for web-hosted client creation.
214231
api_key : str, optional
215-
API key for web-hosted client authentication. Will be sent as Bearer token.
232+
API key for web-hosted client authentication.
233+
Will be sent as Bearer token.
216234
**kwargs
217235
Additional parameters passed to the selected client
218-
236+
219237
Returns
220238
-------
221239
CuOptServiceWebHostedClient or CuOptServiceSelfHostClient
222240
Web-hosted client if endpoint provided, self-hosted client otherwise
223-
241+
224242
Examples
225243
--------
226244
# Creates web-hosted client
227245
client = create_client(
228246
endpoint="https://api.nvidia.com/cuopt/v1",
229247
api_key="your-key"
230248
)
231-
249+
232250
# Creates self-hosted client
233251
client = create_client(ip="192.168.1.100", port="5000")
234252
"""
235253
if endpoint:
236254
# Create web-hosted client - endpoint is required
237255
return CuOptServiceWebHostedClient(
238-
endpoint=endpoint,
239-
api_key=api_key,
240-
**kwargs
256+
endpoint=endpoint, api_key=api_key, **kwargs
241257
)
242258
elif api_key:
243259
# Authentication provided but no endpoint - this is an error

0 commit comments

Comments
 (0)