Twilio migration to low code. #40248
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
cristina.mariscal seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
octavia-squidington-iii
added
area/connectors
cmm-airbyte
force-pushed
the
cmm-airbyte/twilioLCAccounts
branch
from
53f6da0 to
eb7382d
Compare
cmm-airbyte
changed the title
cmm-airbyte
force-pushed
the
cmm-airbyte/twilioLCAccounts
branch
from
c848546 to
560e9a4
Compare
…m-airbyte/twilioLCAccounts
…m-airbyte/twilioLCAccounts
…m-airbyte/twilioLCAccounts
|
Important Review skippedAuto reviews are limited to specific labels. Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Testing Manually
I have only tested the streams mentioned below although there are many more. I was using our sandbox account. Can we investigate the ones marked as to be investigated?
accounts
[Accepted change] Pagination has been removed from the account streams which makes request go from https://api.twilio.com/2010-04-01/Accounts.json?PageSize=1000 to https://api.twilio.com/2010-04-01/Accounts.json?
addresses
OK
available_phone_number_countries
OK
available_phone_numbers_local
[to be investigated] Getting Not found. The requested resource was not found on the server when the connector queries https://api.twilio.com/2010-04-01/Accounts/****/AvailablePhoneNumbers/BE/Local.json?PageSize=1000
available_phone_numbers_mobile
[to be investigated] Getting Not found. The requested resource was not found on the server when the connector queries https://api.twilio.com/2010-04-01/Accounts/****/AvailablePhoneNumbers/AR/Mobile.json?PageSize=1000
available_phone_numbers_toll-Free
[to be investigated] This stream was added in the manifest version but does not exist in the current version
incoming_phone_numbers
OK
keys
OK
flows
OK
step
[to be investigated] The latest version produces two records while the manifest one does not have any record. While reading through the logs of the manifest version output, I can see The requested resource /Flows/FW7ad717a690629a6da33bd3c8b9cf7d97/Executions/FN9715cbc94b32ed1090874270ad72efb7/Steps was not found
alerts
[to be investigated] There is a mismatch between the records. I get 4 from the latest version and 6 from the CDK one. Trying to match the first one of the latest version, I can't match 100% with any of the manifest one.
applications
OK
dependent_phone_numbers
[to be investigated] I get The requested resource /2010-04-01/Accounts/AC4cac489c46197c9ebc91c840120a4dee/Addresses/AD0164001bc0f84d9bc29e17378fe47c20/DependentPhoneNumbers.json was not found with the manifest version
…m-airbyte/twilioLCAccounts
…m-airbyte/twilioLCAccounts
| } | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Could you please add steps how to migrate to new version for the users? for example refresh source schema + clear data for affected streams. You can find an example in migration guide for other connectors(tiktok, slack)
| breakingChanges: | ||
| 1.0.0: | ||
| message: This release changes the state format of existing connections. | ||
| upgradeDeadline: "2024-12-01" |
There was a problem hiding this comment.
Please add scopedImpact here
| - $ref: "#/definitions/streams/Users" | ||
| - $ref: "#/definitions/streams/User Conversations" | ||
|
|
||
| spec: |
There was a problem hiding this comment.
we have spec in manifest and in spec.json, should we delete one of them?
| definitions: | ||
| base_requester: | ||
| type: HttpRequester | ||
| url_base: https:// |
There was a problem hiding this comment.
should we add https://api.twilio.com/2010-04-01 here to url_base, instead of defining it in requester.path for each stream?
| type: MinMaxDatetime | ||
| datetime: "{{ now_utc().strftime('%Y-%m-%dT%H:%M:%SZ') }}" | ||
| datetime_format: "%Y-%m-%dT%H:%M:%SZ" | ||
| cursor_granularity: P1D |
There was a problem hiding this comment.
should cursor_granularity be 1s in this case because we have datetime format with seconds precision? doesn't P1D lead to missing records?
| - type: AddedFieldDefinition | ||
| path: | ||
| - date_created | ||
| value: '{% set formatted_datetime = format_datetime(record["date_created"], "%Y-%m-%dT%H:%M:%SZ", "%a, %d %b %Y %H:%M:%S %z") %}{{ formatted_datetime }}' |
There was a problem hiding this comment.
Just a question, what was the reason to change the format of date related fields?
| fields: | ||
| - type: AddedFieldDefinition | ||
| path: | ||
| - duration |
There was a problem hiding this comment.
You can add schema_normalization: Default to record selector and it will autotransform fields like this into type from the schema, so you don't need to transform them here.
|
@girarda if you'd like, community devs team would love to help out with any remaining low-code migrations, including Twilio. I think @pabloescoder will have capacity soon. |
|
@natikgadzhi @girarda Sure! I would certainly love to pick up any low code migrations including Twilio. Please let me know if there are any I could take up. |
|
@pabloescoder wait for @girarda's blessing before getting started |
|
@natikgadzhi Haha I'll wait 😄 |
|
Hello @pabloescoder, sorry for the delays. thank you for offering! your help would be much welcomed here! |
|
No problem! Apologies for the late reply. I'll look into it right away! @girarda |
|
@pabloescoder I've put this on our project board and set the priority. Still important to get to! |
Sorry! This slipped out of my radar, will pick it up soon |
|
@pabloescoder taking after the quickbooks tasks? |
|
@natikgadzhi Yes! Let's wrap up with quickbooks by this weekend or early next week, then I'll start with this. It's been delayed for a long time. |
|
@pabloescoder this is still valid to work on, but you probably can use this as reference and start from scratch. Also, we should go to manifest-only and just make a schema-close-to-compatible version from scratch. |
What & how
Migrating Twilio connector to low code.
Review guide
User Impact
This must be transparent for users.
Can this PR be safely reverted and rolled back?