chore(uber): delete unlaunched Uber Ride Requests integration#376
Conversation
Migrate the Uber Ride Requests integration from autohive-integrations-sdk 1.0.2 to 2.0.0. Source (uber.py): - Unwrap FetchResponse.data inside the uber_fetch / uber_fetch_v1 helpers so the action layer is unchanged (renamed the helper-local var to avoid the fetch-pattern linter false positive on callers named `response`). - Convert both error paths in the handle_uber_errors decorator to return ActionError(message=...); error classification preserved in the message. config.json: - Bump version to 2.0.0; drop the error/error_type properties from every output schema (no longer returned). - Make output fields the code can emit as null actually nullable (driver/vehicle/eta/surge_multiplier, href, last_used, whole-body objects). SDK 2.0 validates output, and a freshly-requested ride legitimately returns null driver/vehicle — without this the action would raise a runtime VALIDATION_ERROR. requirements.txt: pin autohive-integrations-sdk~=2.0.0. Tests: - Replace the old test_uber.py/context.py shim with test_uber_unit.py (78 tests, all 15 actions, via execute_action + FetchResponse mocks) and test_uber_integration.py (live live_context fixture, read-only + a destructive request/status/map/cancel lifecycle). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 268c3fc59b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| data = await resp.json(content_type=None) | ||
| except Exception: | ||
| data = await resp.text() | ||
| return FetchResponse(status=resp.status, headers=dict(resp.headers), data=data) |
There was a problem hiding this comment.
When the live Uber API returns any non-2xx response (expired token, missing scope, or a restricted/deprecated endpoint), this mock still returns a FetchResponse; SDK 2's ExecutionContext.fetch raises HTTPError/RateLimitError in that situation. The action helpers can then consume the error body as normal data and return ResultType.ACTION with empty defaults, so _data_or_skip will not skip/fail and the new integration suite can pass while every live call is actually failing. Please raise on failed resp.status here, matching the SDK contract.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 37f2487 — real_fetch now matches the SDK contract (raises RateLimitError/HTTPError on non-2xx), so failed live calls surface as ACTION_ERROR and _data_or_skip skips instead of passing on empty defaults. Thanks!
…ion warnings Addresses Codex P2 and the structure/code-check warnings on PR #376. - tests: the integration `real_fetch` mock now raises `HTTPError`/`RateLimitError` on non-2xx, matching `ExecutionContext.fetch`. Previously it returned a FetchResponse for error bodies, so a failed live call (expired token, missing scope, deprecated endpoint) would be consumed as normal data and surface as an empty-but-successful ACTION result — letting the suite pass while every live call was actually failing. - config: dropped the unused `places` OAuth scope (no places endpoint is called). - uber.py: access required params via `inputs["x"]` instead of `.get()` (SDK validates required inputs before execute, so they are guaranteed present) and build the optional-field maps with explicit `inputs.get("literal")` calls. Both silence config-code sync's false "never accessed"/"safe for missing" warnings; behavior is unchanged. Structure + code checks now report 0 warnings. Verified: 78/78 unit tests pass; integration suite collects & skips without a token; validate_integration.py and check_code.py both clean (0 warnings). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two issues caught by a self-review pass on the branch. 1. Output schema — array items could throw VALIDATION_ERROR on a 200. The migration made top-level output fields nullable but left the array-item sub-fields strict. Uber legitimately returns explicit null inside list items (low_estimate/high_estimate/duration for metered/TAXI fares; end_time/distance for in-progress rides in history; payment-method description; product capacity/image). Since these fields aren't `required` a *missing* key was fine, but an explicit null fails Draft7 output validation and turns a successful call into ResultType.VALIDATION_ERROR. Made the affected item fields accept null across get_products, get_price_estimate, get_time_estimate, get_ride_history, and get_payment_methods. Identifiers (product_id, request_id, payment_method_id) stay strict. 2. Integration test — destructive lifecycle could orphan a booked ride. Now that real_fetch raises on non-2xx (prior commit), a transient error on the status/map step surfaces as ACTION_ERROR and _data_or_skip would exit before cancel_ride ran, leaving a real ride uncancelled. Moved the cancel into a try/finally so cleanup always runs once request_id is known. Verified: 78/78 unit tests pass; integration suite collects & skips without a token; validate_integration.py and check_code.py both clean (0 warnings). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Pushed
On |
TheRealAgentK
left a comment
TheRealAgentK
left a comment
There was a problem hiding this comment.
@sumitramanga FYI: this integration appears to have no registered Uber OAuth provider and has never been launched. Before we merge/launch this, someone should decide whether we’re registering the provider and requesting the required privileged Uber scopes, or keeping this unlaunched/limiting the supported actions.
|
|
||
| pytestmark = pytest.mark.integration | ||
|
|
||
| ACCESS_TOKEN = os.environ.get("UBER_ACCESS_TOKEN", "") |
There was a problem hiding this comment.
Please add UBER_ACCESS_TOKEN and UBER_API_BASE_URL to the root .env.example.
| also set UBER_API_BASE_URL=https://sandbox-api.uber.com. | ||
|
|
||
| Run all read-only tests: | ||
| pytest uber/tests/test_uber_integration.py -m integration |
There was a problem hiding this comment.
This command also runs the destructive ride-booking test. Please make the read-only command -m "integration and not destructive".
| "profile", | ||
| "history", | ||
| "places", | ||
| "partner-loyalty.link-account" |
There was a problem hiding this comment.
These scopes don’t cover ride requests or receipts. Please add Uber’s required request / request_receipt scopes, or remove/disable the unsupported actions.
| @pytest.mark.asyncio | ||
| async def test_success(self): | ||
| ctx = make_ctx({"total_charged": 15.50, "currency_code": "USD"}) | ||
| result = await uber_integration.execute_action("get_ride_receipt", {"request_id": "req_123"}, ctx) |
There was a problem hiding this comment.
Please add a URL assertion here too: the Riders API receipt endpoint is /requests/{request_id}/receipts (plural), but the action currently calls singular /receipt.
The Uber integration has no registered OAuth provider and has never been launched. Per review, drop it instead of upgrading: remove the uber/ directory and its entry from the repo README. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Tram-Nguyen87
changed the title
TheRealAgentK
changed the title
2 participants
Summary
Reworked from the original SDK 2.0.0 upgrade into a removal PR.
Per @TheRealAgentK's review, the Uber Ride Requests integration has no registered Uber OAuth provider and has never been launched. Rather than register the provider and request the privileged Uber scopes to launch it, we're dropping the integration.
Changes
uber/directory (source, config, tests, icon)README.mdmasterso the diff shows only the Uber removalNotes
TEST_STOCKTAKE.mdstill referencesuberin two places, but it's a point-in-time test inventory snapshot, not the live README — left untouched (out of scope of the README purge).