🚨✨ Source Convex - switch the JSON format requested from Convex by nipunn1313 · Pull Request #30853 · airbytehq/airbyte

Nipunn Koorapati (nipunn1313) · 2023-09-28T21:45:08Z

What

We're updating the json format which is requested from the backend from convex_json to json. The new format is more readable/usable (rather than prioritizing roundtrippable).

How

Updating the source connector to request json format (rather than convex_json format). The format is a tad simpler and cleaner in how it handles a few cases.

Added a fmt parameter to the configuration structure, defaulting it to json format. Intentionally left it out of the yml.

We updated the airbyte-client version number which we send up as a header so the server can detect the new version.

🚨 User Impact 🚨

The users will see a simpler JSON format which encodes a few values a bit differently. Most exports will not see a difference.
Examples:
Bigints go from {"$integer": "55"} to "55"
Infinite floats go from {"$float": "Inf"} to {"Inf"}
... a few more cases

Pre-merge Actions

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-09-28T21:45:12Z

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 28, 2023 2:46pm

github-actions · 2023-09-28T21:45:27Z

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.

Add fmt param to the source-convex connector for future extensibility. May add it to the configuration options in the future, but for now, just have it there for testability, defaulting to json format.

Nipunn Koorapati (nipunn1313) · 2023-10-03T17:58:53Z

Hi. I am blocked on this - in order to make sure all the tests and formatting pass CI.

Marcos Marx (marcosmarxm)

Thanks for the contribution Nipunn Koorapati (@nipunn1313) and sorry the missing update here.
This introduces a breaking change to existing connectors? If someone update the message read by Airbyte will be different?
Convex is deprecating the convex_json or you're introducing only another supported type?
If the first, what is the ETA to remove it?

Can you remove the parse_config function? The UI already apply this validation and it's duplicating features here.
If you have only a format type you as a constant in the code, if users can select the format type, which I don't believe it brings any performance to them add the option in the spec.json/yaml file.

Nipunn Koorapati (nipunn1313) · 2023-10-11T17:49:09Z

This introduces a breaking change to existing connectors?

Yes... sort of. It (subtly/minorly) simplifies the JSON format for subsequent documents.

Fortunately, we have stats on our side (from the server API endpoints) on who is using the airbyte connector with convex and it is only one customer - whom we are directly in contact with. They are ok with this change.

If someone update the message read by Airbyte will be different? Convex is deprecating the convex_json or you're introducing only another supported type?

There is only one documented type json. https://docs.convex.dev/database/types
The backend will continue to support the old convex_json format indefinitely, but it is not documented. This PR is switching to the documented format. Again, we only have one customer using airbyte with whom we are in close contact with.

Can you remove the parse_config function? The UI already apply this validation and it's duplicating features here. If you have only a format type you as a constant in the code, if users can select the format type, which I don't believe it brings any performance to them add the option in the spec.json/yaml file.

Yes - I can do that.

Nipunn Koorapati (nipunn1313) · 2023-10-16T20:49:31Z

Hello. I've removed parse_config and wanted to check in here.

Nipunn Koorapati (nipunn1313) · 2023-10-28T00:17:59Z

Hello. I wanted to check in here again.

Marcos Marx (marcosmarxm) · 2023-10-30T13:15:42Z

Nipunn Koorapati (@nipunn1313) at the moment the team is dedicated reviewing Hackathon contribution. We're going to return to the community backlog in the future.

Nipunn Koorapati (nipunn1313) · 2023-11-13T19:29:46Z

Hello. Wanted to check in here. I understand your team has been busy recently.

What expectations should I have around the timing of this review? (IE - when should I check in again?).

Natalie Kwong (nataliekwong) · 2023-11-17T06:33:03Z

Hey Nipunn Koorapati (@nipunn1313) , I apologize for the delay here. This is certainly not the experience we want you to have when contributing. Your PR, among others, sparked conversations internally about how we can improve the speed of PR review for those contributing their own company's connectors.

Kat Wilson (@katmarkham) is our steward of API sources here, and is working with her team to set a process to fast-track these in the future. For this particular PR, the team is hoping to start review by the end of next week as other projects are getting shuffled.

Serhii Lazebnyi (lazebnyi)

Approved with condition. The braking changes process must be followed.

Serhii Lazebnyi (lazebnyi) · 2023-11-24T17:06:45Z

airbyte-integrations/connectors/source-convex/source_convex/source.py

        deployment_url = config["deployment_url"]
        access_key = config["access_key"]
-        url = f"{deployment_url}/api/json_schemas?deltaSchema=true&format=convex_json"
+        url = f"{deployment_url}/api/json_schemas?deltaSchema=true&format=json"


Look like braking changes as schema will be updated.

Serhii Lazebnyi (lazebnyi)

Approved with condition. The braking changes process must be followed.

Natalie Kwong (nataliekwong) · 2023-11-25T12:17:06Z

Serhii Lazebnyi (@lazebnyi) In the comments, it's noted there is only 1 user of the connector and they are already aware of this change as the contributor is already in close contact with the user. While it is a schema change, I recommend not requiring the breaking change process given we can simply inform the user of the connector of the change. Kat Wilson (@katmarkham)

Nipunn Koorapati (nipunn1313) · 2023-11-25T16:18:12Z

While it is a schema change, I recommend not requiring the breaking change process given we can simply inform the user of the connector of the change.

Yep! That sounds good to us.

Removed parse_config and GL approved

Marcos Marx (marcosmarxm)

Releasing without breaking change as Natalie Kwong (@nataliekwong) commented.

Marcos Marx (marcosmarxm) · 2023-11-28T14:57:02Z

Nipunn Koorapati (@nipunn1313) thanks for the contribution. Waiting the master branch allow to merge your contribution :)

Octavia Squidington III (octavia-squidington-iii) added the area/connectors Connector related issues label Sep 28, 2023

Octavia Squidington III (octavia-squidington-iii) added community connectors/source/convex area/documentation Improvements or additions to documentation labels Sep 28, 2023

Improve formatting and fix mypy for source-convex connector.

6f42e0f

vercel bot deployed to Preview September 28, 2023 21:49 View deployment

Nipunn Koorapati (nipunn1313) added 2 commits September 28, 2023 14:50

Switch default value from convex_json to json

a0795d9

Add fmt param to the source-convex connector for future extensibility. May add it to the configuration options in the future, but for now, just have it there for testability, defaulting to json format.

Add to release notes

614a12f

Nipunn Koorapati (nipunn1313) force-pushed the nipunn/formatting branch from 6f9fb70 to 614a12f Compare September 28, 2023 21:50

Nipunn Koorapati (nipunn1313) changed the title ~~🚨✨ source-convex: Convex source - switch the JSON format requested from Convex~~ 🚨✨ Source Convex - switch the JSON format requested from Convex Sep 28, 2023

vercel bot deployed to Preview September 28, 2023 21:52 View deployment

Marcos Marx (marcosmarxm) previously requested changes Oct 10, 2023

View reviewed changes

Remove parse_config

3c78894

Nipunn Koorapati (nipunn1313) requested a review from Marcos Marx (marcosmarxm) October 11, 2023 19:46

Serhii Lazebnyi (lazebnyi) self-requested a review November 24, 2023 16:57

Serhii Lazebnyi (lazebnyi) approved these changes Nov 24, 2023

View reviewed changes

Merge branch 'master' into nipunn/formatting

1df3165

vercel bot deployed to Preview November 27, 2023 16:12 View deployment

Marcos Marx (marcosmarxm) approved these changes Nov 27, 2023

View reviewed changes

Marcos Marx (marcosmarxm) added 4 commits November 27, 2023 16:39

Automated Commit - Formatting Changes

5f53091

Merge branch 'master' into nipunn/formatting

e43a354

Merge branch 'master' into nipunn/formatting

8926533

Merge branch 'master' into nipunn/formatting

58bb63b

vercel bot deployed to Preview November 28, 2023 14:46 View deployment

Marcos Marx (marcosmarxm) merged commit eb25eac into airbytehq:master Nov 29, 2023

Conversation

Nipunn Koorapati (nipunn1313) commented Sep 28, 2023

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Actions

Community member or Airbyter

Airbyter

Uh oh!

vercel bot commented Sep 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 28, 2023

Before Merging a Connector Pull Request

Uh oh!

Nipunn Koorapati (nipunn1313) commented Oct 3, 2023

Uh oh!

Marcos Marx (marcosmarxm) left a comment

Choose a reason for hiding this comment

Uh oh!

Nipunn Koorapati (nipunn1313) commented Oct 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nipunn Koorapati (nipunn1313) commented Oct 16, 2023

Uh oh!

Nipunn Koorapati (nipunn1313) commented Oct 28, 2023

Uh oh!

Marcos Marx (marcosmarxm) commented Oct 30, 2023

Uh oh!

Nipunn Koorapati (nipunn1313) commented Nov 13, 2023

Uh oh!

Natalie Kwong (nataliekwong) commented Nov 17, 2023

Uh oh!

Serhii Lazebnyi (lazebnyi) left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Serhii Lazebnyi (lazebnyi) Nov 24, 2023

Choose a reason for hiding this comment

Uh oh!

Serhii Lazebnyi (lazebnyi) left a comment

Choose a reason for hiding this comment

Uh oh!

Natalie Kwong (nataliekwong) commented Nov 25, 2023

Uh oh!

Nipunn Koorapati (nipunn1313) commented Nov 25, 2023

Uh oh!

Marcos Marx (marcosmarxm) left a comment

Choose a reason for hiding this comment

Uh oh!

Marcos Marx (marcosmarxm) commented Nov 28, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

vercel bot commented Sep 28, 2023 •

edited

Loading

Nipunn Koorapati (nipunn1313) commented Oct 11, 2023 •

edited

Loading

Serhii Lazebnyi (lazebnyi) left a comment •

edited

Loading