Nicole #2
Nicole #2
Conversation
There was a problem hiding this comment.
Hi @nsmina914!
Thanks for putting this together. Overall it is quite nice. We needed a demo notebook and I'm glad we have one. Also you are on the right track with a convenience search function.
I do have some comments noted below. How about we use our upcoming meeting to go over them?
escalateclient/escalateclient.py
Outdated
| @@ -54,6 +54,173 @@ def get(self, | |||
| return r.json()['results'] | |||
| print('Returning response object') | |||
| return r | |||
|
|
|||
| def search(self, | |||
There was a problem hiding this comment.
This method is a little long, and a quadruply nested if will likely be hard to maintain. @nsmina914 can you shorten it somehow? Either factor out reused logic or break into subroutines, each with a clear name and docstring? (The method should have a docstring as well)
escalateclient/escalateclient.py
Outdated
| @@ -89,6 +256,18 @@ def put(self, url=None, endpoint=None, resource_id=None, data=None): | |||
| url = f'{self.base_url}/api/{endpoint}/{resource_id}' | |||
| r = requests.api.put(url, data, headers=self._token_header) | |||
| return r | |||
|
|
|||
| def patch(self, url=None, endpoint=None, resource_id=None, data=None): | |||
There was a problem hiding this comment.
This looks good, thanks!
README.md
Outdated
| @@ -17,8 +17,11 @@ escalate=ESCALATEClient(escalate_url, | |||
|
|
|||
| escalate.list_endpoints() | |||
|
|
|||
| escalate.search(endpoint, search_field, criteria, data=[], exact=False, negate=False) | |||
There was a problem hiding this comment.
The rest of the methods are obvious, but its not clear from this what search does. We should go through this file together and add some more comments and concrete examples
| @@ -0,0 +1,815 @@ | |||
| { | |||
There was a problem hiding this comment.
This notebook looks good. I have some suggestions to clean it up a bit
No description provided.