-
Notifications
You must be signed in to change notification settings - Fork 436
Implement get server info #84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
import xml.etree.ElementTree as ET | ||
from .. import NAMESPACE | ||
|
||
|
||
class ServerInfoItem(object): | ||
def __init__(self, product_version, build_number, rest_api_version): | ||
self._product_version = product_version | ||
self._build_number = build_number | ||
self._rest_api_version = rest_api_version | ||
|
||
@property | ||
def product_version(self): | ||
return self._product_version | ||
|
||
@property | ||
def build_number(self): | ||
return self._build_number | ||
|
||
@property | ||
def rest_api_version(self): | ||
return self._rest_api_version | ||
|
||
@classmethod | ||
def from_response(cls, resp): | ||
parsed_response = ET.fromstring(resp) | ||
product_version_tag = parsed_response.find('.//t:productVersion', namespaces=NAMESPACE) | ||
rest_api_version_tag = parsed_response.find('.//t:restApiVersion', namespaces=NAMESPACE) | ||
|
||
build_number = product_version_tag.get('build', None) | ||
product_version = product_version_tag.text | ||
rest_api_version = rest_api_version_tag.text | ||
|
||
return cls(product_version, build_number, rest_api_version) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
from .exceptions import ServerResponseError | ||
import logging | ||
|
||
|
||
logger = logging.getLogger('tableau.endpoint') | ||
|
||
Success_codes = [200, 201, 204] | ||
|
@@ -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): | ||
if request_object is not None: | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
logger.debug(u'Server response from {0}:\n\t{1}'.format( | ||
url, server_response.content.decode(server_response.encoding))) | ||
return server_response | ||
|
||
def get_request(self, url, request_object=None): | ||
if request_object is not None: | ||
url = request_object.apply_query_params(url) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
from .endpoint import Endpoint | ||
from ...models import ServerInfoItem | ||
import logging | ||
|
||
logger = logging.getLogger('tableau.endpoint.server_info') | ||
|
||
|
||
class ServerInfo(Endpoint): | ||
def __init__(self, parent_srv): | ||
super(Endpoint, self).__init__() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
self.parent_srv = parent_srv | ||
|
||
@property | ||
def baseurl(self): | ||
return "{0}/serverInfo".format(self.parent_srv.baseurl) | ||
|
||
def get(self): | ||
""" Retrieve the server info for the server. This is an unauthenticated call """ | ||
server_response = self.get_unauthenticated_request(self.baseurl) | ||
server_info = ServerInfoItem.from_response(server_response.content) | ||
return server_info |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
<tsResponse xmlns="http://tableau.com/api" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://tableau.com/api http://tableau.com/api/ts-api-2.4.xsd"> | ||
<serverInfo> | ||
<productVersion build="10100.16.1024.2100">10.1.0</productVersion> | ||
<restApiVersion>2.4</restApiVersion> | ||
</serverInfo> | ||
</tsResponse> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
import unittest | ||
import os.path | ||
import requests_mock | ||
import tableauserverclient as TSC | ||
|
||
TEST_ASSET_DIR = os.path.join(os.path.dirname(__file__), 'assets') | ||
|
||
SERVER_INFO_GET_XML = os.path.join(TEST_ASSET_DIR, 'server_info_get.xml') | ||
|
||
|
||
class ServerInfoTests(unittest.TestCase): | ||
def setUp(self): | ||
self.server = TSC.Server('http://test') | ||
self.server.version = '2.4' | ||
self.baseurl = self.server.server_info.baseurl | ||
|
||
def test_server_info_get(self): | ||
with open(SERVER_INFO_GET_XML, 'rb') as f: | ||
response_xml = f.read().decode('utf-8') | ||
with requests_mock.mock() as m: | ||
m.get(self.baseurl, text=response_xml) | ||
actual = self.server.server_info.get() | ||
|
||
self.assertEqual('10.1.0', actual.product_version) | ||
self.assertEqual('10100.16.1024.2100', actual.build_number) | ||
self.assertEqual('2.4', actual.rest_api_version) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.