fix: update source-twilio with HTTP cache disabled and configurable slice duration#74799
fix: update source-twilio with HTTP cache disabled and configurable slice duration#74799devin-ai-integration[bot] wants to merge 8 commits intomasterfrom
Conversation
Co-Authored-By: gl_anatolii.yatsuk <gl_anatolii.yatsuk@airbyte.io>
🤖 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 |
👋 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-prereleas |
|
/publish-connectors-prerelease |
|
|
Co-Authored-By: gl_anatolii.yatsuk@airbyte.io <gl_anatolii.yatsuk@airbyte.io>
Co-Authored-By: gl_anatolii.yatsuk@airbyte.io <gl_anatolii.yatsuk@airbyte.io>
Co-Authored-By: gl_anatolii.yatsuk@airbyte.io <gl_anatolii.yatsuk@airbyte.io>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔴 Missing dockerImageTag bump when base image is changed
The base image is updated from source-declarative-manifest:7.6.5 to 7.13.0.post3.dev23025648211, but the dockerImageTag remains at 0.17.4. The established pattern in this repo is to bump dockerImageTag whenever the base image changes. For example, the previous base-image update for this same connector (commit 0a33617a489) bumped dockerImageTag from 0.17.3 to 0.17.4 alongside the CDK change. Similarly, source-intercom (commit 13a2c42c562) bumped from 0.13.16-rc.4 to 0.13.16-rc.5 when it switched to its .dev CDK image. Without a version bump, either: (1) the CI/CD system will overwrite the already-published stable 0.17.4 image (which used CDK 7.6.5) with one built on a dev CDK, silently changing behavior for existing users, or (2) the CI may skip the build entirely because the tag is unchanged, so the fix never deploys.
(Refers to line 16)
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This is addressed — dockerImageTag was bumped from 0.17.4 to 0.17.5 in a subsequent commit (cherry-picked from #72494). The latest state of the file has dockerImageTag: 0.17.5.
devin-ai-integration
Bot
changed the title
|
Deploy preview for airbyte-docs ready! ✅ Preview Built with commit 54a8785. |
|
/publish-connectors-prerelease |
|
/publish-connectors-prerelease
|
|
/publish-connectors-prerelease
|
…n with 0.17.5 Co-Authored-By: gl_anatolii.yatsuk <gl_anatolii.yatsuk@airbyte.io>
|
/publish-connectors-prerelease |
|
/publish-connectors-prerelease
|
|
/publish-connectors-prerelease
|
|
/publish-connectors-prerelease
|
3 participants
What
Combines two changes for the source-twilio connector:
Disable HTTP response caching — Updates the base image to a CDK prerelease that has HTTP response caching hardcoded off (
_use_cache = FalseinHttpClient.__init__). This addresses unbounded container memory growth (up to 8 GB) caused by therequests_cacheSQLite backend accumulating cached HTTP responses in OS page cache. Investigation confirmed that disabling caching entirely keeps container memory stable at ~300 MB.Add configurable slice step duration (cherry-picked from #72494) — Adds a new
slice_step_durationconfiguration option with enum valuesP1D(1 Day),P1W(1 Week),P1M(1 Month),P1Y(1 Year). The default is changed fromP1YtoP1M, which reduces the time window per slice and may help avoid timeouts for accounts with large data volumes.Related CDK PR: airbytehq/airbyte-python-cdk#952
How
metadata.yaml— BumpsbaseImagefromsource-declarative-manifest:7.6.5tosource-declarative-manifest:7.13.0.post3.dev23025648211(CDK prerelease with cache disabled). BumpsdockerImageTagfrom0.17.4to0.17.6.manifest.yaml— Replaces hardcodedstep: P1Ywithstep: "{{ config.get('slice_step_duration', 'P1M') }}"in both incremental sync cursor definitions. Adds theslice_step_durationspec property with enum values and labels.unit_tests/test_streams.py— Adjusts test date ranges to work with the new 1-month default slice window.docs/integrations/sources/twilio.md— Adds changelog entries for0.17.5(slice duration feature from feat(source-twilio): Add configurable slice step duration (default: 1 month) #72494) and0.17.6(cache disable + slice duration combined).Updates since last revision
0.17.5→0.17.6: The prerelease publish for0.17.5-preview.e8ab5d2was incorrectly stored in the connector registry withdockerImageTag: "0.17.5"(without the preview suffix), causing version pins in retool to be overwritten with the base version. Bumping to0.17.6avoids this collision with the existing0.17.5prerelease from PR feat(source-twilio): Add configurable slice step duration (default: 1 month) #72494.Review guide
airbyte-integrations/connectors/source-twilio/metadata.yaml— Verify the CDK prerelease version/digest and the version bump to0.17.6airbyte-integrations/connectors/source-twilio/manifest.yaml— Verify the Jinja template{{ config.get('slice_step_duration', 'P1M') }}is correct in both cursor definitions, and the new spec property is well-formedairbyte-integrations/connectors/source-twilio/unit_tests/test_streams.py— Verify test date ranges are consistent with 1-month slicingdocs/integrations/sources/twilio.md— Changelog entries for both0.17.5and0.17.6Key things to verify:
7.13.0.post3.dev23025648211) contains only the cache disable change from CDK PR #952 — no ddtrace, jemalloc, or other experimental changesP1Y→P1Mmeans ~12x more API requests for existing connections that don't set this config option0.17.5(from feat(source-twilio): Add configurable slice step duration (default: 1 month) #72494) — if that PR is merged separately, this could cause a duplicate entryUser Impact
slice_step_durationto tune the trade-off between request count and per-request data volumeCan this PR be safely reverted and rolled back?
Link to Devin Session: https://app.devin.ai/sessions/92b4f9310d1e4f4e9e3b66232e5311d3
Requested by: gl_anatolii.yatsuk