🚨✨ Source Convex - switch the JSON format requested from Convex #30853

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.

nipunn1313 · 2023-10-03T17:58:53Z

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

marcosmarxm

Thanks for the contribution @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.

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.

nipunn1313 · 2023-10-16T20:49:31Z

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

nipunn1313 · 2023-10-28T00:17:59Z

Hello. I wanted to check in here again.

marcosmarxm · 2023-10-30T13:15:42Z

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

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?).

nataliekwong · 2023-11-17T06:33:03Z

Hey @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.

@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.

lazebnyi

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

lazebnyi · 2023-11-24T17:06:45Z

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



 # Source
 class SourceConvex(AbstractSource):
    def _json_schemas(self, config: ConvexConfig) -> requests.Response:
        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.

lazebnyi

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

nataliekwong · 2023-11-25T12:17:06Z

@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. @katmarkham

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

marcosmarxm

Releasing without breaking change as @nataliekwong commented.

marcosmarxm · 2023-11-28T14:57:02Z

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

octavia-squidington-iii added the area/connectors Connector related issues label Sep 28, 2023

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

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

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

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

marcosmarxm previously requested changes Oct 10, 2023

View reviewed changes

Remove parse_config

3c78894

nipunn1313 requested a review from marcosmarxm October 11, 2023 19:46

lazebnyi self-requested a review November 24, 2023 16:57

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

marcosmarxm approved these changes Nov 27, 2023

View reviewed changes

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

marcosmarxm merged commit eb25eac into airbytehq:master Nov 29, 2023
31 of 38 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🚨✨ Source Convex - switch the JSON format requested from Convex #30853

🚨✨ Source Convex - switch the JSON format requested from Convex #30853

nipunn1313 commented Sep 28, 2023

vercel bot commented Sep 28, 2023 •

edited

github-actions bot commented Sep 28, 2023

nipunn1313 commented Oct 3, 2023

marcosmarxm left a comment

nipunn1313 commented Oct 11, 2023 •

edited

nipunn1313 commented Oct 16, 2023

nipunn1313 commented Oct 28, 2023

marcosmarxm commented Oct 30, 2023

nipunn1313 commented Nov 13, 2023

nataliekwong commented Nov 17, 2023

lazebnyi left a comment •

edited

lazebnyi Nov 24, 2023

lazebnyi left a comment

nataliekwong commented Nov 25, 2023

nipunn1313 commented Nov 25, 2023

marcosmarxm left a comment

marcosmarxm commented Nov 28, 2023

🚨✨ Source Convex - switch the JSON format requested from Convex #30853

🚨✨ Source Convex - switch the JSON format requested from Convex #30853

Conversation

nipunn1313 commented Sep 28, 2023

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Actions

Community member or Airbyter

Airbyter

vercel bot commented Sep 28, 2023 • edited

github-actions bot commented Sep 28, 2023

Before Merging a Connector Pull Request

nipunn1313 commented Oct 3, 2023

marcosmarxm left a comment

Choose a reason for hiding this comment

nipunn1313 commented Oct 11, 2023 • edited

nipunn1313 commented Oct 16, 2023

nipunn1313 commented Oct 28, 2023

marcosmarxm commented Oct 30, 2023

nipunn1313 commented Nov 13, 2023

nataliekwong commented Nov 17, 2023

lazebnyi left a comment • edited

Choose a reason for hiding this comment

lazebnyi Nov 24, 2023

Choose a reason for hiding this comment

lazebnyi left a comment

Choose a reason for hiding this comment

nataliekwong commented Nov 25, 2023

nipunn1313 commented Nov 25, 2023

marcosmarxm left a comment

Choose a reason for hiding this comment

marcosmarxm commented Nov 28, 2023

vercel bot commented Sep 28, 2023 •

edited

nipunn1313 commented Oct 11, 2023 •

edited

lazebnyi left a comment •

edited