feat: expand Python SDK coverage and clean up repo scaffolding#12
feat: expand Python SDK coverage and clean up repo scaffolding#12saksham-nexla wants to merge 15 commits intomainfrom
Conversation
- Deleted the coverage report file `lcov.info` from the TypeScript SDK. - Removed the `test_auth_param_import.py` script that verified auth_parameters imports. - Deleted the `verify_type_checking.py` script that tested TYPE_CHECKING pattern implementation. - Added unit tests for `NexlaClient` initialization and model conversion in `tests/unit/test_client_init.py`. - Updated `package.json` to include additional metadata and scripts for building and testing.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| - name: Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 20 | ||
| cache: 'pnpm' | ||
|
|
||
| - name: Setup pnpm | ||
| uses: pnpm/action-setup@v4 | ||
| with: | ||
| version: 9 |
There was a problem hiding this comment.
🔴 CI workflows install pnpm after setup-node cache, causing cache setup failure
All three TypeScript CI workflows (ci-ts.yml, ci-ts-integration.yml, release-ts.yml) run actions/setup-node@v4 with cache: 'pnpm' before pnpm/action-setup@v4. The actions/setup-node cache feature for pnpm requires pnpm to already be installed so it can run pnpm store path to determine the cache directory. The correct order is: (1) install pnpm via pnpm/action-setup, then (2) setup Node.js with cache. This is documented in both the setup-node and pnpm/action-setup READMEs. The steps should be swapped in all three workflows.
| - name: Setup Node.js | |
| uses: actions/setup-node@v4 | |
| with: | |
| node-version: 20 | |
| cache: 'pnpm' | |
| - name: Setup pnpm | |
| uses: pnpm/action-setup@v4 | |
| with: | |
| version: 9 | |
| - name: Setup pnpm | |
| uses: pnpm/action-setup@v4 | |
| with: | |
| version: 9 | |
| - name: Setup Node.js | |
| uses: actions/setup-node@v4 | |
| with: | |
| node-version: 20 | |
| cache: 'pnpm' |
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| # Build OpenAPI/admin-routes/SDK parity matrices | ||
| python scripts/parity/build_matrices.py \ | ||
| --admin-routes /Users/sakshammittal/Documents/GitHub/admin-api/config/routes.rb |
There was a problem hiding this comment.
🟡 Hardcoded local filesystem path in README and parity script violates AGENTS.md security rules
The README.md at line 732 contains a hardcoded local path /Users/sakshammittal/Documents/GitHub/admin-api/config/routes.rb, and scripts/parity/build_matrices.py:230-235 uses the same path as a default. AGENTS.md explicitly states: "Do not hardcode tokens or URLs; read from env". The path should use an environment variable (e.g., NEXLA_ADMIN_API_PATH) with a documented placeholder instead of a developer's local machine path.
| --admin-routes /Users/sakshammittal/Documents/GitHub/admin-api/config/routes.rb | |
| --admin-routes "$NEXLA_ADMIN_API_PATH/config/routes.rb" |
Was this helpful? React with 👍 or 👎 to provide feedback.
Merges 3 PRs from main: - #13: Make DataMapInfo.description and NotificationSetting.resource_id optional - #14: Add monitoring endpoint methods with explicit params for MCP integration - #15: Add copy_and_replace_credentials convenience method to FlowsResource Resolved conflicts in base_resource.py, metrics.py, organizations.py, and users.py by keeping branch's additional methods while incorporating main's enhanced audit_log params and new monitoring methods.
The reject endpoint uses DELETE, not PUT. Updated the HTTP method and added a test assertion to verify the correct method is used.
POST requests with a body failed on retry because the original request body was already consumed. Now clone on every attempt, not just retries. Added a test for POST retry with body.
Add TS equivalents for Python SDK methods merged from main: - FlowsResource: copy_and_replace_credentials convenience method - OrganizationsResource: get_audit_log - UsersResource: get_audit_log
Add 36 new tests (29 Python, 7 TypeScript) covering methods from merged PRs #13, #14, #15 and their TS SDK mirrors: Python: - base_resource get_audit_log filter/pagination params (6 tests) - metrics publish_raw, resource_flow_metrics, flow_metrics_summary (4) - organizations audit_log explicit params, flow_status_metrics (5) - users audit_log params, account_metrics aggregate, flow_status_metrics (6) - flows get_flow_logs, search_flow_logs, active_flows_metrics, run_status (8) TypeScript: - flows copy_and_replace_credentials edge cases (3) - organizations/users audit_log query params and empty responses (4)
| response = self._make_request( | ||
| "PUT", f"/self_signup_requests/{request_id}/approve" | ||
| "POST", f"/self_signup_requests/{request_id}/approve" | ||
| ) |
There was a problem hiding this comment.
🔴 approve_request uses POST but generated operation map and TS SDK use PUT
The approve_request method was changed from PUT to POST at nexla_sdk/resources/self_signup.py:32, with a comment claiming "Backend routes use POST." However, the auto-generated operation map in this same PR (nexla_sdk/generated/operation_map.py:462) defines approve_self_sign_up_request as method: 'PUT', and the TS SDK generated from the same OpenAPI spec also uses PUT (packages/ts-sdk/src/resources/generated/self_signup_admin.ts:19). This creates an inconsistency within the PR: either the Python SDK is sending the wrong HTTP method (POST instead of PUT), or the operation map and TS SDK are wrong. Either way, the Python SDK and TS SDK will behave differently for the same operation.
Prompt for agents
Resolve the inconsistency between the Python SDK, the generated operation map, and the TypeScript SDK for the approve_self_sign_up_request operation. In nexla_sdk/resources/self_signup.py:31-33 the method is POST. In nexla_sdk/generated/operation_map.py:462 the method is PUT. In packages/ts-sdk/src/resources/generated/self_signup_admin.ts:19 the method is PUT. All three should agree. If the backend truly requires POST, update the operation map and regenerate the TS SDK. If the OpenAPI spec (PUT) is correct, revert self_signup.py back to PUT.
Was this helpful? React with 👍 or 👎 to provide feedback.
| path = f"{self._path}/{org_id}/custodians" | ||
| data = self._serialize_data(payload) | ||
| response = self._make_request("PUT", path, json=data) | ||
| if isinstance(response, list): | ||
| return [CustodianUser.model_validate(item) for item in response] | ||
| return [] |
There was a problem hiding this comment.
🔴 add_custodians HTTP method silently changed from POST to PUT, breaking API contract
The add_custodians method's HTTP method was changed from POST to PUT (nexla_sdk/resources/organizations.py:493). The previous implementation used POST which matched the OpenAPI operation add_org_custodians (defined as method: 'POST' in nexla_sdk/generated/operation_map.py:407). The PUT method corresponds to update_org_custodians which has replace/update semantics, not additive semantics. A new reset_custodians method was introduced using POST (line 503), but this means the old add_custodians now silently performs a replace (PUT) instead of an add (POST). Existing callers expecting additive behavior will get replace behavior, potentially removing existing custodians.
Prompt for agents
In nexla_sdk/resources/organizations.py, the add_custodians method on line 493 was changed from POST to PUT, but the OpenAPI operation add_org_custodians (in nexla_sdk/generated/operation_map.py line 407) is defined as POST. Either: (1) Revert add_custodians to use POST (matching the OpenAPI spec and preserving backward compatibility), and keep reset_custodians as POST renamed to something like replace_custodians using PUT. Or (2) If the intent is truly to have add_custodians use PUT for additive semantics, document this breaking change and ensure the operation map is updated accordingly.
Was this helpful? React with 👍 or 👎 to provide feedback.
Resolve five conflicts: - nexla_sdk/models/__init__.py: union both sides; add DocContainerInput. - nexla_sdk/resources/metrics.py: adopt origin's Union[ResourceType,str] + _resolve_enum_value + new flow paths + typed response parsing; keep our publish_raw method and Optional[] typings. - nexla_sdk/resources/organizations.py: combine Optional and Union imports; apply origin's get_resource_audit_log enum coercion; keep all our new methods and reset_custodians. - tests/unit/test_metrics.py: take origin's six new tests inside TestMetricsResource; keep our test_publish_raw and test_get_flow_metrics_summary; drop two stale singular-form tests. - tests/unit/test_organizations.py: union of imports and new tests.
origin/main refactored FlowsResource.get_run_status from (resource_type, resource_id, run_id) to (flow_id, run_id), and added three replacement tests (test_get_run_status_uses_flow_id_path, test_get_run_status_rejects_deprecated_resource_signature, test_get_run_status_rejects_string_flow_id). Our pre-merge test was a duplicate exercising the now-deprecated signature.
Removes JS/TS-only artifacts so this repo houses only the Python SDK: - packages/ts-sdk/ (entire directory) - Top-level workspace configs: package.json, pnpm-workspace.yaml, pnpm-lock.yaml, turbo.json - .changeset/ release config and pending entry - docs/ts-sdk/ (TS-specific architecture/coverage docs) - .github/workflows/ci-ts.yml, ci-ts-integration.yml, release-ts.yml Pre-existing fixes bundled in: - README.md: drop the "TypeScript SDK (New)" section. - pyproject.toml: update project URLs to nexla-opensource/nexla-sdk-python. - .gitignore: drop the standalone Node.js section and the plugin-redoc-0.yaml entry; keep docs-site Docusaurus ignores since the docs site stays in this repo. - .pre-commit-config.yaml: drop the redundant `black` hook that fought with `ruff-format` on a couple of files (ruff-format is Black-compatible and was already in the chain). - nexla_sdk/: apply ruff --fix across the package, add NotificationSettingBrief to nexla_sdk/models/__init__.py __all__ to resolve the last F401, run ruff format + isort across the codebase. - tests/: drop unused `result` assignments and `== True` comparisons flagged by ruff (12 fixes across test_flows, test_organizations, test_users, test_nexsets, etc.). - tests/unit/test_parity_tooling.py: skip cleanly when plugin-redoc-0.yaml is absent (the OpenAPI spec is sourced from upstream and is not committed to this repo). - docs-site/: regenerate Python API MDX docs against the current SDK surface via `python3 docs-site/scripts/gen_api_docs.py` (catches up on previously stale auto-generated documentation). docs-site/ stays in this repo and continues to generate the Python API docs via scripts/gen_api_docs.py.
saksham-nexla
changed the title
The check_operation_map_sync.py CI step was hard-failing on every PR because plugin-redoc-0.yaml is sourced from upstream and not committed, which cancelled the rest of the build-test matrix. Mirror the skipif pattern already used by tests/unit/test_parity_tooling.py: when the spec is missing, log a clear message and exit 0 instead of crashing.
add_custodians had been silently changed from POST to PUT, leaving it
as a literal duplicate of update_custodians (same path, same method,
same body). The OpenAPI spec defines add_org_custodians as POST and
update_org_custodians as PUT on /orgs/{org_id}/custodians, so the
SDK now matches:
- add_custodians: POST (add_org_custodians)
- update_custodians: PUT (update_org_custodians)
- remove_custodians: DELETE (remove_org_custodians)
reset_custodians was a renamed copy of the original POST add semantics
and is removed; no callers reference it. API docs are regenerated to
reflect the trimmed surface.
build_matrices.py defaulted --admin-routes to a personal filesystem path (/Users/sakshammittal/...), which violates AGENTS.md and made the script unusable for anyone else. The env-var fallback is now strict: when neither --admin-routes nor NEXLA_ADMIN_API_PATH is set, the script errors out with a clear message instead of silently pointing at a path that doesn't exist on the user's machine. README updates the parity-tooling example to use $NEXLA_ADMIN_API_PATH.
The PR review surfaced two silent HTTP-method changes (add_custodians POST->PUT and approve_request PUT->POST) that no test caught. Add explicit method assertions so future drift is caught at unit-test time: - test_add_custodians_uses_post / update / remove (POST / PUT / DELETE) - approve_request now asserts last_request method == POST, locking the divergence already documented inline (backend uses POST while the OpenAPI spec advertises PUT in some versions).
pyproject.toml declares requires-python = ">=3.8", but typing.Annotated only landed in 3.9. The Python 3.8 CI job was failing with ImportError: cannot import name 'Annotated' from 'typing'. Pull it from typing_extensions instead, which pydantic v2 already pulls in transitively.
2 participants
Summary
This PR substantially expands the Nexla Python SDK and cleans up repository scaffolding so the repo houses only the Python SDK going forward.
Python SDK enhancements
api_keys,auth_parameters,auth_templates,clusters,cluster_endpoints,connectors,custom_data_flows,dashboard_transforms,data_credentials_groups,flow_triggers,notification_settings,notification_types,org_tiers,quarantine_settings,resource_parameters,service_keys,user_settings,user_tiers,validators,vendors,vendor_endpointsapproval_requestsreject method fromPUTtoDELETEtools,tool_sets,mcp_sessions)Repository cleanup
packages/,pnpm-*,turbo.json,package.json,.changeset/,docs/ts-sdk/, and the JS-specific CI workflowspyproject.tomlURLs tonexla-opensource/nexla-sdk-pythonblackhook in.pre-commit-config.yaml(it fought withruff-formaton edge cases;ruff-formatis Black-compatible and was already in the chain)ruff --fixacrossnexla_sdk/andtests/, addNotificationSettingBrieftonexla_sdk/models/__init__.py__all__to clear F401, drop unusedresultassignments and== Truepatterns in tests (F841/E712), runruff format+isorttests/unit/test_parity_tooling.py: skip cleanly whenplugin-redoc-0.yamlis absent (the OpenAPI spec is sourced from upstream and not committed)docs-site/against the current SDK surfaceTest plan
python -m pytest tests/unit→ 763 passed, 3 skipped (parity-tool tests skip when the spec is absent locally)pre-commit run -aclean and idempotent across two consecutive runsruff check nexla_sdkcleanfrom nexla_sdk import NexlaClientsmokes cleanlydocs-site/scripts/gen_api_docs.pyruns successfully against the new SDK surface