✨ Source Pipedrive : Migrate Python CDK to Low-code CDK #29293

avirajsingh7 · 2023-08-09T17:30:09Z

What

Migrating Source pipedrive to Low-Code CDK
Closes #29135

How

Developed using (Configuration Based Source) low-code CDK

🚨 User Impact 🚨

Are there any breaking changes? What is the end result perceived by the user?
Yes, there are breaking changes due to Changes in spec.yaml file
At many places previous data type were wrong and added some new properties in schema as per response from API

Test

Acceptance Test

Migrate Connector : Source Pipedrive

github-actions · 2023-08-09T17:30:35Z

Before Merging a Connector Pull Request

Wow! What a great pull request you have here! 🎉

To merge this PR, ensure the following has been done/considered for each connector added or updated:

PR name follows PR naming conventions
Breaking changes are considered. If a Breaking Change is being introduced, ensure an Airbyte engineer has created a Breaking Change Plan.
Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
You've updated the connector's metadata.yaml file any other relevant changes, including a breakingChanges entry for major version bumps. See metadata.yaml docs
Secrets in the connector's spec are annotated with airbyte_secret
All documentation files are up to date. (README.md, bootstrap.md, docs.md, etc...)
Changelog updated in docs/integrations/<source or destination>/<name>.md with an entry for the new version. See changelog example
Migration guide updated in docs/integrations/<source or destination>/<name>-migrations.md with an entry for the new version, if the version is a breaking change. See migration guide example
If set, you've ensured the icon is present in the platform-internal repo. (Docs)

If the checklist is complete, but the CI check is failing,

Check for hidden checklists in your PR description
Toggle the github label checklist-action-run on/off to re-run the checklist CI.

marcosmarxm

Some comments.

marcosmarxm · 2023-08-11T16:25:59Z

airbyte-integrations/connectors/source-pipedrive/source_pipedrive/schemas/activity_types.json

-  "$schema": "http://json-schema.org/schema#",
-  "type": "object",


Any reason why you remove this?

@avirajsingh7

@marcosmarxm added

marcosmarxm · 2023-08-11T16:29:11Z

airbyte-integrations/connectors/source-pipedrive/source_pipedrive/manifest.yaml

+      paginator:
+        type: NoPagination


Why this is not using paginator?

@marcosmarxm activity_types, currencies and permission_sets
does not have any clear pagination technique and nor they take any query parameter to get next page
So we have two ideas implementing NoPagination or removing this streams
whatever you suggest...

marcosmarxm · 2023-08-11T16:33:55Z

airbyte-integrations/connectors/source-pipedrive/source_pipedrive/manifest.yaml

+    type: RecordSelector
+    extractor:
+      type: DpathExtractor
+      field_path: ["data", "*", "data","*"]


Why are you using this path?

{
"success": true,
"data": [
{
"item": "user",
"id": 18047894,
"data": [
{
"id": 18047894,
"name": "Aviraj G",
"default_currency": "INR",
"timezone_name": "Asia/Kolkata",
"timezone_offset": "+05:30",
"locale": "en_US",
"email": "",
"phone": "",
"created": "2023-08-07 05:15:57",
"modified": "2023-08-11 20:44:25",
"lang": 1,
"active_flag": true,
"is_admin": 1,
"last_login": "2023-08-11 20:44:25",
"signup_flow_variation": "",
"role_id": 1,
"has_created_company": false,
"icon_url": null,
"is_you": true
}
]
},
{
"item": "user",
"id": 18105578,
"data": [
{
"id": 18105578,
"name": "Av",
"default_currency": "INR",
"timezone_name": "Asia/Kolkata",
"timezone_offset": "+05:30",
"locale": "en_US",
"email": "***",
"phone": null,
"created": "2023-08-12 09:23:56",
"modified": "2023-08-12 09:24:55",
"lang": 1,
"active_flag": true,
"is_admin": 0,
"last_login": "2023-08-12 09:24:55",
"signup_flow_variation": "",
"role_id": 1,
"has_created_company": false,
"icon_url": null,
"is_you": false
}
]
}
],
"additional_data": {
"since_timestamp": "2017-01-25 00:00:00",
"last_timestamp_on_page": "2023-08-12 09:24:55",
"pagination": {
"start": 0,
"limit": 2,
"more_items_in_collection": false
}
}
}

Api response for user that's why used this way

else read is throwing error

marcosmarxm · 2023-08-11T16:35:39Z

airbyte-integrations/connectors/source-pipedrive/source_pipedrive/manifest.yaml

+    paginator:
+      type: "DefaultPaginator"
+      pagination_strategy:
+        type: "PageIncrement"
+        page_size: 50
+      page_token_option:
+        type: "RequestOption"
+        inject_into: "request_parameter"
+        field_name: "page"


Not sure if the simple DefaultPagination will work with Pipedrive pagination response.

@marcosmarxm ya true thanks for mentioning,
we can implement cursorpagination
only in lead stream we cant implement cursorpagination
can you suggest anything else for lead stream

paginator: type: "DefaultPaginator" pagination_strategy: type: "CursorPagination" cursor_value: "{{ response['additional_data']['pagination']['next_start'] }}" page_token_option: type: "RequestOption" inject_into: "request_parameter" field_name: "start"

airbyte-integrations/connectors/source-pipedrive/source_pipedrive/source.py

marcosmarxm

Some comments.

marcosmarxm · 2023-08-31T18:05:24Z

airbyte-integrations/connectors/source-pipedrive/acceptance-test-config.yml

-    tests:
-      - config_path: "secrets/config.json"
-        expect_records:
-          path: "integration_tests/expected_records.jsonl"
-        ignored_fields:
-          users:
-            - name: modified
-              bypass_reason: "constantly increasing date-time field"


Please keep the same.

expected_records.jsonl this file was old that's why I removed it and it will lead to failures

@avirajsingh7 that files is used to compare records when running integration tests.

So for that I need to fix expected_records.jsonl file also according to latest schemas or I can keep old one?

marcosmarxm · 2023-08-31T18:05:38Z

airbyte-integrations/connectors/source-pipedrive/acceptance-test-config.yml

      - config_path: "integration_tests/invalid_config.json"
        status: "failed"
  discovery:
    tests:
      - config_path: "secrets/config.json"
        backward_compatibility_tests_config:
-          disable_for_version: 0.1.19
+          disable_for_version: 1.0.0


There are not reason to change to breaking change.

Due to schema changes, I have done

It shouldn't be the case. For migrating a Beta/GA the schema should be broke right now.

Should I remove disable for version

Yes, please remove those lines.

marcosmarxm · 2023-08-31T18:05:47Z

airbyte-integrations/connectors/source-pipedrive/acceptance-test-config.yml

+        backward_compatibility_tests_config:
+          disable_for_version: 1.0.0
  connection:


Please keep the same config as before.

marcosmarxm · 2023-09-04T21:39:05Z

Did you check my latest comments @avirajsingh7 ?

marcosmarxm · 2023-09-05T13:12:49Z

airbyte-integrations/connectors/source-pipedrive/source_pipedrive/schemas/activity_types.json

-  "$schema": "http://json-schema.org/schema#",
-  "type": "object",


@avirajsingh7

avirajsingh7 · 2023-09-05T13:16:46Z

@marcosmarxm added

marcosmarxm · 2023-09-06T12:53:58Z

Thanks @avirajsingh7 the plans is to review your contribution next week. Meanwhile can you solve the conflicts your PR has?

airbyte-integrations/connectors/source-pipedrive/source_pipedrive/manifest.yaml

airbyte-integrations/connectors/source-pipedrive/source_pipedrive/spec.json

archangelic · 2023-09-13T23:04:37Z

airbyte-integrations/connectors/source-pipedrive/Dockerfile


 ENV AIRBYTE_ENTRYPOINT "python /airbyte/integration_code/main.py"
 ENTRYPOINT ["python", "/airbyte/integration_code/main.py"]

-LABEL io.airbyte.version=1.0.0
+LABEL io.airbyte.version=2.0.0


Suggested change

LABEL io.airbyte.version=2.0.0

LABEL io.airbyte.version=1.1.0

archangelic · 2023-09-13T23:05:48Z

airbyte-integrations/connectors/source-pipedrive/acceptance-test-config.yml

      - config_path: "integration_tests/invalid_config.json"
        status: "failed"
  discovery:
    tests:
      - config_path: "secrets/config.json"
        backward_compatibility_tests_config:
-          disable_for_version: 0.1.19
+          disable_for_version: 1.0.0


Yes, please remove those lines.

archangelic · 2023-09-13T23:08:43Z

airbyte-integrations/connectors/source-pipedrive/acceptance-test-config.yml

+   tests:
+     - config_path: "secrets/config.json"
+       configured_catalog_path: "integration_tests/configured_catalog.json"
+       future_state:
+         future_state_path: "integration_tests/abnormal_state.json"


Suggested change

tests:

- config_path: "secrets/config.json"

configured_catalog_path: "integration_tests/configured_catalog.json"

future_state:

future_state_path: "integration_tests/abnormal_state.json"

tests:

- config_path: "secrets/config.json"

configured_catalog_path: "integration_tests/configured_catalog.json"

future_state:

future_state_path: "integration_tests/abnormal_state.json"

archangelic · 2023-09-13T23:09:10Z

airbyte-integrations/connectors/source-pipedrive/metadata.yaml

  connectorSubtype: api
  connectorType: source
  definitionId: d8286229-c680-4063-8c59-23b9b391c700
-  dockerImageTag: 1.0.0
+  dockerImageTag: 2.0.0


Suggested change

dockerImageTag: 2.0.0

dockerImageTag: 1.1.0

archangelic · 2023-09-13T23:09:50Z

airbyte-integrations/connectors/source-pipedrive/setup.py

@@ -7,11 +7,13 @@

 MAIN_REQUIREMENTS = [
    "airbyte-cdk~=0.1",


Suggested change

"airbyte-cdk~=0.1",

"airbyte-cdk",

archangelic · 2023-09-13T23:10:07Z

airbyte-integrations/connectors/source-pipedrive/setup.py

+TEST_REQUIREMENTS = [
+    "pytest~=6.2",
+    "pytest-mock~=3.6.1",
+    "connector-acceptance-test",
+]


Suggested change

TEST_REQUIREMENTS = [

"pytest~=6.2",

"pytest-mock~=3.6.1",

"connector-acceptance-test",

]

TEST_REQUIREMENTS = [

"pytest~=6.2",

"pytest-mock~=3.6.1",

]

archangelic · 2023-09-13T23:11:10Z

airbyte-integrations/connectors/source-pipedrive/source_pipedrive/manifest.yaml

+    url_base: "https://api.pipedrive.com/v1"
+    http_method: "GET"
+    request_parameters:
+      api_token: "{{config['api_token']}}"


Suggested change

api_token: "{{config['api_token']}}"

api_token: "{{ config['authorization']['api_token'] }}"

This is to maintain compatibility with current config.

archangelic · 2023-09-13T23:11:43Z

airbyte-integrations/connectors/source-pipedrive/source_pipedrive/manifest.yaml

+    cursor_field: "update_time"
+    datetime_format: "%Y-%m-%d %H:%M:%S"
+    start_datetime:
+      datetime: "{{ config['replication_start_date'] }}"


Suggested change

datetime: "{{ config['replication_start_date'] }}"

datetime: "{{ format_datetime(config['replication_start_date'], '%Y-%m-%d %H:%M:%S') }}"

archangelic · 2023-09-13T23:13:45Z

airbyte-integrations/connectors/source-pipedrive/source_pipedrive/manifest.yaml

+spec:
+  type: Spec
+  documentation_url: https://docs.airbyte.com/integrations/sources/pipedrive
+  connection_specification:
+    title: Pipedrive Spec
+    type: object
+    required:
+      - api_token
+      - replication_start_date
+    additionalProperties: true
+    properties:
+      api_token:
+        title: API Token
+        type: string
+        description: The Pipedrive API Token.
+        airbyte_secret: true
+      replication_start_date:
+        title: Start Date
+        description: UTC date and time in the format 2017-01-25T00:00:00Z. Any data
+          before this date will not be replicated. When specified and not None, then
+          stream will behave as incremental
+        examples:
+        - '2017-01-25 00:00:00Z'
+        type: string


Suggested change

spec:

type: Spec

documentation_url: https://docs.airbyte.com/integrations/sources/pipedrive

connection_specification:

title: Pipedrive Spec

type: object

required:

- api_token

- replication_start_date

additionalProperties: true

properties:

api_token:

title: API Token

type: string

description: The Pipedrive API Token.

airbyte_secret: true

replication_start_date:

title: Start Date

description: UTC date and time in the format 2017-01-25T00:00:00Z. Any data

before this date will not be replicated. When specified and not None, then

stream will behave as incremental

examples:

- '2017-01-25 00:00:00Z'

type: string

spec:

type: Spec

documentation_url: https://docs.airbyte.com/integrations/sources/pipedrive

connection_specification:

title: Pipedrive Spec

type: object

required:

- replication_start_date

additionalProperties: true

properties:

authorization:

type: object

title: API Key Authentication

required:

- api_token

properties:

api_token:

title: API Token

type: string

description: The Pipedrive API Token.

airbyte_secret: true

replication_start_date:

title: Start Date

description: UTC date and time in the format 2017-01-25T00:00:00Z. Any data

before this date will not be replicated. When specified and not None, then

stream will behave as incremental

examples:

- '2017-01-25 00:00:00Z'

type: string

marcosmarxm · 2023-09-14T18:14:12Z

Hello, and thank you so much for contributing to the project during the Hackathon. Due to the high complexity of the code migration, we still have a few contributions open. I'm sending this message to update you that this contribution will be added to the final points we're going to announce tomorrow. With that said, we ask for your help to continue the process and finish the review.

archangelic · 2023-09-18T20:46:51Z

I am currently working on this in #30553 and hope to get it merged today if possible!

archangelic · 2023-09-21T17:35:49Z

Congrats! This was merged in #30553!

migrated to low code

43715e1

octavia-squidington-iii added area/connectors Connector related issues community connectors/source/pipedrive labels Aug 9, 2023

metadata and docs modified

5cf197f

octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Aug 9, 2023

marcosmarxm added team/tse Technical Support Engineers Hackathon External low-code/ui connector builder hackathons hackathon-2023-08 labels Aug 9, 2023

marcosmarxm requested changes Aug 11, 2023

View reviewed changes

marcosmarxm reviewed Aug 31, 2023

View reviewed changes

airbyte-integrations/connectors/source-pipedrive/source_pipedrive/source.py Show resolved Hide resolved

marcosmarxm requested changes Aug 31, 2023

View reviewed changes

marcosmarxm requested changes Sep 5, 2023

View reviewed changes

suggested changed

9be5217

avirajsingh7 and others added 2 commits September 6, 2023 21:02

Merge branch 'master' into Hackathon/pipedrive

15e6dcd

Merge branch 'master' into Hackathon/pipedrive

880f28d

marcosmarxm requested changes Sep 13, 2023

View reviewed changes

airbyte-integrations/connectors/source-pipedrive/source_pipedrive/manifest.yaml Show resolved Hide resolved

airbyte-integrations/connectors/source-pipedrive/source_pipedrive/spec.json Show resolved Hide resolved

archangelic suggested changes Sep 13, 2023

View reviewed changes

archangelic mentioned this pull request Sep 18, 2023

[Non-Forked] Source Pipedrive : Migrate Python CDK to Low-code CDK #30553

Merged

archangelic closed this Sep 21, 2023

		"$schema": "http://json-schema.org/schema#",
		"type": "object",

	LABEL io.airbyte.version=2.0.0
	LABEL io.airbyte.version=1.1.0

	api_token: "{{config['api_token']}}"
	api_token: "{{ config['authorization']['api_token'] }}"

	datetime: "{{ config['replication_start_date'] }}"
	datetime: "{{ format_datetime(config['replication_start_date'], '%Y-%m-%d %H:%M:%S') }}"

✨ Source Pipedrive : Migrate Python CDK to Low-code CDK #29293

✨ Source Pipedrive : Migrate Python CDK to Low-code CDK #29293

Conversation

avirajsingh7 commented Aug 9, 2023

What

How

Recommended reading order

🚨 User Impact 🚨

Test

github-actions bot commented Aug 9, 2023

Before Merging a Connector Pull Request

marcosmarxm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avirajsingh7 Aug 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avirajsingh7 Aug 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcosmarxm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcosmarxm commented Sep 4, 2023

Choose a reason for hiding this comment

avirajsingh7 commented Sep 5, 2023

marcosmarxm commented Sep 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcosmarxm commented Sep 14, 2023

archangelic commented Sep 18, 2023

archangelic commented Sep 21, 2023

avirajsingh7 Aug 12, 2023 •

edited

Loading

avirajsingh7 Aug 12, 2023 •

edited

Loading