Skip to content

Enahcements: zoho creator v2#135

Open
royred98 wants to merge 3 commits into
prod/source-zoho-creatorfrom
feature/zoho-creator-v2
Open

Enahcements: zoho creator v2#135
royred98 wants to merge 3 commits into
prod/source-zoho-creatorfrom
feature/zoho-creator-v2

Conversation

@royred98
Copy link
Copy Markdown

What

Enhancements made to the zoho creator connector: Major bug fixes & performance improvements

How

  • Long running syncs for high-volume reports was failing due to auth token refresh issue
  • For incremental streams with zero new records sync was failing due to zoho returning http 400 for this condition, airbyte by default marks 4xx codes as failures
  • Rate limit hit & backoff due to unnecessary data api hitting due to schema fetch
  • Sluggish performance due to unnecessary overhead in method calls
  • Unnecessary console writes clogging up logs

Review guide

  • auth token refresh now delegated to 3rd party library, which has superior refresh logic & safeguards
  • zoho return codes captured & return codes for 0 new records condition is checked & explictly sync marked success.
  • schema is constructed once during discover & cached. subsequent schema reads are sent from cache
  • unnecessary & redundent code removed.

User Impact

  • High-volume reports long running syncs running without failing.
  • incremental syncs with zero new records are now marked success with 0 records synced.
  • Improved performance overall

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

royred98 added 3 commits March 9, 2026 01:49
1. Auth token refresh failure for long-running sycs:
-token refresh was previously being handled manually, but this logic was failing. Now we are using a library zoho_creator_sdk for the auth handling. This is tested to be working for long running sycs requirng token refresh.

2. Incremental streams with zero-records marked as sync failure:
-zoho API return http 400 for no records matching given criteria i.e. no new records after the latest stream state. 400 by default is marked as fail by airbyte. we have employed a system error handler get_error_handler to work around this. Http 400-code 9280 is the exact response zoho sends in this case which our error handler catches and marks as success. thus, sync doesnt fail but is marked success with 0 records synced.
Productionize Zoho Creator connector: bug fixes, hardening & test overhaul

Bug fixes:
- Fix incremental sync failing on "no new records": override get_error_handler()
  in CDK 7.x (raise_on_http_errors is ignored) to remap HTTP 404 + code 3100
  and HTTP 400 + code 9280 to ResponseAction.SUCCESS so parse_response() handles
  them as empty result sets instead of failures
- Remove debug print(reports) that was writing raw Python to stdout, causing
  "Malformed non-Airbyte record" and failing every sync
- Fix get_application_reports() pagination params (from_index/limit) that Zoho
  rejected with HTTP 401 — the meta endpoint is non-paginated

Hardening (streams.py):
- Add cursor field whitelist + date format validation in request_params() to
  prevent criteria injection via corrupted/tampered state
- Extract _is_auth_error() helper to eliminate duplicated auth detection logic
- Guard next_page_token() against paginating on remapped 400/404 responses
- Add isinstance(list) guard on code 3000 data payload
- Downgrade noisy Params/Updated-state logs from INFO to DEBUG

Hardening (api.py):
- Validate datacenter in __init__; raise ZohoCreatorConfigError with clear
  message instead of crashing with KeyError
- Handle Zoho code 9280 (HTTP 400) as a valid empty state in get_report_data()
- Add instance-level _schema_cache to deduplicate schema inference API calls

Hardening (source.py):
- Catch ZohoCreatorConfigError in check_connection() to surface invalid
  datacenter as (False, error_msg) instead of an unhandled exception

Tests (62 → 82, all passing):
- Rewrite TestZohoCreatorAPIReportData: was mocking dead SDK path, now patches
  requests.get correctly
- Add TestZohoCreatorAPIDatacenterValidation, TestZohoCreatorAPIApplicationReports,
  TestZohoCreatorAPIReportSchema (13 new tests)
- Add TestZohoCreatorStreamAuthErrorDetection, TestZohoCreatorStreamCriteriaInjection
  (6 new tests)
- Fix test_source.py: remove real network calls in empty-field tests, replace
  stale list_applications mocks with get_application_reports throughout
@@ -0,0 +1,244 @@
# CLAUDE.md
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this reference folder needed ?

can we remove the .txt & .md files ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kept it so that for further iterations, the ref file would be useful context for the LLM to understand the working of the connector.
This can be removed.


found_cursor_fields = []

for cursor_field in self.DEFAULT_CURSOR_FIELDS:
Copy link
Copy Markdown
Member

@himanshudube97 himanshudube97 May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image Image

As per above two images->
a. cursor fields cannot be composite unlike primary keys.
b. two cursor fields in an array means they are nested -> thats why we say that Added_time.Modified_time , which is causing the error -> Only top-level cursors are supported"

--> Now as per this code if the cursor_fields present in schema are also in DEFAULT_CURSOR_FIELDS, then it will append both values.
--> in case of our Catalyse_Tracker_Catalyse stream, if you check you will see we have both Added_time and Modified_time columns, which by this logic is making a nested cursor field.
So we will have to fix this.

just stop the loop at one - prioritize modified rather then added -> my opinion. You can think little more or this.

@royred98

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants