fix(zoom): derive start date from sync time; make manifest strictly valid - #1746
Merged
mitasovr merged 10 commits intoJul 13, 2026
Merged
Conversation
…alid The Zoom Dashboard API (/v2/metrics/*) only serves the last six months. With a fixed zoom_start_date the pre-sync connection check always probes the first slice from that date, and started failing with HTTP 400 (code 300) once the configured date fell out of the window — every sync job died at check (dev-vhc job 529) even though the connection state was well inside the window. Replace the config knob with a computed start (now - 150 days, a safety margin inside the window) and drop zoom_start_date from the spec, descriptor required_fields, secret example and README. Extra zoom_start_date fields in existing K8s secrets remain harmless (additionalProperties: true). Also make the manifest Builder-UI strictly valid (validate-strict): inline the typed-object $refs (login_requester request_body, error handlers, paginators, authenticators) and replace the whole-object parent $ref "#/streams/1" on participants with an inline _meetings parent stream, following the youtrack pattern. Bump descriptor version to 1.1.0 so reconcile republishes the manifest. Verified: validate-strict and validate green; live check green; full read against dev-vhc credentials — meetings 24525, users 375, participants 77085 records. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe Zoom connector removes ChangesZoom connector and metric validation
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/ingestion/connectors/collaboration/zoom/connector.yaml`:
- Around line 117-163: Restore the users stream’s request_parameters in the
inlined requester, including the configured page_size parameter, while
preserving the existing GET URL, error handling, and pagination configuration.
- Around line 572-683: Update the ParentStreamConfig for the incremental
_meetings stream to set incremental_dependency: true. Ensure this configuration
is applied to the participants fan-out relationship so parent incremental state
advances with child processing while preserving the existing 150-day window and
pagination settings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e59977e7-af1e-4255-9b91-79bca44a3386
📒 Files selected for processing (4)
src/ingestion/connectors/collaboration/zoom/README.mdsrc/ingestion/connectors/collaboration/zoom/connector.yamlsrc/ingestion/connectors/collaboration/zoom/descriptor.yamlsrc/ingestion/secrets/connectors/zoom.yaml.example
💤 Files with no reviewable changes (1)
- src/ingestion/secrets/connectors/zoom.yaml.example
ktursunov
requested changes
Jul 13, 2026
ktursunov
left a comment
ktursunov
left a comment
Contributor
There was a problem hiding this comment.
Zoom metrics are not covered with tests
… zoom fixes Review follow-ups (CodeRabbit on constructorfabric#1746): - users stream now sends page_size (was never applied to this stream, also not on main — inherited from before the inlining) - participants ParentStreamConfig sets incremental_dependency: true so the substream persists/reuses the _meetings parent cursor (verified the SubstreamPartitionRouter parent-state mechanics in CDK 6.60.9 sources) - both changes verified with a live full read: meetings 24780 / users 374 / participants 77093 e2e coverage for every FE metric fed by Zoom data (metric catalog keys with source_tag zoom): - collab_zoom_meeting_hours: per-day multi-meeting sum, re-emit dedup, camera/share modality gating - collab_zoom_meetings: distinct-meeting count incl. session stitching (a 2-min host-drop rejoin counts as ONE meeting) - collab_meeting_hours_zoom_cross / collab_meetings_count_zoom_cross: Teams+Zoom additivity (distribution shifted so a dropped Zoom leg fails every asserted field) - collab_meeting_free_zoom: Zoom activity cancels a meeting-free day - zoom_meeting_hours / zoom_meetings removed from the coverage-gate SKIP_LIST — coverage is now enforced Two e2e-rig bugs found and fixed on the way: - bronze_zoom placeholders diverged from the promoted prod shape: RMT ORDER BY email (participants) / uuid (meetings) with no unique_key column, silently collapsing all of a person's rows on merge — deployed tables are unaffected (promote_bronze_to_rmt produces ORDER BY unique_key; verified on dev-vhc), but the rig runs on the placeholder and could not seed more than one meeting per person. Placeholders now mirror the promoted shape; the stale workaround note in collab_meetings_count.test.yaml is rewritten. - warm-rerun contamination: bronze_zoom.meetings is read (via the zoom__meeting_sessions upstream) by tests that seed only participants, so it was neither seed-truncated nor ledger-truncated; a prior session's leftover rows were re-appended into zoom__meeting_sessions on every such build, tripping its unique dbt test on the second one. bronze_zoom.* added to the conftest session-start truncate list. Verified: cold full e2e suite 155 passed (CI scenario); the previously failing warm sequence (zoom subset, then full suite, no down) now 5 + 155 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>
…ovr/insight into claude/airbyte-job-529-logs-26ea7d
Mock-server tests for collaboration/zoom modeled on the task-tracking/jira reference suite (15 tests: users 6, meetings 5, participants 4), including the job-529 regression pin: with the clock frozen, every meetings slice is matched by an exact from/to matcher and the first slice starts at now-150d — a connector regressing to a static / out-of-window start date (the original zoom_start_date=2026-01-01 failure) issues a request no matcher accepts and the test fails. Also covered: S2S OAuth token exchange (byte-exact form body), per-stream stamping/schema/pagination/empty-page, 429 retry, incremental state + P7D-lookback resume filtering, participants partition-per-meeting with uuid URL-escaping. The suite disproved one review follow-up from this PR: incremental_dependency on the participants ParentStreamConfig is a silent no-op — parent_state only piggybacks on an incremental child's state, and participants is full-refresh (emits only __ab_no_cursor_state_message; the earlier live read agrees). The flag is removed again with an explanatory manifest comment, and test_full_refresh_substream_emits_no_cursor_state pins the actual contract. CDK facts the suite encodes: the concurrent cursor absorbs the window tail into the last slice while the synchronous parent-read path emits a 1-day tail slice (both layouts in tests/config.py); POST matchers compare the body byte-exact. Verified: harness run 35 passed / 1 skipped (pre-existing jira schema-drift skip); validate-strict + validate green after the manifest change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>
cyberantonz
reviewed
Jul 13, 2026
ktursunov
approved these changes
Jul 13, 2026
cyberantonz
reviewed
Jul 13, 2026
| from pathlib import Path | ||
|
|
||
| # Local builder modules (config.py) are importable under --import-mode=importlib. | ||
| sys.path.insert(0, str(Path(__file__).parent)) |
Contributor
There was a problem hiding this comment.
Why claude likes to modify syspath? I think there is easier import fix like from . import...
Contributor
Author
There was a problem hiding this comment.
seems like copypaste
will fix it later
Contributor
There was a problem hiding this comment.
Now it is later :) Still not fixed
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/ingestion/connectors/collaboration/zoom/connector.yaml (1)
580-691: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffSignificant duplication of requester/auth/error/paginator config across 4 stream definitions.
The
_meetingsnested stream (lines 588–658) is a near-exact copy of themeetingsrequester (lines 303–373), and theparticipantsrequester (lines 707–768) duplicates the same authenticator/error_handler pattern a third time. While Builder-UI strict validation requires inlining typed-object references, this means any change to auth, error handling, or pagination must be mirrored in 4 places — creating real drift risk.Consider whether the
_meetingsnested stream could reuse the mainmeetingsstream definition via a different reference mechanism, or at minimum add a comment cross-referencing the duplicated blocks so future changes are applied consistently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ingestion/connectors/collaboration/zoom/connector.yaml` around lines 580 - 691, The requester, authenticator, error-handler, and paginator configuration in the `_meetings` DeclarativeStream duplicates the corresponding `meetings` and `participants` definitions. Reuse the existing `meetings` configuration through a supported reference mechanism if strict validation permits; otherwise retain the inline typed objects and add a concise cross-reference comment to each duplicated block instructing future changes to stay synchronized.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/ingestion/connectors/collaboration/zoom/connector.yaml`:
- Around line 580-691: The requester, authenticator, error-handler, and
paginator configuration in the `_meetings` DeclarativeStream duplicates the
corresponding `meetings` and `participants` definitions. Reuse the existing
`meetings` configuration through a supported reference mechanism if strict
validation permits; otherwise retain the inline typed objects and add a concise
cross-reference comment to each duplicated block instructing future changes to
stay synchronized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 108465a3-513a-408f-bfd9-069098b6e3ef
📒 Files selected for processing (10)
src/ingestion/connectors/collaboration/zoom/connector.yamlsrc/ingestion/connectors/collaboration/zoom/tests/config.pysrc/ingestion/connectors/collaboration/zoom/tests/conftest.pysrc/ingestion/connectors/collaboration/zoom/tests/fixtures/meeting.jsonsrc/ingestion/connectors/collaboration/zoom/tests/fixtures/participant.jsonsrc/ingestion/connectors/collaboration/zoom/tests/fixtures/user.jsonsrc/ingestion/connectors/collaboration/zoom/tests/test_meetings.pysrc/ingestion/connectors/collaboration/zoom/tests/test_participants.pysrc/ingestion/connectors/collaboration/zoom/tests/test_users.pysrc/ingestion/scripts/create-bronze-placeholders.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- src/ingestion/scripts/create-bronze-placeholders.sh
…f config Applied ruff 0.15.21 (the .pre-commit-config.yaml pin) check --fix + format with the root ruff.toml (isort I rules, line-length 120, skip-magic-trailing-comma). Suite unchanged behaviorally: 15/15 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>
…ovr/insight into claude/airbyte-job-529-logs-26ea7d
cyberantonz
approved these changes
Jul 13, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Zoom syncs on dev-vhc fail at the pre-sync connection check (e.g. Airbyte job 529):
The Zoom Dashboard API (
/v2/metrics/*) only serves the last six months. Themeetingsstream built its first slice from the staticzoom_start_dateconfig (2026-01-01), and the Airbytecheckcommand receives no state — so once the configured date fell out of the window, every sync died at check, even though the connection's saved cursor (end_time: 2026-06-29) was perfectly valid.Fix
meetings.start_datetimeis now computed:now_utc() - P150D(~5 months — a safety margin inside Zoom's window, whose month-granularity semantics are ambiguous at the boundary). Same pattern as m365.zoom_start_dateremoved from the connector spec, descriptorrequired_fields, secret example, and README. Existing K8s secrets that still carry the field stay valid (additionalProperties: true), so no secret rotation is needed.Also: Builder-UI strict validation
validate-strictfailed on main with 3 errors (and more masked behind them — the validator surfaces one leaf error per stream). The strict validator performs no $ref resolution, so any typed-object slot holding a $ref fails. Fixed by inlining:request_bodyin bothlogin_requesters,error_handler/paginatorinusers,authenticatorinparticipants,$ref: "#/streams/1"onparticipantsreplaced with an inline_meetingsparent stream (youtrack pattern).Note:
collaboration/m365currently failsvalidate-strictwith the samelogin_requester.request_body$ref pattern (4 streams) — left out of scope here.Verification
source.sh validate-strict collaboration/zoom— strictly validsource.sh validate collaboration/zoom— manifest validsource.sh checkwith real dev-vhc credentials — Check succeeded (first slice now inside the window)source.sh readwith real credentials — meetings 24 525, users 375, participants 77 085 records; the inline_meetingsparent routes partitions correctly. (One transient 400 mid-pagination of a 30-day slice — Zoom'snext_page_token15-min expiry during the 150-day cold read; pagination behavior unchanged from main.)Existing connection state is untouched; syncs continue from their saved cursor.
🤖 Generated with Claude Code
Summary by CodeRabbit
Updates
zoom_start_dateconfiguration requirement, and updated the connector version, documentation, and credential secret example accordingly.Improvements
Tests
Chores