feat(source-hubspot): Migrate deals_archived, forms, form_submissions, owners, owners_archived to low-code #58105
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
/format-fix
|
tolik0
changed the title
| @@ -1,654 +0,0 @@ | |||
| { | |||
There was a problem hiding this comment.
Shouldn't we also be removing all the other .json stream schemas that we are deleting?
tolik0
force-pushed
the
tolik0/source-hubspot/migrate-deals-archived
branch
from
9b02942 to
4e9234c
Compare
|
/format-fix
|
|
| path: /marketing/v3/forms | ||
| request_parameters: | ||
| formTypes: | ||
| "['hubspot', 'captured', 'flow']" |
There was a problem hiding this comment.
Testing in Postman, does this result in an error when formatted as a string list? I had to format each formType=hubspot as a separate parameter. For example:
https://api.hubapi.com/marketing/v3/forms?limit=100&formTypes=captured&formTypes=hubspot&formTypes=flow
There was a problem hiding this comment.
This will be formatted as separate parameters: https://api.hubapi.com/marketing/v3/forms?formTypes=hubspot&formTypes=captured&formTypes=flow&
| # TODO: consider migrating FormSubmissions stream to use `/v3` api version | ||
| # then uncomment the last 2 form types. | ||
| # "blog_comment", | ||
| # "all", |
There was a problem hiding this comment.
i see this was a comment from the python code, I wonder if we can add these params back. I can't tell if this comment is still relevant because it does look like we're now already using the /marketing/v3/forms endpoint now and this stream does not appear to interact w/ the FormSubmissions stream or v1 API in any way.
I did some experimentation and it seems like at the very least we can uncomment and include both blog_comment and all in query parameters. And I also tested on our own dataset and we may be able to simply specify formTypes=all to retrieve all forms. When I counted records I got the same amount for formTypes=all vs &formTypes=captured&formTypes=blog_comment&formTypes=hubspot&formTypes=flow
what do you think?
There was a problem hiding this comment.
I think it's safer to stick with a 1:1 migration for now to avoid any unexpected issues. I'll create an issue to further investigate this.
There was a problem hiding this comment.
okay yes and thank you for creating the issue. you are right just for simplicity we can retain the same behavior
There was a problem hiding this comment.
There was a problem hiding this comment.
pending
- validation that query params work in that format mentioned here: feat(source-hubspot): Migrate deals_archived, forms, form_submissions, owners, owners_archived to low-code #58105 (comment)
- updating to rebase w/ the latest hubspot changes
but approving to not block releasing once the above are complete
|
/format-fix
|
|
/format-fix
|
|
Regression tests: |
| paginator: | ||
| $ref: "#/definitions/cursor_paginator" | ||
| record_selector: | ||
| type: RecordSelector |
There was a problem hiding this comment.
wanted to note that when @darynaishchenko was implementing goals, she noticed that we aren't properly normalizing the properties values back to their expected types. She added a custom normalization strategy for this that I think we'll need to incorporate into some of these streams with dynamic properties. See here:
What
Migrate
deals_acrhived,forms,form_submissions,owners,owners_archivedstreams to low-code.How
Review guide
User Impact
Can this PR be safely reverted and rolled back?