✨ Source Insightly : Migrate Python CDK to Low-code CDK #31162

aazam-gh · 2023-10-08T11:55:08Z

What

How

Use Low Code/ No Code CDK

🚨 User Impact 🚨

Are there any breaking changes? What is the end result perceived by the user?

For connector PRs, use this section to explain which type of semantic versioning bump occurs as a result of the changes. Refer to our Semantic Versioning for Connectors guidelines for more information. Breaking changes to connectors must be documented by an Airbyte engineer (PR author, or reviewer for community PRs) by using the Breaking Change Release Playbook.

If there are breaking changes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.

Pre-merge Actions

Expand the relevant checklist and delete the others.

Updating a connector

Community member or Airbyter

Grant edit access to maintainers (instructions)
Unit & integration tests added

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
If new credentials are required for use in CI, add them to GSM. Instructions.

vercel · 2023-10-08T11:55:13Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
airbyte-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 14, 2023 3:53pm

github-actions · 2023-10-08T11:55: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

I made a quick review asking to update a few files this will help us during the code review. Sorry the long delay to get into your contribution. Probably this week or next one the team will start the process. Are you able to run the tests in your local setup? It can speed up a lot the process if you share the integration tests results.
Don't forget to update the doc changelog.

marcosmarxm · 2023-10-23T14:23:57Z

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

@@ -27,4 +27,4 @@ acceptance_tests:
      - config_path: "secrets/config.json"
        configured_catalog_path: "integration_tests/configured_catalog.json"
        future_state:
-          future_state_path: integration_tests/abnormal_state.json
+          future_state_path: integration_tests/abnormal_state.json


marcosmarxm · 2023-10-23T14:24:13Z

airbyte-integrations/connectors/source-insightly/icon.svg

@@ -1 +0,0 @@
-<svg xmlns="http://www.w3.org/2000/svg" width="250" height="250" fill="none"><path fill="#FF5621" d="m204.648 193.714.047.177c-6.087-.791-9.532-7.065-9.532-24.255 0-10.023 1.265-19.599 2.501-28.957 1.182-8.94 2.336-17.681 2.336-26.421 0-16.75-4.837-30.098-23.581-30.098-16.63 0-34.771 18.74-41.726 33.505.894-5.736 2.376-10.645 3.649-14.863 1.31-4.341 2.399-7.95 2.399-10.976a8.898 8.898 0 0 0-2.422-7.594 8.987 8.987 0 0 0-7.556-2.63c-9.676 0-31.447 9.09-34.168 22.438-1.813 6.526-2.72 11.641-3.024 58.213-.139 8.025-.865 16.026-1.602 23.488a10.138 10.138 0 0 1-4.033 6.422 10.226 10.226 0 0 1-7.377 1.852c-6.96 0-10.292-9.095-10.292-38.383 0-18.698 2.84-34.268 4.892-45.523 1.162-6.373 2.072-11.362 2.072-14.755a10.72 10.72 0 0 0-.65-4.326 10.759 10.759 0 0 0-2.324-3.712 10.819 10.819 0 0 0-3.615-2.485 10.858 10.858 0 0 0-4.31-.849 41.184 41.184 0 0 0-16.575 6.62 40.966 40.966 0 0 0-12.186 12.996C33.028 112.699 30 151.937 30 174.684c0 26.156 9.688 47.197 38.147 47.197a35.364 35.364 0 0 0 22.453-7.622c.7 4.805 2.568 7.622 6.602 7.622 28.419 0 33.864-12.208 33.864-19.88 0-45.432 10.582-80.644 22.675-80.644 5.141 0 6.954 5.114 6.954 12.497 0 6.187-1.063 13.975-2.16 22.007-1.168 8.552-2.374 17.381-2.374 24.849 0 25.555 13.303 41.171 37.491 41.171a35.943 35.943 0 0 0 24.017-8.584c3.129-4.779 2.579-8.77 1.636-13.937-.867-4.751-9.775-5.739-14.657-5.646ZM72.733 26.88c-10.549 0-28.34 8.22-28.34 25.81 0 19.856 8.75 23.188 17.489 23.189A31.792 31.792 0 0 0 83.28 64.933a31.509 31.509 0 0 0 7.541-22.744c0-11.058-8.435-15.31-18.089-15.31Z"/></svg>


Why you removed this file?

Must have been an accident. I have added it back now

marcosmarxm · 2023-10-23T14:24:24Z

airbyte-integrations/connectors/source-insightly/build.gradle

+plugins {
+    id 'airbyte-python'
+    id 'airbyte-docker'
+}
+
+airbytePython {
+    moduleDirectory 'source_insightly'
+}


Remove this file, not used anymore

marcosmarxm · 2023-10-23T14:24:34Z

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

    "pytest-mock~=3.6.1",
+    "connector-acceptance-test",


Remove this import not used anymore.

marcosmarxm · 2023-10-23T23:41:28Z

@aazam-gh can you share your output for integration tests? I tried in my side and the connector is not working.

aazam-gh · 2023-10-24T04:23:55Z

@marcosmarxm thanks for the review but I am having trouble running the integration tests locally.
The acceptance test command isn't working. Should i try with the cli?

aazam-gh · 2023-10-24T05:50:44Z

airbyte-integrations/connectors/source-insightly/source_insightly/manifest.yaml

+      page_token_option:
+        type: RequestOption
+        inject_into: "request_parameter"
+        field_name: "skip"


I also want to highlight this part regarding the pagination. Insightly has the skip parameter which skips a certain amount of records. For example if the first request gives us 500 records then for the next request we will add skip=500 to the parameter, however for the next request after it we will have to add skip=100, i.e the skip gets incremented so that all records are parsed. How do we apply it here?

Did you try to use the Offset paginator?

marcosmarxm · 2023-10-24T19:02:56Z

The acceptance test command isn't working. Should i try with the cli?

You can use the sh acceptance-test.sh file inside the connector folder or the airbyte-ci.

aazam-gh · 2023-10-29T19:00:03Z

You can use the sh acceptance-test.sh file inside the connector folder or the airbyte-ci.

I keep getting sh: 0: Can't open acceptance-test.sh when I try to run the command

addressed

migrated insightly

12c80ac

octavia-squidington-iii added area/connectors Connector related issues community connectors/source/insightly labels Oct 8, 2023

sajarin added the hackathon-2023-10 label Oct 9, 2023

sajarin self-assigned this Oct 9, 2023

marcosmarxm added the low-code-migration This connector has been migrated to the low-code CDK label Oct 9, 2023

marcosmarxm unassigned sajarin Oct 20, 2023

marcosmarxm previously requested changes Oct 23, 2023

View reviewed changes

Merge branch 'master' into insight

32bfe42

vercel bot deployed to Preview October 23, 2023 23:35 View deployment

made quick fixes

d9c22b3

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

vercel bot deployed to Preview October 23, 2023 23:47 View deployment

aazam-gh commented Oct 24, 2023

View reviewed changes

Merge branch 'master' into insight

0227142

vercel bot deployed to Preview October 30, 2023 15:41 View deployment

Merge branch 'master' into insight

37052a0

vercel bot deployed to Preview November 9, 2023 17:22 View deployment

fix: update manifest, configured_catalog and get tests passing

59e54d9

sajarin approved these changes Nov 13, 2023

View reviewed changes

sajarin mentioned this pull request Nov 13, 2023

Run ci checks for ✨ Source Insightly : Migrate Python CDK to Low-code CDK #32463

Closed

sajarin and others added 2 commits November 13, 2023 17:44

Automated Commit - Formatting Changes

5a8c724

fix: update url links with proper search endpoints

636a27d

sajarin added 3 commits November 13, 2023 13:04

fix: correct organisations stream definition

c9f0d92

fix: add 429 error handling

f153d6e

Merge branch 'master' into insight

0ecc6f2

vercel bot deployed to Preview November 13, 2023 20:36 View deployment

sajarin added 3 commits November 14, 2023 10:23

fix: update check streams

1c3dbfd

fix: add ignored column for users stream

79b98b4

Merge branch 'master' into insight

d6514ea

vercel bot deployed to Preview November 14, 2023 15:53 View deployment

sajarin merged commit a94cfdf into airbytehq:master Nov 14, 2023
35 of 41 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Source Insightly : Migrate Python CDK to Low-code CDK #31162

✨ Source Insightly : Migrate Python CDK to Low-code CDK #31162

aazam-gh commented Oct 8, 2023

vercel bot commented Oct 8, 2023 •

edited

github-actions bot commented Oct 8, 2023

marcosmarxm left a comment

marcosmarxm Oct 23, 2023

marcosmarxm Oct 23, 2023

aazam-gh Oct 24, 2023

marcosmarxm Oct 23, 2023

marcosmarxm Oct 23, 2023

marcosmarxm commented Oct 23, 2023

aazam-gh commented Oct 24, 2023

aazam-gh Oct 24, 2023

marcosmarxm Oct 24, 2023

marcosmarxm commented Oct 24, 2023

aazam-gh commented Oct 29, 2023

		@@ -1 +0,0 @@
		<svg xmlns="http://www.w3.org/2000/svg" width="250" height="250" fill="none"><path fill="#FF5621" d="m204.648 193.714.047.177c-6.087-.791-9.532-7.065-9.532-24.255 0-10.023 1.265-19.599 2.501-28.957 1.182-8.94 2.336-17.681 2.336-26.421 0-16.75-4.837-30.098-23.581-30.098-16.63 0-34.771 18.74-41.726 33.505.894-5.736 2.376-10.645 3.649-14.863 1.31-4.341 2.399-7.95 2.399-10.976a8.898 8.898 0 0 0-2.422-7.594 8.987 8.987 0 0 0-7.556-2.63c-9.676 0-31.447 9.09-34.168 22.438-1.813 6.526-2.72 11.641-3.024 58.213-.139 8.025-.865 16.026-1.602 23.488a10.138 10.138 0 0 1-4.033 6.422 10.226 10.226 0 0 1-7.377 1.852c-6.96 0-10.292-9.095-10.292-38.383 0-18.698 2.84-34.268 4.892-45.523 1.162-6.373 2.072-11.362 2.072-14.755a10.72 10.72 0 0 0-.65-4.326 10.759 10.759 0 0 0-2.324-3.712 10.819 10.819 0 0 0-3.615-2.485 10.858 10.858 0 0 0-4.31-.849 41.184 41.184 0 0 0-16.575 6.62 40.966 40.966 0 0 0-12.186 12.996C33.028 112.699 30 151.937 30 174.684c0 26.156 9.688 47.197 38.147 47.197a35.364 35.364 0 0 0 22.453-7.622c.7 4.805 2.568 7.622 6.602 7.622 28.419 0 33.864-12.208 33.864-19.88 0-45.432 10.582-80.644 22.675-80.644 5.141 0 6.954 5.114 6.954 12.497 0 6.187-1.063 13.975-2.16 22.007-1.168 8.552-2.374 17.381-2.374 24.849 0 25.555 13.303 41.171 37.491 41.171a35.943 35.943 0 0 0 24.017-8.584c3.129-4.779 2.579-8.77 1.636-13.937-.867-4.751-9.775-5.739-14.657-5.646ZM72.733 26.88c-10.549 0-28.34 8.22-28.34 25.81 0 19.856 8.75 23.188 17.489 23.189A31.792 31.792 0 0 0 83.28 64.933a31.509 31.509 0 0 0 7.541-22.744c0-11.058-8.435-15.31-18.089-15.31Z"/></svg>

✨ Source Insightly : Migrate Python CDK to Low-code CDK #31162

✨ Source Insightly : Migrate Python CDK to Low-code CDK #31162

Conversation

aazam-gh commented Oct 8, 2023

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Actions

Community member or Airbyter

Airbyter

vercel bot commented Oct 8, 2023 • edited

github-actions bot commented Oct 8, 2023

Before Merging a Connector Pull Request

marcosmarxm left a comment

Choose a reason for hiding this comment

marcosmarxm Oct 23, 2023

Choose a reason for hiding this comment

marcosmarxm Oct 23, 2023

Choose a reason for hiding this comment

aazam-gh Oct 24, 2023

Choose a reason for hiding this comment

marcosmarxm Oct 23, 2023

Choose a reason for hiding this comment

marcosmarxm Oct 23, 2023

Choose a reason for hiding this comment

marcosmarxm commented Oct 23, 2023

aazam-gh commented Oct 24, 2023

aazam-gh Oct 24, 2023

Choose a reason for hiding this comment

marcosmarxm Oct 24, 2023

Choose a reason for hiding this comment

marcosmarxm commented Oct 24, 2023

aazam-gh commented Oct 29, 2023

vercel bot commented Oct 8, 2023 •

edited