Observed server issue

[ke-zhang-rd]

I observed those issues and expect debug on server side summary

1. put_property method get a empty response now. However, client expect a dict like tags response

In [1]: from olog import Client                                                                                                                

In [2]: cli = Client(<URL>, <USER>, <PASSWORD>)                                                                          

In [3]: t = {'name': 'TEST7', 'state': 'Active'}                                                                                               

In [4]: cli.put_tag(t)                                                                                                                         
Out[4]: <Response [200 OK]>

In [5]: res =cli.put_tag(t)                                                                                                                    

In [6]: type(res)                                                                                                                              
Out[6]: httpx._models.Response

In [7]: res.content                                                                                                                            
Out[7]: b'{"name":"TEST7","state":"Active"}'

In [8]: PROPERTY = {'name': 'Ticket', 
   ...:             'owner': 'olog-logs', 
   ...:             'state': 'Active', 
   ...:             'attributes': [{'name': 'url', 'value': None, 'state': 'Active'}, 
   ...:                            {'name': 'id', 'value': None, 'state': 'Active'}]}                                                          

In [9]: cli.put_property(PROPERTY)                                                                                                             
Out[9]: <Response [200 OK]>

In [10]: res = cli.put_property(PROPERTY)                                                                                                      

In [11]: res.content                                                                                                                           
Out[11]: b''

In [12]:  

2. Same issue of put_logbook.

3. get_logs() doesn't handle start and end correctly

In [18]: cli.get_logs(start=1587674374209)                                                                                                                                      
{'start': 1587674374209}
---------------------------------------------------------------------------
HTTPError                                 Traceback (most recent call last)
<ipython-input-18-72f30616acbc> in <module>
----> 1 cli.get_logs(start=1587674374209)
~/pyolog2/olog.py in get_logs(self, **params)
     66 
     67     def get_logs(self, **params):
---> 68         return asyncio.run(self.aget_logs(**params))
     69 
     70     async def aget_log(self, id):
~/conda_envs/olog/lib/python3.8/asyncio/runners.py in run(main, debug)
     41         events.set_event_loop(loop)
     42         loop.set_debug(debug)
---> 43         return loop.run_until_complete(main)
     44     finally:
     45         try:
~/conda_envs/olog/lib/python3.8/asyncio/base_events.py in run_until_complete(self, future)
    614             raise RuntimeError('Event loop stopped before Future completed.')
    615 
--> 616         return future.result()
    617 
    618     def stop(self):
~/pyolog2/olog.py in aget_logs(self, **params)
     62         async with self._session as api:
     63             res = await api.get('logs', params=params)
---> 64         res.raise_for_status()
     65         return res.json()
     66 
~/conda_envs/olog/lib/python3.8/site-packages/httpx/_models.py in raise_for_status(self)
    844         elif StatusCode.is_server_error(self.status_code):
    845             message = message.format(self, error_type="Server Error")
--> 846             raise HTTPError(message, response=self)
    847 
    848     def json(self, **kwargs: typing.Any) -> typing.Union[dict, list]:
HTTPError: 500 Server Error: Internal Server Error for url: http://proxy:8888
For more information check: https://httpstatuses.com/500

4. unexpect keys in get_logs params , e.g. `get_logs(foo='bar') should return None or empty list not all logs.

[ke-zhang-rd]

@shroffk

[danielballan]

I am not sure (1) and (2) are bugs. I'm surprised that the server echos the tag content back at us in the response to PUT /tags. What does that achieve? It seems a 200 status code would be a sufficient acknowledgement of success---no need to pay for echoing the data back.

Likewise, in our Python client, if the user gives us a tag or a logbook in a function call, there's no reason to return it if the return value is always identical to the input value. All we need to do it either raise (if the server gives us a bad status code) or return None. Let's ignore the response.content for PUT /tags or---for extra paranoia---just verify that it equals the value that we submitted in our requests before returning.

(3) This looks like a server bug. No request should be able to trigger a 500. This should probably be a 400 Bad Request. Is there no documentation of what the expected parameters are?

(4) I think this should also return 400 Bad Request. The server should be very clear about the difference between "No results" and "Invalid request" because on our side the first should return in [] and the second should raise an exception.

[shroffk]

Hi

For 1 & 2, I decided to return the created resource so the client can verified that the created resource was in fact what they wanted to create...I can see the argument that 200 is clear enough.

I am fine with either returning only 200 or returning the created resource. What would you guys prefer?

[shroffk]

3. It was requested that I support the following format, I will update the documentation. "yyyy-MM-dd HH:mm:ss.SSS" Does this work for you?

[danielballan]

@shroffk For (1) and (2) I'm happy either way. We can always ignore the response or use it to perform that extra validation. I hope my comments above weren't too strident ("What does that achieve?") --- I just meant to be thinking it through out loud.

@ke-zhang-rd Regardless, I think we should return None at our level because we can do the validation internally, where applicable, and the caller can trust that if no exception was raised everything worked as expected.

For (3) that sounds good to me. @ke-zhang-rd Does it work if we submit requests in that format?

Is (4) actionable on your side, @shroffk?

[shroffk]

Ok, I will return the created resources

above weren't too strident

no worries :)...I have been part of API designs that I did not even register this. I should be able to explain why something is done a particular way and you are free to ask for that.

4. This is on me for the time being. I will be looking into this.

[ke-zhang-rd]

@shroffk For 3) cli.get_logs(start="2015-01-01 00:00:00.000") this doesn't work. It return 500

[shroffk]

@shroffk For 3) cli.get_logs(start="2015-01-01 00:00:00.000") this doesn't work. It return 500

This looks correct to me...

Can you check if the http request library is encoding this url path parameter correctly, can you send me a copy of the complete request.

In the server logs the service receives and empty string for 'end'.