fix(source-facebook-marketing): fix since>until, retry Job Failed, and apply fields_exceptions#75372
fix(source-facebook-marketing): fix since>until, retry Job Failed, and apply fields_exceptions#75372devin-ai-integration[bot] wants to merge 10 commits intomasterfrom
Conversation
…d add retry for Job Failed Combines two independent fixes in _collect_child_ids(): 1. After retention-period clamping, 'since' could be pushed past 'until', causing Facebook API error #100. Now clamps since <= until. (Previously tracked in PR #74707 / oncall#11578) 2. Transient 'Job Failed' / 'Job Skipped' statuses from Facebook's async insights API now trigger up to 3 retries instead of immediately raising. (Previously tracked in PR #74817 / oncall#11643) Co-Authored-By: unknown <>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Note 📝 PR Converted to Draft More info...Thank you for creating this PR. As a policy to protect our engineers' time, Airbyte requires all PRs to be created first in draft status. Your PR has been automatically converted to draft status in respect for this policy. As soon as your PR is ready for formal review, you can proceed to convert the PR to "ready for review" status by clicking the "Ready for review" button at the bottom of the PR page. To skip draft status in future PRs, please include |
Co-Authored-By: unknown <>
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksPR Slash CommandsAirbyte Maintainers (that's you!) can execute the following slash commands on your PR:
📚 Show Repo GuidanceHelpful Resources
|
|
|
/publish-connectors-prerelease |
|
…ge build Co-Authored-By: unknown <>
|
Co-Authored-By: unknown <>
|
Deploy preview for airbyte-docs ready! ✅ Preview Built with commit 24fc6de. |
|
|
|
Due to a bug in the artifact generator (airbytehq/airbyte-ops-mcp#604), previous pre-release publishes from this PR registered an incorrect Corrupted version: Replacement version: Any connections currently pinned to version The underlying bug has been fixed in airbytehq/airbyte-ops-mcp#607 and airbytehq/airbyte#75435. |
|
Devin: #75372 (comment) |
|
Aaron ("AJ") Steers (@aaronsteers) Acknowledged — thanks for flagging the corrupted registry version. The latest working pre-release image is: Updating oncall issue #11643 with this corrected tag so the support team pins to the right version. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
↪️ Triggering Reason: Combined fix for since>until and Job Failed retry. CI fully green (37/37), prerelease image published. Ready for regression validation against customer connections. |
|
Fix Validation EvidenceOutcome: Fix Proven Successfully Evidence SummaryTested on 1 live connection that was previously failing with both
The sync is still running (processing 2+ years of historical data), but the fix code paths have been exercised and are working correctly. Next Steps
Connector & PR DetailsConnector: Evidence PlanProving Criteria
Disproving Criteria
Cases Attempted
Pre-flight Checks
Detailed Evidence LogRegression Tests (run_id: 23539646210):
Live Connection Test (Job 76410819, attempt 2):
Note: Connection IDs and detailed logs are recorded in the linked private oncall issues. |
Fix Validation — Evidence PlanStatus: 🔄 In Progress — Regression tests running, evidence plan posted Session: Devin Session Pre-flight Checks
Pre-release Image
Regression TestsTriggered comparison regression tests: Workflow Run #23539646210 ⏳ Awaiting results... Evidence PlanProving Criteria
Disproving Criteria
Candidate Cases (ranked)
Approach
Will update this comment as evidence is gathered. |
|
/ai-review
Reviewing PR for connector safety and quality.
|
AI PR Review ReportReview Action: NO ACTION (NOT ELIGIBLE)
📋 PR Details & EligibilityConnector & PR InfoConnector(s): Auto-Approve EligibilityEligible: No Review Action DetailsNO ACTION (NOT ELIGIBLE) — All enforced gates pass but the PR is not eligible for auto-approval. The Behavioral Changes gate detected intentional changes to retry/error-handling behavior that require human sign-off. No PR review submitted.
🔍 Gate Evaluation DetailsGate-by-Gate Analysis
Behavioral Changes — Detailed TriggersFiles:
These changes are the explicit purpose of the bug fix (oncall airbytehq/oncall#11643) and mirror the retry/split policy already used in Code Hygiene — Coverage WarningThe PR author's own review checklist acknowledges: "Consider whether unit tests should be added for the retry logic and since-clamping guard before merge (existing 347 tests pass but don't cover new code paths)". New code paths that could benefit from tests:
📚 Evidence ConsultedEvidence
❓ How to RespondBehavioral Changes — Human Sign-Off RequiredThe Behavioral Changes gate is flagged because this PR adds retry logic and changes error handling. Since this is the intended fix behavior, a human reviewer should confirm the behavioral change is acceptable and sign off. Code Hygiene — Optional ImprovementConsider adding unit tests for the new retry logic and since-clamping guard before merge. This is a WARNING, not a blocking issue. Providing Context or JustificationYou can add explanations that the bot will see on the next review: Option 1: PR Description (recommended) ## AI PR Review Justification
### Behavioral Changes
[Your explanation here]Option 2: PR Comment After adding your response, re-run Note: Justifications provide context for the bot to evaluate. For Anti-Pattern gates like Behavioral Changes, justifications help explain the situation but still require human sign-off. |
…oblematic fields The fields_exceptions attribute was defined on streams like CustomAudiences to exclude fields that cause 'Too much data was requested in batch' errors, but the base class fields() method never actually filtered them out. This fix modifies FBMarketingStream.fields() to exclude any fields listed in fields_exceptions from the API request, which resolves the issue for streams like custom_audiences where the 'rule' field was causing failures. Co-Authored-By: teo@airbyte.io <teo@airbyte.io>
…ream.fields() Co-Authored-By: bot_apk <apk@cognition.ai>
Co-Authored-By: bot_apk <apk@cognition.ai>
devin-ai-integration
bot
changed the title
Cherry-picked
|
…rty setter validation Co-Authored-By: bot_apk <apk@cognition.ai>
3 participants
What
Fixes three independent bugs that cause syncs to fail for
source-facebook-marketing:since > untildate-range error (oncall #11578): Aftervalidate_start_date()clampssinceforward to the 37-month retention boundary, it can land afteruntil, causing Facebook API error#100.Job Failed(oncall #11643): When Facebook returnsasync_status = "Job Failed"during ID-collection polling, the connector immediately raises instead of retrying — unlike the main job polling in_check_statuswhich has retry/split logic.fields_exceptionsnever applied (oncall #2765): TheCustomAudiencesstream definesfields_exceptions = ["rule"]but the base classfields()method never filtered these out, causing "Too much data was requested in batch" errors.Fixes 1 & 2 were previously addressed in separate PRs #74707 and #74817. Fix 3 was in PR #71045 (never merged). This PR combines all three fixes into a single v5.2.3 release.
Resolves https://github.com/airbytehq/oncall/issues/11806:
How
In
_collect_child_ids()(async_job.py):since <= untilbefore building the API params.MAX_ID_COLLECTION_ATTEMPTS = 3). OnFAILED/SKIPPED, log a warning and retry instead of raising immediately. Only raise after all attempts are exhausted.In
fields()(base_streams.py):fields_exceptionsout of the returned field list so streams likeCustomAudiencesactually exclude problematic fields (e.g.rule) from API requests.Review guide
async_job.py— since>until fix and retry logic. Focus on:since.date() > untilguard — verify the type comparison is safe (AirbyteDateTime.date()returnsdate;self._interval.endis alsodate).for...elsepattern — theelseblock (L481–482) fires only when the loop completes withoutbreak, i.e. all attempts failed.statusset by the innerwhile Trueloop. This relies on Python's loop-variable scoping —statusis always assigned before any innerbreak.get_insights()call is now inside the retry loop, meaning a fresh async job is submitted on each retry attempt.base_streams.py—fields_exceptionsfix (cherry-picked from #71045, commitf0c06f54d3e). Thefields()method now excludes any field listed inself.fields_exceptions.test_base_streams.py— new unit tests forfields_exceptionsfiltering (single and multiple exclusions). Note: tests rely on the stream's default state whereconfigured_json_schemaisNone(unset), sofields()falls through toget_json_schema(). Directly settingconfigured_json_schema = Nonevia the property setter triggers CDK validation and fails.metadata.yaml/pyproject.toml— version bump 5.2.2 → 5.2.3.facebook-marketing.md— changelog entry for 5.2.3 (updated to cover all three fixes).Human review checklist
since.date() > untilcomparison is type-safe givenDateInterval.endtypefor...else+ inner-loopstatusscoping behaves correctly for all paths (completed on first try, completed on retry, all attempts exhausted)fields_exceptionsfiltering infields()correctly excludes listed fields and doesn't affect streams with emptyfields_exceptionsPre-release image
A pre-release Docker image has been published for customer testing:
Publish workflow: https://github.com/airbytehq/airbyte/actions/runs/23478459406
User Impact
since must be less than or equal to untilerror.Job Failedstatuses during job splitting will see automatic retries instead of immediate failure.custom_audiencesstream will no longer encounter "Too much data was requested in batch" errors from the Facebook API.Can this PR be safely reverted and rolled back?
Link to Devin session: https://app.devin.ai/sessions/e81cbf21f2634f19a7ec408f096cc45f