🎉 New Destination: Datastax Astra #34058

anomnaco · 2024-01-09T18:10:14Z

New destination for vector database Datastax Astra via the new JSON API. Requires python cdk to include embedder and other vector db utilities included in the cdk.

github-actions · 2024-01-09T18:10:34Z

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.

vercel · 2024-01-09T18:12:10Z

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	Jan 25, 2024 8:59am

flash1293

Hey, this PR looks great already! I left some higher level comments but there is nothing big in there. Let me know once I can take another look

flash1293 · 2024-01-15T12:04:39Z

airbyte-integrations/connectors/destination-astra/integration_tests/integration_test.py

+            logging.getLogger("airbyte"), invalid_config)
+        assert outcome.status == Status.FAILED
+
+    def test_write(self):


for this test to run successfully in CI, there needs to be an Airbyte controlled astra account. It seems like there is no free tier to easily set that up, so we need to check how to make that work in a robust fashion.

If that's difficult to do I'm also happy to skip this test in CI so it can be used to test locally.

Astra does have a free tier. Confirmed with Datastax that it gives access to vector search and the JSON API so this should be doable.

Hey @flash1293 - let me know if you need anymore help with this. I think we resolved this by turning off auto-hibernation for your org.

flash1293 · 2024-01-15T12:05:18Z

docs/integrations/destinations/astra.md

@@ -0,0 +1,64 @@
+# Astra Destination


This file is shown in the Airbyte UI when users set up the connector, so it's not directed at a technical audience developing / interacting with the python code directly.

Check out

airbyte/docs/integrations/destinations/chroma.md

Lines 2 to 3 in 6dff143

This page guides you through the process of setting up the [Chroma](https://docs.trychroma.com/?lang=py) destination connector.

for example for how these normally look.

It should guide the user through creating the db in the astra UI and so on.

flash1293 · 2024-01-15T12:06:19Z

airbyte-integrations/connectors/destination-astra/setup.py

+from setuptools import find_packages, setup
+
+MAIN_REQUIREMENTS = [
+    "airbyte-cdk==0.55.1",


Using airbyte-cdk[vector-db-based]==0.57.0 here makes sure all transitive dependencies like langchain and openai are installed properly.

flash1293 · 2024-01-15T12:09:00Z

airbyte-integrations/connectors/destination-astra/destination_astra/indexer.py

+            self.client.create_collection(self.config.collection)
+
+    def pre_sync(self, catalog: ConfiguredAirbyteCatalog):
+        self._create_collection()


It looks like the pre_sync hook here is creating the collection automatically.

However, in the normal user flow it works like this:

User fills out form in Airbyte UI

Airbyte platform runs check - if it succeeds, the config is saved

Sync can be started (calling pre_sync and index)

So in this case if the collection isn't created, a user won't be able to create the destination in the UI. It looks like we should create the collection in the check method as well as the pre_sync to make sure it works.

flash1293 · 2024-01-15T12:13:10Z

airbyte-integrations/connectors/destination-astra/destination_astra/astra_client.py

+        self.embedding_dim = embedding_dim
+        self.similarity_function = similarity_function
+
+        self.request_base_url = f"https://{self.astra_id}-{self.astra_region}.apps.astra.datastax.com/api/json/v1/{self.keyspace_name}"


I've seen in the UI that there is a handy "API Endpoint" field that can be copied with a single click. Rather than requiring the Airbyte user to copy paste id, region and so on manually, what do you think about just having a single "endpoint" field in the config and using that?

The pattern and pattern_descriptor fields in the config can be used to properly document this for the user (and to enforce a proper format right in the UI). See this example in Google drive for folder URLs which is pretty similar:

airbyte/airbyte-integrations/connectors/source-google-drive/source_google_drive/spec.py

Line 54 in 5324da8

folder_url: str = Field(

…ra into destination-astra

flash1293

Played around with it and it works pretty well, great job!

Had some smaller comments left.

Also, I tried to run the unit tests and they are failing for me, could you take a look at that?

flash1293 · 2024-01-18T15:51:08Z

airbyte-integrations/connectors/destination-astra/destination_astra/config.py

+        ...,
+        title="AstraDB Endpoint",
+        description="AstraDB Endpoint",
+        pattern="^https:\/\/([a-z]|[0-9]){8}-([a-z]|[0-9]){4}-([a-z]|[0-9]){4}-([a-z]|[0-9]){4}-([a-z]|[0-9]){12}-(af|il|ap|ca|eu|me|sa|us|cn|us-gov|us-iso|us-isob)-(central|north|(north(?:east|west))|south|south(?:east|west)|east|west)([0-9]{1})\\.apps\\.astra\\.datastax\\.com",


This doesn't work for my test-db: https://551e73d7-3904-4cc8-844e-20b5ff3404f9-us-east-2.apps.astra.datastax.com

Maybe it should be relaxed a bit? If it's wrong, the check will fail anyways, so this is mostly to catch early if the user is copy/pasting something completely wrong in there.

flash1293 · 2024-01-18T15:51:32Z

airbyte-integrations/connectors/destination-astra/destination_astra/config.py

+        title="AstraDB Endpoint",
+        description="AstraDB Endpoint",
+        pattern="^https:\/\/([a-z]|[0-9]){8}-([a-z]|[0-9]){4}-([a-z]|[0-9]){4}-([a-z]|[0-9]){4}-([a-z]|[0-9]){12}-(af|il|ap|ca|eu|me|sa|us|cn|us-gov|us-iso|us-isob)-(central|north|(north(?:east|west))|south|south(?:east|west)|east|west)([0-9]{1})\\.apps\\.astra\\.datastax\\.com",
+        examples=["us-east1"],


The example should be the full value that needs to be entered by the user.

flash1293 · 2024-01-18T16:02:57Z

airbyte-integrations/connectors/destination-astra/destination_astra/destination.py

+    def check(self, logger: AirbyteLogger, config: Mapping[str, Any]) -> AirbyteConnectionStatus:
+        parsed_config = ConfigModel.parse_obj(config)
+        self._init_indexer(parsed_config)
+        self.indexer._create_collection()


This should be in the indexers check method instead. Exceptions thrown from here won't resolve in a nice error message because they are not handled (the indexer check method already has a try-catch clause to print a nice error message.

flash1293 · 2024-01-18T16:34:25Z

docs/integrations/destinations/astra.md

+- Click Create Database.
+-- You are redirected to your new database’s Overview screen. Your database starts in Pending status before transitioning to Initializing. You’ll receive a notification once your database is initialized.
+
+#### Setting up a Vector Collection


Do we need this section with the new logic of the connector auto-creating the collection? If there's a good reason to still do it manually in some cases we can leave it in with a note that the connector can also take care of it, wdyt?

flash1293 · 2024-01-18T16:35:48Z

docs/integrations/destinations/astra.md

+
+## Supported Sync Modes
+
+Full Refresh Sync


The connector supports all sync modes (the vector DB CDK takes care of this if you implement index and delete on the indexer properly which you did. You can use the same table as here:

airbyte/docs/integrations/destinations/chroma.md

Line 10 in 6dff143

| Full Refresh Sync | Yes | |

flash1293 · 2024-01-18T16:36:43Z

airbyte-integrations/connectors/destination-astra/destination_astra/spec.json

This file should be in the integration_tests folder (don't forget to adjust the reference from acceptance-test-config.yml)

hc33brackles · 2024-01-19T22:27:05Z

Thanks for this feedback @flash1293 -- we are close to resolving all of these issues and updating the branch with the correction. I'll check back in on Monday - there's still a final issue we are working on resolving.

anomnaco · 2024-01-22T16:27:47Z

Moved create collection back to destination.py because it was causing issues with integration and unit tests.

hc33brackles · 2024-01-22T20:59:59Z

@flash1293 - we are done with your most recent comments as well. Please let me know what the next steps are!

…ra into destination-astra

flash1293

Hi @hc33brackles and @anomnaco , apologies, I was off yesterday and couldn't give it another look.

This is starting to look very good! I updated a few small things:

Apply formatting
Add integration test secret to our CI
Update spec for integration test
Prepare the icon in our platform repo so it will show properly

CI is currently running here: #34442

Let's see whether it works. I need to do this on a separate PR for permission reasons. Could you give me write access to the destination-astra on the Anant fork so I can push the same things over there (or do it yourself)? This will make it easier to wrap this work up.

Left two smaller comments in the PR

flash1293 · 2024-01-23T17:07:38Z

airbyte-integrations/connectors/destination-astra/destination_astra/destination.py

+
+    def check(self, logger: AirbyteLogger, config: Mapping[str, Any]) -> AirbyteConnectionStatus:
+        parsed_config = ConfigModel.parse_obj(config)
+        self._init_indexer(parsed_config)


On https://github.com/airbytehq/airbyte/pull/34058/files#r1459806142 you wrote you moved the create_collection call, but it seems like some merge foo brought it back (it's also not in the indexer). Could you double check please?

Its on the next line here

@anomnaco yes, it shouldn’t be here in the destination class, it should be in the indexer class check method so it’s properly wrapped in a try/except block and a meaningful error message is produced on check

That caused issues with the unit and integration tests

Hi @flash1293 - I've been chatting with Obi about this offline. He was testing with this in the indexer class check method and got failures with the unit tests with it there. Do you need us to look deeper into getting this to work in the way that you suggested or is it acceptable here? We can obviously dig deeper and get this to work in the other location if we need to. Would adding more meaningful error messaging with the function where it is now work?

We are working on resolving this now.

The important point to me is that the user gets an actionable error message on check as part of a well-formed status message and the connector doesn't crash as this won't produce a nice output in the UI.

flash1293 · 2024-01-23T17:09:35Z

airbyte-integrations/connectors/destination-astra/destination_astra/config.py

+    astra_db_endpoint: str = Field(
+        ...,
+        title="AstraDB Endpoint",
+        airbyte_secret=True,


Could we keep the endpoint non-secret? This is causing an issue in the UI when editing an existing configuration:

I think nobody ran into this problem yet. If something is marked as secret, two things happen:

it's not shown in the UI in cleartext

It is stored in a separate secret manager, not in the regular Airbyte DB with the rest of the configuration

I might miss something, but it seems like the endpoint isn't sensitive in this sense and can be stored with the rest of the configuration (the app token definitely is and should be declared a secret)

flash1293 · 2024-01-24T11:00:10Z

@anomnaco @hc33brackles About the problem that's still left - maybe the create_collection call needs to be mocked out in the unit test (plus validation it was called as expected)? https://github.com/airbytehq/airbyte/pull/34058/files#diff-ad0e480b627023296a1ae9ff8e684e73cf34f8337430d07d549b65ae583158acR157

…ra into destination-astra

flash1293

I had to fix the unit tests as the create_collection call needs to be mocked out now, but tested again and everything seems to work well, LGTM.

We have a CI issue right now, as soon as that's resolved I'm going to merge

Co-authored-by: Joe Reuter <[email protected]>

Destination Astra

3b6b82f

octavia-squidington-iii added area/connectors Connector related issues area/documentation Improvements or additions to documentation community connectors/destination/astra labels Jan 9, 2024

vercel bot deployed to Preview January 9, 2024 18:12 View deployment

Merge remote-tracking branch 'origin/master' into destination-astra

441a244

flash1293 reviewed Jan 15, 2024

View reviewed changes

anomnaco added 5 commits January 17, 2024 19:12

Updating main airbyte requirement

06b8950

Removing extraneous reqs

f0d7c26

Add create collection to check

0326741

Switch from id and region to endpoint

86d3c67

Updating Astra Destination Docs

6e9c528

vercel bot deployed to Preview January 17, 2024 21:15 View deployment

Joe Reuter added 2 commits January 18, 2024 15:58

Merge branch 'destination-astra' of github.com:Anant/airbyte-dest-ast…

ae832cc

…ra into destination-astra

Merge remote-tracking branch 'origin/master' into destination-astra

d0fb593

flash1293 reviewed Jan 18, 2024

View reviewed changes

PR Comments except Unit Tests

5f52ca7

vercel bot deployed to Preview January 19, 2024 21:29 View deployment

Integration and unit test fixes

c209d34

Joe Reuter added 4 commits January 23, 2024 16:28

Merge branch 'destination-astra' of github.com:Anant/airbyte-dest-ast…

7b2f020

…ra into destination-astra

Merge remote-tracking branch 'origin/master' into destination-astra

7259b3a

cleanup

975b89c

enable

705131c

flash1293 mentioned this pull request Jan 23, 2024

CI PR for Destination astra #34442

Closed

Merge remote-tracking branch 'origin/master' into destination-astra

8e50884

flash1293 reviewed Jan 23, 2024

View reviewed changes

Remove airbyte_secret from endpoint

276da69

anomnaco and others added 4 commits January 24, 2024 20:11

Move create collection indexer and Readable error message

c17b7c1

Merge branch 'destination-astra' of github.com:Anant/airbyte-dest-ast…

6e03f43

…ra into destination-astra

Merge remote-tracking branch 'origin/master' into destination-astra

59b639f

fix some things

4d2f5f4

flash1293 approved these changes Jan 25, 2024

View reviewed changes

update docs

149c8ad

vercel bot deployed to Preview January 25, 2024 08:59 View deployment

flash1293 merged commit ff12ddf into airbytehq:master Jan 25, 2024
35 of 40 checks passed

jatinyadav-cc pushed a commit to ollionorg/datapipes-airbyte that referenced this pull request Feb 21, 2024

🎉 New Destination: Datastax Astra (airbytehq#34058)

5217dd7

Co-authored-by: Joe Reuter <[email protected]>

jatinyadav-cc pushed a commit to ollionorg/datapipes-airbyte that referenced this pull request Feb 26, 2024

🎉 New Destination: Datastax Astra (airbytehq#34058)

ca4eef1

Co-authored-by: Joe Reuter <[email protected]>

jatinyadav-cc pushed a commit to ollionorg/datapipes-airbyte that referenced this pull request Feb 26, 2024

🎉 New Destination: Datastax Astra (airbytehq#34058)

638ced9

Co-authored-by: Joe Reuter <[email protected]>

	This page guides you through the process of setting up the [Chroma](https://docs.trychroma.com/?lang=py) destination connector.

🎉 New Destination: Datastax Astra #34058

🎉 New Destination: Datastax Astra #34058

Conversation

anomnaco commented Jan 9, 2024

github-actions bot commented Jan 9, 2024 • edited by flash1293 Loading

Before Merging a Connector Pull Request

vercel bot commented Jan 9, 2024 • edited Loading

flash1293 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

Choose a reason for hiding this comment

flash1293 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hc33brackles commented Jan 19, 2024

anomnaco commented Jan 22, 2024

hc33brackles commented Jan 22, 2024

flash1293 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

hc33brackles Jan 23, 2024 • 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

Choose a reason for hiding this comment

flash1293 commented Jan 24, 2024

flash1293 left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 9, 2024 •

edited by flash1293

Loading

vercel bot commented Jan 9, 2024 •

edited

Loading

hc33brackles Jan 23, 2024 •

edited

Loading