Skip to content

Implement view filters on the populate* request options #260

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
graysonarts merged 6 commits into from
Jan 31, 2018
Merged

Implement view filters on the populate* request options #260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c0e9243
Implement view filters on the populate* request options
graysonarts Jan 30, 2018
0bb7f9e
pep8 fix
graysonarts Jan 30, 2018
24313e8
Fix stream=true being passed to requests
graysonarts Jan 31, 2018
0e0622b
Fixing typo
graysonarts Jan 31, 2018
098a857
Addressing Tyler's Feedback
graysonarts Jan 31, 2018
a01dd4b
typo
graysonarts Jan 31, 2018
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/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
SiteItem, TableauAuth, UserItem, ViewItem, WorkbookItem, UnpopulatedPropertyError, \
HourlyInterval, DailyInterval, WeeklyInterval, MonthlyInterval, IntervalItem, TaskItem, \
SubscriptionItem
from .server import RequestOptions, ImageRequestOptions, PDFRequestOptions, Filter, Sort, \
from .server import RequestOptions, CSVRequestOptions, ImageRequestOptions, PDFRequestOptions, Filter, Sort, \
Server, ServerResponseError, MissingRequiredFieldError, NotSignedInError, Pager
from ._version import get_versions
__version__ = get_versions()['version']
Expand Down
2 changes: 1 addition & 1 deletion tableauserverclient/server/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from .request_factory import RequestFactory
from .request_options import ImageRequestOptions, PDFRequestOptions, RequestOptions
from .request_options import CSVRequestOptions, ImageRequestOptions, PDFRequestOptions, RequestOptions
from .filter import Filter
from .sort import Sort
from .. import ConnectionItem, DatasourceItem, JobItem, \
Expand Down
2 changes: 1 addition & 1 deletion tableauserverclient/server/endpoint/views_endpoint.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ def csv_fetcher():
def _get_view_csv(self, view_item, req_options):
url = "{0}/{1}/data".format(self.baseurl, view_item.id)

with closing(self.get_request(url, parameters={"stream": True})) as server_response:
Copy link
Copy Markdown
Collaborator

@t8y8 t8y8 Jan 31, 2018

Choose a reason for hiding this comment

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

I don't think this is right, the 'stream=true' is an option for the requests library, not something that gets included in the url string, which is what apply_query_parameters does.

with closing(self.get_request(url, request_object=req_options, parameters={"stream": True})) as server_response:
csv = server_response.iter_content(1024)
return csv

Expand Down
35 changes: 32 additions & 3 deletions tableauserverclient/server/request_options.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,24 +62,50 @@ def apply_query_params(self, url):
return "{0}?{1}".format(url, '&'.join(params))


class ImageRequestOptions(RequestOptionsBase):
class _FilterOptionsBase(RequestOptionsBase):
Copy link
Copy Markdown
Collaborator

@t8y8 t8y8 Jan 31, 2018

Choose a reason for hiding this comment

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

Seems like a reasonable refactor.

Can you give this class a docstring just so at a glance we can tell what it's used for vs other optionsbases?

""" Provide a basic implementation of adding view filters to the url """
def __init__(self):
self.view_filters = []

def apply_query_params(self, url):
raise NotImplementedError()

def vf(self, name, value):
Copy link
Copy Markdown
Collaborator

@t8y8 t8y8 Jan 31, 2018

Choose a reason for hiding this comment

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

There's no code testing this or sampling this yet, but I'm guessing this makes it:

opts = ImageRequestOptions()
opts.vf('a', 1).vf('b', 2) etc?

Baby-builder pattern!
Did you still want to refactor this all to eventually just be builder-y and call str on those? (referenced in #212)

Copy link
Copy Markdown
Contributor Author

@graysonarts graysonarts Jan 31, 2018

Choose a reason for hiding this comment

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

Yes, eventually, we will refactor to a builder pattern!

self.view_filters.append((name, value))
return self

def _append_view_filters(self, params):
for name, value in self.view_filters:
params.append('vf_{}={}'.format(name, value))


class CSVRequestOptions(_FilterOptionsBase):
def apply_query_params(self, url):
params = []
self._append_view_filters(params)
return "{0}?{1}".format(url, '&'.join(params))


class ImageRequestOptions(_FilterOptionsBase):
# if 'high' isn't specified, the REST API endpoint returns an image with standard resolution
class Resolution:
High = 'high'

def __init__(self, imageresolution=None):
Copy link
Copy Markdown
Collaborator

@t8y8 t8y8 Jan 31, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Side note: I thought I made this so it was image_resolution... maybe I never checked that fix in?

I might make a PR after this merges

super(ImageRequestOptions, self).__init__()
self.image_resolution = imageresolution

def apply_query_params(self, url):
params = []
if self.image_resolution:
params.append('resolution={0}'.format(self.image_resolution))

self._append_view_filters(params)

return "{0}?{1}".format(url, '&'.join(params))


class PDFRequestOptions(RequestOptionsBase):
# if 'high' isn't specified, the REST API endpoint returns an image with standard resolution
class PDFRequestOptions(_FilterOptionsBase):
class PageType:
A3 = "a3"
A4 = "a4"
Expand All @@ -100,6 +126,7 @@ class Orientation:
Landscape = "landscape"

def __init__(self, page_type=None, orientation=None):
super(PDFRequestOptions, self).__init__()
self.page_type = page_type
self.orientation = orientation

Expand All @@ -111,4 +138,6 @@ def apply_query_params(self, url):
if self.orientation:
params.append('orientation={0}'.format(self.orientation))

self._append_view_filters(params)

return "{0}?{1}".format(url, '&'.join(params))