Add component to index aws opensearch #740

shub-kris · 2023-12-21T13:27:55Z

This PR aims to add support for indexing to AWS OpenSearch.

mrchtr

Thanks @shub-kris! Looks already quite promising. Could you try to add a component test as well? You could have a look into the write to Qdrant component. There was something similar implemented.

mrchtr · 2023-12-21T13:49:32Z

components/index_aws_opensearch/src/main.py

+        Args:
+            index_body (Dict[str, Any]): Parameters that specify index settings, mappings, and aliases for newly created index.
+        """
+        response = self.client.indices.create(self.index_name, body=index_body)


What happens if the index already exists? Will the index be overwritten? Maybe we add a check if an index exists.

Let me check what it does if an index already exists.

Added tests too

Thanks :)

I think you still need to modify the Docker image to be able to run the test with it similar to this so that you can run tests using

docker build . --target test

Could you please update and test?

Thanks for pointing it out. I almost missed it. But now, Updated and tested

great :)

did you manage to fix the failing pipelines?

after installing pre-commit you should run

pre-commit run --all-files

mrchtr · 2023-12-21T14:03:38Z

components/index_aws_opensearch/src/main.py

+        Args:
+            dataframe (dd.DataFrame): The Dask DataFrame containing the data to be written.
+        """
+        if not self.client.indices.exists(index=self.index_name):


I think we could call the create_index here if the index doesn't exist.

Sure, we can do that

It's not needed anymore as I am doing it inside the __init__ function.

Removed the Exception too

PhilippeMoussalli · 2023-12-21T14:33:15Z

components/index_aws_opensearch/src/main.py

+        region: str,
+        index_name: str,
+        index_body: Dict[str, Any],
+        port: int = 443,


I would not include the default types here for the optional argument since you already define them in the spec and it can be misleading (they are not actually used here it will always default to the ones defined in the spec). You can instead define their types as optional just so that it's clear that they do have default values and they don't need to be explicitly defined. Similar to here

PhilippeMoussalli · 2023-12-21T14:35:45Z

components/index_aws_opensearch/src/main.py

+        """Creates an index in AWS OpenSearch.
+
+        Args:
+            index_body (Dict[str, Any]): Parameters that specify index settings,


Suggested change

index_body (Dict[str, Any]): Parameters that specify index settings,

index_body: Parameters that specify index settings,

Our convention, we only define data types in the argument and not the docstring (single source of truth)

PhilippeMoussalli · 2023-12-21T14:35:58Z

components/index_aws_opensearch/src/main.py

+        Writes the data from the given Dask DataFrame to AWS OpenSearch Index.
+
+        Args:
+            dataframe (dd.DataFrame): The Dask DataFrame containing the data to be written.


Suggested change

dataframe (dd.DataFrame): The Dask DataFrame containing the data to be written.

dataframe: The Dask DataFrame containing the data to be written.

PhilippeMoussalli · 2023-12-21T14:37:58Z

components/index_aws_opensearch/src/main.py

+            msg = f"Index: {self.index_name} doesn't exist. Please Create"
+            raise ValueError(msg)
+
+        for part in dataframe.partitions:


Suggested change

for part in dataframe.partitions:

for part in tqdm(

dataframe.partitions,

desc="Processing partitions",

total=dataframe.npartitions,

):

Useful to add logs in case the dataset is large. Similar to https://github.com/ml6team/fondant/blob/main/components/index_weaviate/src/main.py

Don't forget to add tqdm to the list of requirements

components/index_aws_opensearch/requirements.txt

RobbeSneyders · 2023-12-21T16:03:51Z

FYI, there's another test example in the index_weaviate component.

You can install pre-commit to take care of all the static checks for you (which are the reason the pipeline is currently failing). Just execute the following 3 commands:

pip install poetry
poetry install --all-extras
pre-commit install

PhilippeMoussalli

Thanks @shub-kris! Nice first contribution

RobbeSneyders · 2023-12-22T13:06:42Z

components/index_aws_opensearch/tests/aws_opensearch_test.py

+    """
+    index_name = "pytest-index"
+    aws_os_comp = IndexAWSOpenSearchComponent(
+        host="search-genai-vectordb-domain-f7vxqkogveaie2qdrivnkr66om.eu-west-1.es.amazonaws.com",


Is there a way to test this locally instead?

Otherwise it might be better to mock this.

Mocking is the only solution to test it without having access to AWS OpenSearch Cluster.

Yes indeed, so I would prefer that. Currently I cannot test this component myself. The goal of these tests is to be unit tests, we have other mechanism for integration testing.

Will do that and update the PR

@RobbeSneyders updated

…b-kris/fondant into feature/index-aws-opensearch

RobbeSneyders

Thanks @shub-kris!

Shubham Krishna added 2 commits December 21, 2023 14:11

Add support for indexing to AWS OpenSearch

8197306

Add creation of index inside init

ea0050e

shub-kris requested a review from mrchtr December 21, 2023 13:28

Shubham Krishna and others added 2 commits December 21, 2023 14:32

Replace Dict[str,Any] to dict in README

a6730a5

Merge branch 'main' into feature/index-aws-opensearch

7add822

shub-kris requested review from RobbeSneyders and PhilippeMoussalli December 21, 2023 13:42

Fix code formatting and README

2ca8d35

mrchtr reviewed Dec 21, 2023

View reviewed changes

PhilippeMoussalli reviewed Dec 21, 2023

View reviewed changes

components/index_aws_opensearch/requirements.txt Show resolved Hide resolved

RobbeSneyders changed the title ~~Feature/index aws opensearch~~ Add component to index aws opensearch Dec 21, 2023

Shubham Krishna and others added 3 commits December 21, 2023 17:39

Fix docstrings, failing tests, add tqdm

1a6a8a0

Add Tests

d0179af

Merge branch 'main' into feature/index-aws-opensearch

fc251b5

shub-kris requested review from PhilippeMoussalli and mrchtr December 21, 2023 20:16

Shubham Krishna and others added 4 commits December 22, 2023 10:06

Add Tests in Dockerfile

39f6d91

Merge branch 'main' into feature/index-aws-opensearch

08a6756

Fix README

09f93fd

Merge branch 'main' into feature/index-aws-opensearch

4a20c90

PhilippeMoussalli approved these changes Dec 22, 2023

View reviewed changes

RobbeSneyders reviewed Dec 22, 2023

View reviewed changes

Shubham Krishna added 2 commits January 2, 2024 20:51

Add Unit tests mocking real artifacts

7916e76

Merge branch 'feature/index-aws-opensearch' of https://github.com/shu…

c5eeac0

…b-kris/fondant into feature/index-aws-opensearch

shub-kris requested a review from RobbeSneyders January 2, 2024 19:58

RobbeSneyders approved these changes Jan 3, 2024

View reviewed changes

RobbeSneyders merged commit ea8c6f4 into ml6team:main Jan 3, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add component to index aws opensearch #740

Add component to index aws opensearch #740

shub-kris commented Dec 21, 2023

mrchtr left a comment

mrchtr Dec 21, 2023

shub-kris Dec 21, 2023 •

edited

Loading

shub-kris Dec 21, 2023

PhilippeMoussalli Dec 22, 2023

shub-kris Dec 22, 2023 •

edited

Loading

PhilippeMoussalli Dec 22, 2023

shub-kris Dec 22, 2023

mrchtr Dec 21, 2023

shub-kris Dec 21, 2023

shub-kris Dec 21, 2023 •

edited

Loading

shub-kris Dec 21, 2023

PhilippeMoussalli Dec 21, 2023

PhilippeMoussalli Dec 21, 2023

PhilippeMoussalli Dec 21, 2023

PhilippeMoussalli Dec 21, 2023

RobbeSneyders commented Dec 21, 2023 •

edited

Loading

PhilippeMoussalli left a comment

RobbeSneyders Dec 22, 2023

RobbeSneyders Dec 22, 2023

shub-kris Dec 22, 2023

RobbeSneyders Jan 2, 2024

shub-kris Jan 2, 2024

shub-kris Jan 3, 2024

RobbeSneyders left a comment

	index_body (Dict[str, Any]): Parameters that specify index settings,
	index_body: Parameters that specify index settings,

	dataframe (dd.DataFrame): The Dask DataFrame containing the data to be written.
	dataframe: The Dask DataFrame containing the data to be written.

-        for part in dataframe.partitions:
+            for part in tqdm(
+                dataframe.partitions,
+                desc="Processing partitions",
+                total=dataframe.npartitions,
+            ):

Add component to index aws opensearch #740

Add component to index aws opensearch #740

Conversation

shub-kris commented Dec 21, 2023

mrchtr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shub-kris Dec 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shub-kris Dec 22, 2023 • 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

shub-kris Dec 21, 2023 • 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

Choose a reason for hiding this comment

RobbeSneyders commented Dec 21, 2023 • edited Loading

PhilippeMoussalli 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

RobbeSneyders left a comment

Choose a reason for hiding this comment

shub-kris Dec 21, 2023 •

edited

Loading

shub-kris Dec 22, 2023 •

edited

Loading

shub-kris Dec 21, 2023 •

edited

Loading

RobbeSneyders commented Dec 21, 2023 •

edited

Loading