Implement get server info #84
Conversation
| url = request_object.apply_query_params(url) | ||
| server_response = self.parent_srv.session.get(url, **self.parent_srv.http_options) | ||
| self._check_status(server_response) | ||
| if server_response.encoding: |
There was a problem hiding this comment.
Question -- why check for encoding? Will it ever not be there?
Does this mean if there isn't an encoding there is nothing to log?
(I know the other get does the same thing, just curious)
There was a problem hiding this comment.
I don't remember why we needed to do this, but it's the pattern that all of the others follow.
If we cannot encode the response into a string, then we do not log anything.
| @@ -15,6 +16,16 @@ def _check_status(server_response): | |||
| if server_response.status_code not in Success_codes: | |||
| raise ServerResponseError.from_response(server_response.content) | |||
|
|
|||
| def get_unauthenticated_request(self, url, request_object=None): | |||
There was a problem hiding this comment.
Feels like we should probably have a "_get_request" method and these two just call the underlying one. Unauthenticated can call directly. Normal can create the headers list and pass it in. The rest of the code is identical.
There was a problem hiding this comment.
I was planning to do that in a separate checkin anyway. There is a bunch of overlap that I think I can reduce, but it would have been noise in this change.
|
|
||
| class ServerInfo(Endpoint): | ||
| def __init__(self, parent_srv): | ||
| super(Endpoint, self).__init__() |
There was a problem hiding this comment.
We need to go through and update all of these. This is not the correct way to do it. "Endpoint" should be "ServerInfo". It doesn't matter since Endpoint and object both have empty constrctors. Which leaves us with ... we should just delete all of these.
There was a problem hiding this comment.
This was another thing that I noticed while doing this change. EVERY Endpoint takes parent_srv, so I'm going to rip all of the custom ctors out and just allow the base object's ctor which doesn't currently take a parent_srv take the parent_srv
|
Couple of comments for cleanup later. You already submitted so they can be dealt with the next time |
|
@LGraber I think I noticed another weird thing (unrelated to his change), I might be able to incorporate your feedback into that |
* Create CONTRIBUTORS.md * Adding Charley Peng who contributed doc fixes * Update CONTRIBUTORS.md
* Create CONTRIBUTORS.md * Adding Charley Peng who contributed doc fixes * Update CONTRIBUTORS.md
Quickly implemented serverInfo endpoint.