chore: add method documentation #15

poppoerika · 2022-01-11T22:55:20Z

Closes #4

gautamomento

Thank you for doing this.

I have added some comments.

gautamomento · 2022-01-11T23:33:46Z

src/momento/cache.py


 class Cache:
    def __init__(self, auth_token, cache_name, endpoint, default_ttlSeconds):
+        """Inits Cache to perform gets and sets.


Inititalizes

gautamomento · 2022-01-11T23:36:21Z

src/momento/cache.py

+            auth_token: Momento JWT token.
+            cahce_name: String of cache name to perform gets and sets.
+            end_point: String of endpoint to reach Momento Cache
+            default_ttlSeconds: The default time to live of object inside of cache in seconds.


Time (in seconds) for which an item will be stored in the cache.

gautamomento · 2022-01-11T23:41:47Z

src/momento/cache.py

+
+        Args:
+            auth_token: Momento JWT token.
+            cahce_name: String of cache name to perform gets and sets.


Suggested change

cahce_name: String of cache name to perform gets and sets.

cache_name: Name of the cache

gautamomento · 2022-01-11T23:44:30Z

src/momento/cache.py

        self._secure_channel.close()

    def set(self, key, value, ttl_seconds=None):
+        """Set key/value pair in the cache.


Stores an item in cache

gautamomento · 2022-01-11T23:47:43Z

src/momento/cache.py

+            CacheSetResponse
+        
+        Raises:
+            Exception to notify either sdk, grpc, or operation error.


Let's try to be specific -

e.g.
Raises:
CacheNotFound - if an attempt is made to store an item in a cache that doesn't exist
PermissionDenied - if the provided Momento Auth Token is invalid to perform the requested operation
InternalServerError - if server encountered an unknown error while trying to store the item

You will have to look at the exact error names in the errors.py file.

gautamomento · 2022-01-11T23:55:26Z

src/momento/cache_operation_responses.py



    def str_utf8(self):
+        """Decodes string value got from cache to a utf-8 string."""


This should probably say:

Returns value stored in cache as utf-8 string if there was Hit. Returns None otherwise.

gautamomento · 2022-01-11T23:55:39Z

src/momento/cache_operation_responses.py

        return None

    def bytes(self):
+        """Returns byte value got from cache."""


Same as above.

gautamomento · 2022-01-11T23:56:16Z

src/momento/cache_operation_responses.py

+        """Inits ListCacheResponse to handle list cache response.
+
+        Args:
+            gprc_list_cache_response: Response returned from list operation.


See my note about mentioning protobuf from Scs.

gautamomento · 2022-01-11T23:57:09Z

src/momento/cache_operation_responses.py


 class CacheInfo:
    def __init__(self, grpc_listed_caches):
+        """Inits CacheInfo to handle caches returned from list cache operation.


This is information about a single cache. Sorry about bad naming here. The variable in init should be grpc_listed_cache. If you are okay with it, do you mind updating the variable name?

You probably have to update this entire object to say cache and not caches

gautamomento · 2022-01-12T00:00:26Z

src/momento/momento.py

        return cache._connect()

    def list_caches(self, next_token=None):
+        """Lists ll caches.


Probably ll -> all

gautamomento · 2022-01-12T00:01:34Z

src/momento/momento.py

+        """Lists ll caches.
+
+        Args: 
+            next_token: Token to continue paginating through the list. It's ised to hnadle large paginated lists.


Some typos here too.

gautamomento · 2022-01-12T00:43:55Z

src/momento/cache.py

+            ClientSdkError: For all errors raised by the client. Indicates that the request failed on the SDK. 
+                            The request either did not make it to the service or if it did the response from the service could not be parsed successfully.
+            InternalServerError: If server encountered an unknown error while trying to store the item.
+            SdkError: Base exception for all errors raised by Sdk.


This shouldn't be added here. It is the base error and is never thrown. Comments apply to all methods with raise.

gautamomento · 2022-01-12T00:44:29Z

src/momento/cache.py

+            CacheValueError: If service validation fails for provided values.
+            CacheNotFoundError: If an attempt is made to store an item in a cache that doesn't exist.
+            PermissionError: If the provided Momento Auth Token is invalid to perform the requested operation.
+            ClientSdkError: For all errors raised by the client. Indicates that the request failed on the SDK. 


Can you do a quick code check and see if this operation throws ClientSdkError? Comments apply to all methods with raise.

@gautamomento - Checked and for the methods in cache.py do not throw the ClientSdkError; however some methods in momento.py throws the ClientSdkError when anything other than string is passed as an argument. Updated those Raises section accordingly.

gautamomento · 2022-01-12T00:46:45Z

src/momento/cache_operation_responses.py

+            grpc_set_response: Protobuf based response returned by Scs.
+            value (string or bytes): The value to be used to store item in the cache
+
+        Raises:


I doubt if all these errors will be thrown. Can you verify? All I think will come on initialize is InternalServerError from call to error_converter.convert_ecache_result

gautamomento · 2022-01-12T00:47:13Z

src/momento/cache_operation_responses.py

+        Args:
+            grpc_get_response: Protobuf based response returned by Scs.
+
+        Raises:


Ditto: please verify if these errors will ever come from this method.

gautamomento · 2022-01-12T00:47:49Z

src/momento/cache_operation_responses.py

+        self._name = grpc_listed_cache.cache_name

    def name(self):
+        """Returns all caches' names."""


Just this cache's name. This is for a single cache.

gautamomento · 2022-01-12T00:48:12Z

src/momento/momento.py

+            CreateCacheResponse
+
+        Raises:
+            Exception to notify either sdk, grpc, or operation error.


Need to update this file.

gautamomento

LGTM.

poppoerika added 3 commits January 11, 2022 12:46

add docstrings to Momento class methods

04c0fae

add docstrings to Cache class methods

cbc1f80

add docstrings to cache operation responses' classes' methods

1d9a7a3

poppoerika requested review from bruuuuuuuce and gautamomento January 11, 2022 22:55

gautamomento reviewed Jan 12, 2022

View reviewed changes

fix typos and update docstrings

9265939

gautamomento reviewed Jan 12, 2022

View reviewed changes

poppoerika added 2 commits January 12, 2022 17:05

remove SdkError from Raises

4476ca8

update Raises section to match only possible errors thrown

0ce36a0

gautamomento approved these changes Jan 13, 2022

View reviewed changes

poppoerika merged commit e053811 into main Jan 13, 2022

poppoerika deleted the chore/addMethodDoc branch January 13, 2022 18:14

	cahce_name: String of cache name to perform gets and sets.
	cache_name: Name of the cache



		def str_utf8(self):
		"""Decodes string value got from cache to a utf-8 string."""

chore: add method documentation #15

chore: add method documentation #15

Uh oh!

Conversation

poppoerika commented Jan 11, 2022

Uh oh!

gautamomento left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gautamomento left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants