🎉 Destination Weaviate: Support any string based ID and fix issues with additionalProperties #22527

samos123 · 2023-02-07T21:32:02Z

What

Previously string based IDs that weren't convertable to hex would throw an exception. This fixes that issue by converting strings to 128 bit integer and converting the md5 hex into a UUID
Update Weaviate to 1.17.3
fixes issue with arrays of uknown types see Destination Weaviate: Support array of known type #22530
Fixes issue with additionalProperties of type object and of type Array/object

How

Convert a string to md5, then use the hex of the md5 to generate a UUID from a string.
Create a json string of uknown array type
Create json string for additional props of type object and type array of objects

🚨 User Impact 🚨

This fixes issues with data sources that have IDs like some string ID that isn't hex, uuid, or integer

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

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.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub by running the /publish command described here
After the connector is published, connector added to connector index as described here
Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

Updating a connector

Community member or Airbyter

Grant edit access to maintainers (instructions)
Secrets in the connector's spec are annotated with airbyte_secret
Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
Code reviews completed
Documentation updated
- Connector's README.md
- Connector's bootstrap.md. See description and examples
- Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
PR name follows PR naming conventions

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.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub and connector version bumped by running the /publish command described here

Connector Generator

Issue acceptance criteria met
PR name follows PR naming conventions
If adding a new generator, add it to the list of scaffold modules being tested
The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
Documentation which references the generator is updated as needed

samos123 · 2023-02-09T06:45:32Z

rebased with master

samos123 · 2023-02-09T06:49:42Z

integration tests are passing, see screenshot

marcosmarxm · 2023-02-10T08:29:27Z

/test connector=connectors/destination-weaviate

🕑 connectors/destination-weaviate https://github.com/airbytehq/airbyte/actions/runs/4142144002
❌ connectors/destination-weaviate https://github.com/airbytehq/airbyte/actions/runs/4142144002
🐛

Build Passed

Test summary info:

All Passed

marcosmarxm · 2023-02-10T08:53:52Z

@samos123 new QA tests are failing because the connector doesn't have an icon. do you mind to add it?

samos123 · 2023-02-10T17:24:36Z

I don't know how to add the icon and there doesn't seem to be any docs on how to add an icon: #20864

Could we merge without icon? or could you tell me the steps to add one?

samos123 · 2023-02-10T23:45:08Z

after skimming through a bunch of source code finnally found how to add the icon I think? Please review again.

samos123 · 2023-02-10T23:50:02Z

integration tests still passing after latest rebase and commits:

timroes · 2023-02-13T13:27:26Z

@Upmitt Could you please have a look here to check if the connector icon is in correct shape for what we need, and if not provide a new one please.

marcosmarxm · 2023-02-13T21:19:24Z

/test connector=connectors/destination-weaviate

🕑 connectors/destination-weaviate https://github.com/airbytehq/airbyte/actions/runs/4167933984
✅ connectors/destination-weaviate https://github.com/airbytehq/airbyte/actions/runs/4167933984
Python tests coverage:

Name                                  Stmts   Miss  Cover
---------------------------------------------------------
destination_weaviate/utils.py            59      0   100%
destination_weaviate/__init__.py          2      0   100%
destination_weaviate/client.py          106      6    94%
destination_weaviate/destination.py      35      2    94%
---------------------------------------------------------
TOTAL                                   202      8    96%

Build Passed

Test summary info:

All Passed

marcosmarxm · 2023-02-13T22:13:50Z

@samos123 I'll finish tomorrow the reviewing process. Tests are good thanks for that.

marcosmarxm · 2023-02-15T13:40:23Z

@samos123 can you fix the errors? ./gradlew :airbyte-integrations:connectors:destination-weaviate:flakeCheck

* Previously string based IDs that weren't convertable to hex would throw an exception. This fixes that issue by converting strings to 128 bit integer and converting the md5 hex into a UUID * Update Weaviate to 1.17.3

Closes airbytehq#22530

This seems to happen when records sometimes don't contain all the properties that are defined in the schema. This was the error observed by a user connecting Slack <> Weaviate: property 'reactions': invalid dataType: dataType must have at least one element, class 'Threads' not present in schema"

samos123 · 2023-02-15T23:15:29Z

@marcosmarxm I've fixed the flake tests, sorry about that. Could you please give it another review? I also rebased with latest master

marcosmarxm · 2023-02-27T14:31:41Z

/publish connector=connectors/destination-weaviate

🕑 Publishing the following connectors:
connectors/destination-weaviate
https://github.com/airbytehq/airbyte/actions/runs/4283470608

Connector	Did it publish?	Were definitions generated?
connectors/destination-weaviate	❌	❌

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

marcosmarxm · 2023-02-28T19:48:21Z

/publish connector=connectors/destination-weaviate

🕑 Publishing the following connectors:
connectors/destination-weaviate
https://github.com/airbytehq/airbyte/actions/runs/4296814438

Connector	Did it publish?	Were definitions generated?
connectors/destination-weaviate	✅	✅

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

marcosmarxm

Thanks for the contribtuion @samos123 sorry the long delay to publish and merge it.

…th additionalProperties (airbytehq#22527) * Support any string based ID * Previously string based IDs that weren't convertable to hex would throw an exception. This fixes that issue by converting strings to 128 bit integer and converting the md5 hex into a UUID * Update Weaviate to 1.17.3 * Fix issue with arrays of no data type Closes airbytehq#22530 * add more testing coverage * fix error where dataType wasn't set in weaviate This seems to happen when records sometimes don't contain all the properties that are defined in the schema. This was the error observed by a user connecting Slack <> Weaviate: property 'reactions': invalid dataType: dataType must have at least one element, class 'Threads' not present in schema" * add test case for additionalProperties * Support additionalProps of object and array of obj * Update changelog * add weaviate icon * fix flake tests * add commit * make build m1 work * make build m1 work * auto-bump connector version --------- Co-authored-by: marcosmarxm <marcosmarxm@gmail.com> Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com> Co-authored-by: Marcos Marx <marcosmarxm@users.noreply.github.com>

samos123 changed the title ~~🎉 Destination Redshift: Support any string based ID~~ 🎉 Destination Weaviate: Support any string based ID Feb 7, 2023

octavia-squidington-iii added area/connectors Connector related issues bounty community connectors/destination/weaviate labels Feb 7, 2023

samos123 force-pushed the weaviate-fix-id-schema-issue branch 3 times, most recently from 5f93dd3 to 555ae3a Compare February 7, 2023 21:40

samos123 changed the title ~~🎉 Destination Weaviate: Support any string based ID~~ 🎉 Destination Weaviate: Support any string based ID and fix issues with additionalProperties Feb 8, 2023

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

samos123 force-pushed the weaviate-fix-id-schema-issue branch from be8f724 to 00aad30 Compare February 9, 2023 06:45

marcosmarxm self-assigned this Feb 10, 2023

samos123 mentioned this pull request Feb 10, 2023

Document process for adding icons to alpha connectors #20864

Open

samos123 force-pushed the weaviate-fix-id-schema-issue branch from aa2bca4 to aecee60 Compare February 10, 2023 23:46

samos123 added 5 commits February 15, 2023 15:02

Support any string based ID

73034a8

* Previously string based IDs that weren't convertable to hex would throw an exception. This fixes that issue by converting strings to 128 bit integer and converting the md5 hex into a UUID * Update Weaviate to 1.17.3

Fix issue with arrays of no data type

bae1015

Closes airbytehq#22530

add more testing coverage

bb54082

add test case for additionalProperties

c7f1516

samos123 added 4 commits February 15, 2023 15:02

Support additionalProps of object and array of obj

6f52576

Update changelog

8fe2107

add weaviate icon

a2776d1

fix flake tests

ef43e5a

samos123 force-pushed the weaviate-fix-id-schema-issue branch from be4b76e to ef43e5a Compare February 15, 2023 23:03

samos123 and others added 2 commits February 25, 2023 06:49

Merge branch 'master' into weaviate-fix-id-schema-issue

82bbd85

add commit

af8f173

marcosmarxm added 2 commits February 28, 2023 20:46

make build m1 work

484f9db

make build m1 work

605105f

auto-bump connector version

03cdcb0

marcosmarxm approved these changes Feb 28, 2023

View reviewed changes

Merge branch 'master' into weaviate-fix-id-schema-issue

0363a25

marcosmarxm merged commit 3d73771 into airbytehq:master Feb 28, 2023

marcosmarxm mentioned this pull request Mar 3, 2023

Fix no data type error #22602

Closed

37 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🎉 Destination Weaviate: Support any string based ID and fix issues with additionalProperties #22527

🎉 Destination Weaviate: Support any string based ID and fix issues with additionalProperties #22527

samos123 commented Feb 7, 2023 •

edited

Loading

samos123 commented Feb 9, 2023

samos123 commented Feb 9, 2023

marcosmarxm commented Feb 10, 2023 •

edited by github-actions bot

Loading

marcosmarxm commented Feb 10, 2023

samos123 commented Feb 10, 2023

samos123 commented Feb 10, 2023

samos123 commented Feb 10, 2023

timroes commented Feb 13, 2023

marcosmarxm commented Feb 13, 2023 •

edited by github-actions bot

Loading

marcosmarxm commented Feb 13, 2023

marcosmarxm commented Feb 15, 2023

samos123 commented Feb 15, 2023 •

edited

Loading

marcosmarxm commented Feb 27, 2023 •

edited by github-actions bot

Loading

marcosmarxm commented Feb 28, 2023 •

edited by github-actions bot

Loading

marcosmarxm left a comment

🎉 Destination Weaviate: Support any string based ID and fix issues with additionalProperties #22527

🎉 Destination Weaviate: Support any string based ID and fix issues with additionalProperties #22527

Conversation

samos123 commented Feb 7, 2023 • edited Loading

What

How

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

samos123 commented Feb 9, 2023

samos123 commented Feb 9, 2023

marcosmarxm commented Feb 10, 2023 • edited by github-actions bot Loading

Build Passed

marcosmarxm commented Feb 10, 2023

samos123 commented Feb 10, 2023

samos123 commented Feb 10, 2023

samos123 commented Feb 10, 2023

timroes commented Feb 13, 2023

marcosmarxm commented Feb 13, 2023 • edited by github-actions bot Loading

Build Passed

marcosmarxm commented Feb 13, 2023

marcosmarxm commented Feb 15, 2023

samos123 commented Feb 15, 2023 • edited Loading

marcosmarxm commented Feb 27, 2023 • edited by github-actions bot Loading

marcosmarxm commented Feb 28, 2023 • edited by github-actions bot Loading

marcosmarxm left a comment

Choose a reason for hiding this comment

samos123 commented Feb 7, 2023 •

edited

Loading

marcosmarxm commented Feb 10, 2023 •

edited by github-actions bot

Loading

marcosmarxm commented Feb 13, 2023 •

edited by github-actions bot

Loading

samos123 commented Feb 15, 2023 •

edited

Loading

marcosmarxm commented Feb 27, 2023 •

edited by github-actions bot

Loading

marcosmarxm commented Feb 28, 2023 •

edited by github-actions bot

Loading