Fixed ResourceType handling and other small improvements

[abhijit914]

Summary

• Stop pluralizing data_source-style inputs and normalize resource-type path segments through the canonical enum values.
• Align flow metrics, flow logs, and resource audit log endpoints with the actual API paths used by the SDK.
• Update FlowLogsResponse to parse the live nested logs.data / logs.meta payload shape and map live log fields onto the SDK model.
• Tighten tests and mock builders to cover canonical enum inputs, singular-type rejection, and the live logs response shape.

Testing

• Unit coverage updated for flows, metrics, organizations, and the live flow-log model shape.
• Full unit suite passed locally, along with lint and formatting checks.
• Live SDK validation against the Nexla API confirmed real metrics and logs payloads for the supplied source and flow IDs.

---

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 5 additional findings.

[saksham-nexla]

Review Summary

This PR does several things well: the ResourceType enum-based normalization is a clean approach to preventing singular/plural typos in URL path segments, the AliasChoices upgrade to FlowLogsMeta correctly handles multiple live API response shapes, and the normalize_live_logs_shape validator elegantly flattens the nested logs.data / logs.meta payload without breaking the existing flat-list response shape.

Key Concerns

[blocker] get_run_status is a silent breaking change.
The old signature get_run_status(resource_type, resource_id, run_id) is replaced by get_run_status(flow_id, run_id). A caller passing the old three-argument form gets no TypeError — "data_sources" is silently treated as flow_id. This needs either a deprecation shim or an explicit breaking-change notice in the PR description and changelog.

[major] FlowLogEntry.timestamp + mock integer timestamps will silently produce wrong datetimes.
The mock builder now emits 1_700_000_000_000 (a millisecond epoch integer) for timestamp, but the Pydantic v2 field is Optional[datetime], which coerces integers as seconds — yielding a date around year 55843. No test ever asserts on the parsed timestamp value, so the bug is invisible in CI. A @field_validator is needed to divide by 1000 when the value looks like a ms integer, or the mock should revert to ISO-format strings. (See inline comments on responses.py:28 and mock_builders.py:509.)

[major] ResourceType(resource_type).value is copy-pasted across 9+ methods.
Factoring it into a single _resolve_resource_type() helper would centralise the validation logic and allow a friendlier error message than the raw enum ValueError. (See inline comment on flows.py:102.)

Smaller Items

• FlowLogsMeta gains org_id and run_id fields with no documentation or tests.
• The "pages_count" alias (a third spelling for page_count) is unexplained — a one-line comment noting it comes from the live API response would prevent future confusion.
• test_flows.py has no test that invalid resource-type strings are rejected before an HTTP call is made (covered in test_metrics.py but not mirrored here).
• The meta precedence rule in normalize_live_logs_shape is correct but deserves a one-line comment to make the intent clear.

Verdict: Request Changes — the get_run_status breaking change and the timestamp type mismatch both need to be resolved before merging.

---

Generated by Claude Code

[abhijit914]

@saksham-nexla thanks for the review. Addressed the comments.

Key changes:

• Added shared enum normalization in base_resource.py, then replaced the repeated ResourceType(...) / UserMetricResourceType(...) calls across flows, metrics, orgs, and users.
• Added a compatibility guard for flows.get_run_status("data_sources", id, run_id): it now raises a clear DeprecationWarning before any HTTP call. New correct usage remains flows.get_run_status(flow_id, run_id).
• Added explicit millisecond timestamp parsing for live flow log timestamps.
• Documented pages_count, org_id, and run_id in FlowLogsMeta, and added tests that assert they parse.
• Added flow resource-type rejection tests so singular strings fail before HTTP calls.

[devin-ai-integration]

Devin Review found 1 new potential issue.

View 6 additional findings in Devin Review.

[saksham-nexla]

Review Summary

What's done well

The core fix is solid. Routing ResourceType enum values through _resolve_enum_value gives a single, tested validation point that rejects singular strings ("data_source") and accepts both enum members and canonical string values. The FlowLogsResponse model-validator approach to flatten the live logs.data / logs.meta shape is clean and backward-compatible. The AliasChoices usage in FlowLogsMeta correctly handles the camelCase / snake_case / live-API spelling variations without duplicating logic. Test coverage for the new enum paths and the live logs shape is well-structured.

Key concerns

Blocker — raise DeprecationWarning(...) in get_run_status
DeprecationWarning is a Warning subclass, not an Exception. Raising it directly makes it propagate as an unhandled exception instead of participating in Python's standard warning-filter machinery. The test reinforces this wrong behavior by using pytest.raises. This needs to become warnings.warn(..., DeprecationWarning, stacklevel=2) followed by a TypeError to hard-stop the call; the test needs to become pytest.warns(DeprecationWarning). See inline comment on line 509.

Major — Hard breaking change with no migration period
get_run_status completely replaces its public signature and immediately fails callers using the old three-argument form. For a published SDK, the conventional approach is to warn-but-work for one minor release, then hard-fail in the next. If the Nexla API has already removed the old endpoint, document that in a CHANGELOG entry and couple it with a semver bump.

Major — _resolve_resource_type in BaseResource creates unnecessary coupling
BaseResource is the root of every SDK resource, yet this method imports and references ResourceType from the metrics/flows domain. The generic _resolve_enum_value belongs in the base; the domain-specific wrapper should live in FlowsResource, MetricsResource, and OrganizationsResource.

Minor / nit issues

• FlowLogsResponse.normalize_live_logs_shape: prefer logs.get("data", []) over logs.get("data") or [] for clarity.
• coerce_ms_timestamp: the 1e10 threshold should have a brief comment explaining the year-2286 invariant; add a test for seconds-based integer timestamps passing through unchanged.
• Bare except Exception in get_logs / get_metrics silently swallows Pydantic ValidationErrors — add a logger.debug call so model drift is visible.
• mdx_text in the docs generator escapes braces inside code examples in docstrings; scope the escaping to prose-only lines.
• Mock flow_log_entry builder drops run_id and details fields that are still declared Optional on the model, reducing incidental test coverage.

Verdict

Request changes — the raise DeprecationWarning bug is a correctness issue that will break callers in a confusing way and should be fixed before merging. The breaking-change governance question is worth an explicit decision before shipping.

---

Generated by Claude Code

[ameyitis]

Deep audit follow-up. The existing review threads cover most of the larger issues (DeprecationWarning misuse, breaking-change framing, ms-timestamp validator, helper placement). Filing six additional inline points that aren't yet covered — most are minor refinements; the type-lie in get_run_status and the duplicated log/message fields are the most concrete.

[question] One additional point that doesn't anchor cleanly on a changed line: nexla_sdk/resources/metrics.py:134 (get_flow_metrics_summary) still uses path = f"/data_flows/metrics/{period}". Every other /data_flows/... URL in this PR was rewritten to live under /{resource_type}/{resource_id}/flow/.... Is this endpoint intentionally a real survivor, or was it missed during the audit? If it's genuine, a one-line comment there would prevent future cleanup attempts; if it's a miss, this PR should fix it too — otherwise we'll ship inconsistent path conventions for the same conceptual area.

[abhijit914]

Thanks @saksham-nexla @ameyitis. I addressed the latest review pass:

• Removed the legacy escape hatch from flows.get_run_status; the public signature is now cleanly get_run_status(flow_id: int, run_id: int), so generated docs/MCP tools no longer expose fake legacy params.
• Deliberately kept this as a breaking change: I tested the old resource-based run-status paths against real flow/source/run data and they returned 404, while the new /flows/{flow_id}/run_status/{run_id} endpoint returned successfully. Since the old SDK signature pointed at a non-working endpoint, preserving it would just keep broken API surface area.
• Kept OrganizationsResource.get_resource_audit_log constrained to the current ResourceType enum for now based on our decision.
• Added metrics-facing typed response models, ResourceFlowMetricsResponse and ResourceFlowLogsResponse, implemented as thin subclasses of the existing flow response models so we avoid duplicate schemas while giving callers a cleaner metrics-resource interface.
• Updated MetricsResource.get_flow_metrics / get_flow_logs to return those typed models when the response matches, with raw dict fallback for unexpected response shapes.
• Regenerated the checked-in API docs so the signatures and new response models are reflected in the docs.

Validation run: ruff, focused unit tests, full unit suite, docs build, live smoke checks for flow logs/metrics.