-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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.
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 Freshdesk: migrate to CDK #8682
🎉 Source Freshdesk: migrate to CDK #8682
Conversation
/test connector=source-freshdesk
|
{ | ||
"streams": [ | ||
{ | ||
"name": "agents", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this file? Haven't seen it used anywhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like someone someone by mistake added it here instead of at secrets
"stream": { | ||
"name": "agents", | ||
"json_schema": {}, | ||
"supported_sync_modes": ["incremental"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A very strange catalog. Why is supported_sync_modes
a incremental
, can't it be the same as full_refresh
?
"json_schema": {}, | ||
"supported_sync_modes": ["incremental"], | ||
"source_defined_cursor": true, | ||
"default_cursor_field": null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this stream can be Incremental, why default_cursor_field
is null?
"default_cursor_field": null | ||
}, | ||
"sync_mode": "incremental", | ||
"cursor_field": null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
- config_path: "secrets/config.json" | ||
basic_read: | ||
- config_path: "secrets/config.json" | ||
configured_catalog_path: "integration_tests/configured_catalog.json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need configured catalog if test all streams
configured_catalog_path: "integration_tests/configured_catalog.json" |
empty_streams: ["satisfaction_ratings", "tickets", "time_entries", "conversations"] | ||
incremental: | ||
- config_path: "secrets/config.json" | ||
configured_catalog_path: "integration_tests/configured_catalog.json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
configured_catalog_path: "integration_tests/configured_catalog.json" |
future_state_path: "integration_tests/abnormal_state.json" | ||
full_refresh: | ||
- config_path: "secrets/config.json" | ||
configured_catalog_path: "integration_tests/configured_catalog.json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
configured_catalog_path: "integration_tests/configured_catalog.json" |
@@ -2,28 +2,10 @@ plugins { | |||
id 'airbyte-python' | |||
id 'airbyte-docker' | |||
id 'airbyte-integration-test-java' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
id 'airbyte-integration-test-java' |
@@ -5,8 +5,8 @@ | |||
|
|||
from typing import Any, Iterable, Mapping, Tuple | |||
|
|||
from airbyte_protocol import AirbyteStream | |||
from base_python import BaseClient | |||
from airbyte_cdk.models.airbyte_protocol import AirbyteStream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from airbyte_cdk.models.airbyte_protocol import AirbyteStream | |
from airbyte_cdk.models import AirbyteStream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comments
/test connector=source-freshdesk
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
/publish connector=connectors/source-freshdesk
|
What
Refactoring Source Freshdesk to use the CDK
Closes #7764
How
This task is part of issue #6164. The Source significantly relies on dead CDK code. So import BaseClient and BaseSource from base_python were replaced by import from deprecated package of the CDK. Gradle file, setup.py, Dockerfile, requirements.txt were updated. main_dev.py was renamed to main.py, updated, and added to .dockerignore. Also SAT was enabled to Source.
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/integrations/<source or destination>/<name>.md
including changelog. See changelog example