Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Source Hubspot: Add missing types for the Workflows stream schema #39314

Merged
merged 3 commits into from
Jun 6, 2024
Merged

🐛 Source Hubspot: Add missing types for the Workflows stream schema #39314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
94e1452
fixed
bazarnov Jun 6, 2024
91d8525
updated changelog
bazarnov Jun 6, 2024
38bc291
Merge remote-tracking branch 'origin/master' into baz/hubspot/add-mis…
bazarnov Jun 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion airbyte-integrations/connectors/source-hubspot/acceptance-test-config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ acceptance_tests:
discovery:
tests:
- config_path: secrets/config_oauth.json
# Added missing types to the `Workflows` stream schema
# it's a neccessary change
backward_compatibility_tests_config:
disable_for_version: 1.9.0
disable_for_version: "4.2.2"
basic_read:
tests:
- config_path: secrets/config_oauth.json
Expand Down
2 changes: 1 addition & 1 deletion airbyte-integrations/connectors/source-hubspot/metadata.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ data:
connectorSubtype: api
connectorType: source
definitionId: 36c891d9-4bd9-43ac-bad2-10e12756272c
dockerImageTag: 4.2.2
dockerImageTag: 4.2.3
dockerRepository: airbyte/source-hubspot
documentationUrl: https://docs.airbyte.com/integrations/sources/hubspot
githubIssueLabel: source-hubspot
Expand Down
2 changes: 1 addition & 1 deletion airbyte-integrations/connectors/source-hubspot/pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ requires = [ "poetry-core>=1.0.0",]
build-backend = "poetry.core.masonry.api"

[tool.poetry]
version = "4.2.2"
version = "4.2.3"
name = "source-hubspot"
description = "Source implementation for HubSpot."
authors = [ "Airbyte <contact@airbyte.io>",]
Expand Down
4 changes: 4 additions & 0 deletions airbyte-integrations/connectors/source-hubspot/source_hubspot/schemas/workflows.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@
"properties": {
"sourceApplication": {
"description": "Application details of the workflow update source",
"type": ["null", "object"],
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, what is the default type if this is not provided? Should this have been caught by CATs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a great question.

The data structure remains the same, but the schema was missing these types only.
I wonder why the CAT didn't catch such things,
I assume we don't have the type field presence check yet for the schemas, as a separate testing scenario for the CAT, thus we can easily bypass such things.

Correct me if I'm wrong here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would definitely expect this test to fail in that case. Maybe we haven't configured the validator properly..?

Copy link
Collaborator Author

@bazarnov bazarnov Jun 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The type field is optional for the JsonSchema, in general, so probably there should be an additional check on top of the existing validator to validate its presence.

Copy link
Contributor

@maxi297 maxi297 Jun 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

What is the default value? I could not find this information. If it is the case, we either need to have destinations support this or update our CATs. The latter seems to build more leverage and seems easier to implement too. Can we at least create an issue for that?

Copy link
Collaborator Author

@bazarnov bazarnov Jun 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think, in this case; it depends on the destination used. There is no default value for the source; in this case, it's a matter of how the destination treats the missing types. For instance, BigQuery raises an Exception that there is no type, but it expected any of Null, Object based on the actual data type faced.

"properties": {
"serviceName": {
"description": "Name of the service",
Expand All @@ -175,6 +176,7 @@
},
"updatedByUser": {
"description": "Details of the user who updated the workflow",
"type": ["null", "object"],
"properties": {
"userEmail": {
"description": "Email address of the user",
Expand Down Expand Up @@ -202,6 +204,7 @@
},
"createdByUser": {
"description": "Details of the user who created the workflow",
"type": ["null", "object"],
"properties": {
"userEmail": {
"description": "Email address of the user",
Expand All @@ -215,6 +218,7 @@
},
"sourceApplication": {
"description": "Application details of the workflow source",
"type": ["null", "object"],
"properties": {
"serviceName": {
"description": "Name of the service",
Expand Down
1 change: 1 addition & 0 deletions docs/integrations/sources/hubspot.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ The connector is restricted by normal HubSpot [rate limitations](https://legacyd

| Version | Date | Pull Request | Subject |
|:--------|:-----------| :------------------------------------------------------- |:---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| 4.2.3 | 2024-06-06 | [39314](https://github.com/airbytehq/airbyte/pull/39314) | Added missing schema types for the `Workflows` stream schema |
| 4.2.2 | 2024-06-04 | [38981](https://github.com/airbytehq/airbyte/pull/38981) | [autopull] Upgrade base image to v1.2.1 |
| 4.2.1 | 2024-05-30 | [38024](https://github.com/airbytehq/airbyte/pull/38024) | etry when attempting to get scopes |
| 4.2.0 | 2024-05-24 | [38049](https://github.com/airbytehq/airbyte/pull/38049) | Add resumable full refresh support to `contacts_form_submissions` and `contacts_merged_audit` streams |
Expand Down
Loading