Implement Couchbase Collection Datanode and Add Unit Tests #2010

snehaamujri · 2024-10-10T13:50:00Z

Added to handle data interactions with Couchbase collections.
Introduced for unit testing the functionality of the Couchbase Collection Datanode.
Ensured the implementation aligns with existing data node standards and includes necessary validation.

- Added to handle data interactions with Couchbase collections. - Introduced for unit testing the functionality of the Couchbase Collection Datanode. - Ensured the implementation aligns with existing data node standards and includes necessary validation.

jrobinAV

That's promising. Good starting point. I believe the overall idea is correct.

Now we need to go into the real implementation. There are some missing points. For example:

what kind of documents are accepted? Do we need to define a Python object to structure the document?
We are missing the _write, _append, and filter methods implementations
For testing, we should have a Mock class acting like a CouchBase server without having to instantiate a real server
etc...

jrobinAV · 2024-10-10T14:40:58Z

taipy/core/data/couchbase_collection_datanode.py

+    def _read(self):
+        """Read all documents from the Couchbase collection."""
+        try:
+            return [doc.content_as[dict] for doc in documents]


What is documents here?

taipy/core/data/couchbase_collection_datanode.py

jrobinAV · 2024-10-10T14:44:09Z

taipy/core/data/couchbase_collection_datanode.py

+            return [doc.content_as[dict] for doc in documents]
+        except Exception as e:
+             print(f"An error occurred: {e}")
+             return []


TODO: We have to handle this case.

Thank you for the review and the detailed feedback! I appreciate the direction, and I'm eager to work on the improvements.

-> For document structure, I will define a Python class or object to structure the documents, ensuring consistency in what
is accepted.
-> I will proceed with implementing the _write, _append, and filter methods. I understand these are essential for CRUD
operations.
-> For testing, I will create a Mock class to simulate Couchbase interactions, allowing us to test without needing an
actual Couchbase server.

…lass for testing, and updated documentation.

…lass for testing, updated documentation, and included all required features.

snehaamujri · 2024-10-11T06:04:51Z

Hey @jrobinAV

Resolved outstanding issues in couchbase_collection_datanode.py:

Created Couchbase connection in the try block using provided properties
Implemented _write, _append, and filter methods
Added mock class in mock_couchbase.py for unit testing without real server
Updated Couchbase API documentation reference
Enhanced the document structure and Python object handling for clarity
Provided additional documentation for new methods and classes

trgiangdo

Hello @snehaamujri, thank you for your contribution.

I have some questions about the implementation.

trgiangdo · 2024-10-11T08:18:35Z

taipy/core/data/couchbase_collection_datanode.py

+                        properties.get(self.__DB_USERNAME_KEY, ""),
+                        properties.get(self.__DB_PASSWORD_KEY, "")


I'm quite sure that Couchbase requires username and password to connect.

If the properties doesn't contain the username and password, we should raise an error before this, meaning that we should add the self.__DB_USERNAME_KEY and self.__DB_PASSWORD_KEY to the self._REQUIRED_PROPERTIES

trgiangdo · 2024-10-11T08:19:06Z

taipy/core/data/couchbase_collection_datanode.py

+                    )
+                )
+            )
+            bucket = self.cluster.bucket(properties.get(self.__DB_NAME_KEY))


Suggested change

bucket = self.cluster.bucket(properties.get(self.__DB_NAME_KEY))

bucket = self.cluster.bucket(properties.get(self.__BUCKET_KEY))

The constant should be renamed to match what it actually presents

trgiangdo · 2024-10-11T08:34:05Z

taipy/core/data/couchbase_collection_datanode.py

+        try:
+            if isinstance(data, dict):
+                self.collection.upsert(data['id'],data)
+            elif isinstance(data, list):
+                for item in data:
+                   self.collection.upsert(item['id'], item)
+       except CouchbaseException as e:
+            print(f"An error occurred while writing:{e}")


Suggested change

try:

if isinstance(data, dict):

self.collection.upsert(data['id'],data)

elif isinstance(data, list):

for item in data:

self.collection.upsert(item['id'], item)

except CouchbaseException as e:

print(f"An error occurred while writing:{e}")

try:

if isinstance(data, dict):

self.collection.upsert(data['id'],data)

elif isinstance(data, list):

for item in data:

self.collection.upsert(item['id'], item)

except CouchbaseException as e:

print(f"An error occurred while writing:{e}")

The except is misalign

trgiangdo · 2024-10-11T08:39:30Z

taipy/core/data/couchbase_collection_datanode.py

+class CouchbaseDocument:
+    """Class to define the structure of documents stored in Couchbase."""
+
+    def __init__(self, field1: str, field2: int, **kwargs):
+        self.field1 = field1  # Example field of type string
+        self.field2 = field2  # Example field of type integer
+        # Additional fields can be added dynamically
+        for key, value in kwargs.items():
+            setattr(self, key, value)


Where is this class being used?
What is it used for?

trgiangdo · 2024-10-11T08:40:01Z

taipy/core/data/couchbase_collection_datanode.py

+                self.collection.upsert(data['id'],data)
+            elif isinstance(data, list):
+                for item in data:
+                   self.collection.upsert(item['id'], item)


What is the motivation for using upsert here?
What should happens if the document doesn't have an id?

trgiangdo · 2024-10-11T08:42:13Z

tests/mocks/mock_couchbase.py

+from couchbase_collection_datanode import CouchBaseCollectionDataNode  # Make sure this import is correct
+
+
+class TestCouchBaseCollectionDataNode(unittest.TestCase):


Can we add a test_read to this class?

trgiangdo · 2024-10-11T08:43:03Z

tests/core/data/mock_couchbase.py

+class MockCouchbaseCollection:
+    def __init__(self):
+        self.documents = {}
+
+    def insert(self, doc_id, document):
+        if doc_id in self.documents:
+            raise Exception(f"Document with ID {doc_id} already exists.")
+        self.documents[doc_id] = document
+
+    def upsert(self, doc_id, document):
+        self.documents[doc_id] = document
+
+    def get(self, doc_id):
+        if doc_id not in self.documents:
+            raise Exception(f"Document with ID {doc_id} not found.")
+        return self.documents[doc_id]
+
+    def remove(self, doc_id):
+        if doc_id not in self.documents:
+            raise Exception(f"Document with ID {doc_id} not found.")
+        del self.documents[doc_id]


As my understanding, the MockCouchbaseCollection is essentially a dictionary right?

trgiangdo · 2024-10-11T08:45:46Z

tests/mocks/mock_couchbase.py

+    def test_append_document(self):
+        doc_id = "doc2"
+        document = {"name": "Jane", "hobbies": []}
+        self.data_node._write(doc_id, document)  # Assuming _write is implemented correctly


Does this work?

I'm asking because the signature of the CouchBaseCollectionDataNode._write() method only requires 1 argument.
Here you are providing 2.

trgiangdo · 2024-10-11T08:46:12Z

tests/mocks/mock_couchbase.py

+        self.data_node._write(doc_id, document)  # Assuming _write is implemented correctly
+
+        additional_data = ["reading", "hiking"]
+        self.data_node._append(doc_id, additional_data)  # Assuming _append is implemented correctly


Similarly, does _append() with 2 arguments work here?

jrobinAV · 2024-10-19T16:01:02Z

@snehaamujri Your first version is promising but not completed yet. Have you had a chance to see our reviews? Do you have any questions?

snehaamujri · 2024-10-19T16:30:52Z

Hey @jrobinAV,
Thank you for your feedback! Yes, I have reviewed the comments, and I’m working on addressing the issues raised. I would appreciate any additional insights you have as I make the necessary changes. Please let me know if there are specific areas you’d like to collaborate on and i need help and if you have suggestions for improvements.

github-actions · 2024-11-12T09:06:02Z

This PR has been labelled as "🥶Waiting for contributor" because it has been inactive for more than 14 days. If you would like to continue working on this PR, then please add new commit or another comment, otherwise this PR will be closed in 14 days. For more information please refer to the contributing guidelines.

jrobinAV · 2024-11-12T09:47:40Z

@snehaamujri
Is there any news about resolving the comments we made on this PR?

snehaamujri and others added 2 commits October 10, 2024 19:11

Merge branch 'develop' into couchbase-datanode

147fda2

jrobinAV reviewed Oct 10, 2024

View reviewed changes

snehaamujri and others added 3 commits October 11, 2024 07:50

Merge branch 'develop' into couchbase-datanode

a8ddb36

Implemented changes in couchbase_collection_datanode.py, added mock c…

fa7604d

…lass for testing, and updated documentation.

Implemented changes in couchbase_collection_datanode.py, added mock c…

7e1b8ca

…lass for testing, updated documentation, and included all required features.

snehaamujri force-pushed the couchbase-datanode branch from aa9dc40 to 7e1b8ca Compare October 11, 2024 06:01

trgiangdo reviewed Oct 11, 2024

View reviewed changes

jrobinAV added Core Related to Taipy Core 🟨 Priority: Medium Not blocking but should be addressed ✨New feature Core: ⚙️ Configuration Core: Data node labels Oct 19, 2024

jrobinAV added hacktoberfest hacktoberfest issues hacktoberfest - 100💎 Issues rewarded by 100 points labels Oct 19, 2024

jrobinAV self-assigned this Oct 19, 2024

Merge branch 'develop' into couchbase-datanode

cf48597

github-actions bot added the 🥶Waiting for contributor Issues or PRs waiting for a long time label Nov 12, 2024

github-actions bot removed the 🥶Waiting for contributor Issues or PRs waiting for a long time label Nov 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Couchbase Collection Datanode and Add Unit Tests #2010

Implement Couchbase Collection Datanode and Add Unit Tests #2010

snehaamujri commented Oct 10, 2024 •

edited by jrobinAV

Loading

jrobinAV left a comment

jrobinAV Oct 10, 2024

jrobinAV Oct 10, 2024

snehaamujri Oct 11, 2024

snehaamujri commented Oct 11, 2024

trgiangdo left a comment

trgiangdo Oct 11, 2024

trgiangdo Oct 11, 2024

trgiangdo Oct 11, 2024

trgiangdo Oct 11, 2024

trgiangdo Oct 11, 2024

trgiangdo Oct 11, 2024

trgiangdo Oct 11, 2024

trgiangdo Oct 11, 2024

trgiangdo Oct 11, 2024

jrobinAV commented Oct 19, 2024

snehaamujri commented Oct 19, 2024

github-actions bot commented Nov 12, 2024

jrobinAV commented Nov 12, 2024

		properties.get(self.__DB_USERNAME_KEY, ""),
		properties.get(self.__DB_PASSWORD_KEY, "")

	bucket = self.cluster.bucket(properties.get(self.__DB_NAME_KEY))
	bucket = self.cluster.bucket(properties.get(self.__BUCKET_KEY))

		from couchbase_collection_datanode import CouchBaseCollectionDataNode # Make sure this import is correct


		class TestCouchBaseCollectionDataNode(unittest.TestCase):

Implement Couchbase Collection Datanode and Add Unit Tests #2010

Are you sure you want to change the base?

Implement Couchbase Collection Datanode and Add Unit Tests #2010

Conversation

snehaamujri commented Oct 10, 2024 • edited by jrobinAV Loading

jrobinAV 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

snehaamujri commented Oct 11, 2024

trgiangdo 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

jrobinAV commented Oct 19, 2024

snehaamujri commented Oct 19, 2024

github-actions bot commented Nov 12, 2024

jrobinAV commented Nov 12, 2024

snehaamujri commented Oct 10, 2024 •

edited by jrobinAV

Loading