Skip to content

Implement call to move to highest supported REST API version #100

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.

Sign up for GitHub

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

Merged
t8y8 merged 1 commit into from
Jan 5, 2017
Merged

Implement call to move to highest supported REST API version #100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tableauserverclient/server/endpoint/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from .auth_endpoint import Auth
from .datasources_endpoint import Datasources
from .endpoint import Endpoint
from .exceptions import ServerResponseError, MissingRequiredFieldError
from .exceptions import ServerResponseError, MissingRequiredFieldError, ServerInfoEndpointNotFoundError
from .groups_endpoint import Groups
from .projects_endpoint import Projects
from .schedules_endpoint import Schedules
Expand Down
4 changes: 4 additions & 0 deletions tableauserverclient/server/endpoint/exceptions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,7 @@ def from_response(cls, resp):

class MissingRequiredFieldError(Exception):
pass


class ServerInfoEndpointNotFoundError(Exception):
pass
8 changes: 7 additions & 1 deletion tableauserverclient/server/endpoint/server_info_endpoint.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from .endpoint import Endpoint
from .exceptions import ServerResponseError, ServerInfoEndpointNotFoundError
from ...models import ServerInfoItem
import logging

Expand All @@ -12,6 +13,11 @@ def baseurl(self):

def get(self):
""" Retrieve the server info for the server. This is an unauthenticated call """
server_response = self.get_unauthenticated_request(self.baseurl)
try:
server_response = self.get_unauthenticated_request(self.baseurl)
except ServerResponseError as e:
if e.code == "404003":
raise ServerInfoEndpointNotFoundError

server_info = ServerInfoItem.from_response(server_response.content)
return server_info
36 changes: 35 additions & 1 deletion tableauserverclient/server/server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,19 @@
import xml.etree.ElementTree as ET

from .exceptions import NotSignedInError
from .endpoint import Sites, Views, Users, Groups, Workbooks, Datasources, Projects, Auth, Schedules, ServerInfo
from .endpoint import Sites, Views, Users, Groups, Workbooks, Datasources, Projects, Auth, \
Schedules, ServerInfo, ServerInfoEndpointNotFoundError

import requests

_PRODUCT_TO_REST_VERSION = {
'10.0': '2.3',
'9.3': '2.2',
'9.2': '2.1',
'9.1': '2.0',
'9.0': '2.0'
}


class Server(object):
class PublishMode:
Expand Down Expand Up @@ -47,6 +58,29 @@ def _set_auth(self, site_id, user_id, auth_token):
self._user_id = user_id
self._auth_token = auth_token

def _get_legacy_version(self):
response = self._session.get(self.server_address + "/auth?format=xml")
info_xml = ET.fromstring(response.content)
prod_version = info_xml.find('.//product_version').text
version = _PRODUCT_TO_REST_VERSION.get(prod_version, '2.1') # 2.1
Copy link
Collaborator Author

@t8y8 t8y8 Nov 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll make that comment not a random repetition: 2.1 is the version with permissions changes

Copy link
Contributor

@LGraber LGraber Nov 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I am not sure I would include this code. The implication of adding this is that we actually test against some of these older versions, which we don't. While believe we should work with most of it, my inclination would be that if the user asks us to pick the latest version, and we can't find it (ie there is no serverInfo endpoint), we set it to 9.3. I am open to discuss, though. I am sure it is helpful, however we do need to be clear about what we actually test against.

Copy link
Collaborator Author

@t8y8 t8y8 Nov 13, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could just default to 9.3's REST API version -- this was recommended by @RussTheAerialist since I think he plans to decorate versions back farther than 9.3.

I'm fine either way

return version

def _determine_highest_version(self):
try:
old_version = self.version
self.version = "2.4"
version = self.server_info.get().rest_api_version
except ServerInfoEndpointNotFoundError:
version = self._get_legacy_version()

finally:
Copy link
Contributor

@graysonarts graysonarts Nov 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a bit weird. We are setting the version, determining the new version, and then resetting the old version. I think with the way this is written, we probably don't need _determine_highest_version to be it's own function. In this case, we could do something like the following pseudo-code

save old version
set version to 2.4
Try to call get rest api version
If exception, try to get legacy version
If no legacy version found, restore old value.

I'm not sure which version would be clearer to understand.

Copy link
Collaborator Author

@t8y8 t8y8 Nov 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I currently have it this way so that this function simply retrieves the highest supported version, and the caller is responsible for setting it wherever it wants to be set -- this lets us reuse the function elsewhere (I don't know a use case yet, but I don't think the premature refactor makes it harder to follow).

I'd prefer the separation, if there's a strong objection let me know and I can combine it.

I'm pro-flag that does this automatically!!! I'll let Lee weigh in and can respond accordingly

Copy link
Contributor

@graysonarts graysonarts Nov 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not have a strong opinion either way, like I said, I wasn't sure which direction was clearer, and you make a good point about reuse.

Copy link

@divyavanmahajan divyavanmahajan Nov 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer the API is set to the highest level (since that is usually what most people expect). If someone wants to fix the version - they can set the version after sign in.

Copy link
Contributor

@LGraber LGraber Nov 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you think if you put this in the use_highest_version() function it could not be used elsewhere? Just because it is a public function doesn't mean you can't call it from other parts of the code. Seems like we don't need two functions.

Copy link
Collaborator Author

@t8y8 t8y8 Nov 13, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think use_highest_version should do just that, find and set the highest version. get highest version, or do the best guess is something different, and I think it's cleaner to leave the setting to the caller (use_highest_version) and the internal machinery of how it finds that version are behind a different function and could change.

As a side benefit I think it keeps the little bits with the try/excepts much easier to follow to a new reader of the code, vs jamming it all into one function that also sets it on the Server object too

self.version = old_version

return version

def use_highest_version(self):
self.version = self._determine_highest_version()

@property
def baseurl(self):
return "{0}/api/{1}".format(self._server_address, str(self.version))
Expand Down
7 changes: 7 additions & 0 deletions test/assets/server_info_404.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<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.3.xsd">
<error code="404003">
<summary>Resource Not Found</summary>
<detail>Unknown resource '/2.4/serverInfo' specified in URI.</detail>
</error>
</tsResponse>
12 changes: 12 additions & 0 deletions test/assets/server_info_auth_info.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<authinfo>
<version version="0.31">
<api_version>0.31</api_version>
<server_api_version>0.31</server_api_version>
<document_version>9.2</document_version>
<product_version>9.3</product_version>
<external_version>9.3.4</external_version>
<build>hello.16.1106.2025</build>
<publishing_method>unrestricted</publishing_method>
<mobile_api_version>2.6</mobile_api_version>
</version>
</authinfo>
31 changes: 29 additions & 2 deletions test/test_server_info.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,48 @@
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')
SERVER_INFO_404 = os.path.join(TEST_ASSET_DIR, 'server_info_404.xml')
SERVER_INFO_AUTH_INFO_XML = os.path.join(TEST_ASSET_DIR, 'server_info_auth_info.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)
self.server.version = '2.4'
m.get(self.server.server_info.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)

def test_server_info_use_highest_version_downgrades(self):
with open(SERVER_INFO_AUTH_INFO_XML, 'rb') as f:
# This is the auth.xml endpoint present back to 9.0 Servers
auth_response_xml = f.read().decode('utf-8')
with open(SERVER_INFO_404, 'rb') as f:
# 10.1 serverInfo response
si_response_xml = f.read().decode('utf-8')
with requests_mock.mock() as m:
# Return a 404 for serverInfo so we can pretend this is an old Server
m.get(self.server.server_address + "/api/2.4/serverInfo", text=si_response_xml, status_code=404)
m.get(self.server.server_address + "/auth?format=xml", text=auth_response_xml)
self.server.use_highest_version()
self.assertEqual(self.server.version, '2.2')

def test_server_info_use_highest_version_upgrades(self):
with open(SERVER_INFO_GET_XML, 'rb') as f:
si_response_xml = f.read().decode('utf-8')
with requests_mock.mock() as m:
m.get(self.server.server_address + "/api/2.4/serverInfo", text=si_response_xml)
# Pretend we're old
self.server.version = '2.0'
self.server.use_highest_version()
# Did we upgrade to 2.4?
self.assertEqual(self.server.version, '2.4')