-
Notifications
You must be signed in to change notification settings - Fork 0
Schedule tasks #2
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
base: master
Are you sure you want to change the base?
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 |
|---|---|---|
| @@ -1,2 +1,5 @@ | ||
| class UnpopulatedPropertyError(Exception): | ||
| pass | ||
|
|
||
| class ResponseBodyError(Exception): | ||
| pass |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,93 @@ | ||||||
| import xml.etree.ElementTree as ET | ||||||
| from .. import NAMESPACE | ||||||
| from item_types import ItemTypes | ||||||
| from task_item import TaskItem | ||||||
| from exceptions import ResponseBodyError | ||||||
| from property_decorators import property_is_int, property_is_enum | ||||||
|
|
||||||
| class ExtractRefreshTaskItem(TaskItem): | ||||||
|
|
||||||
| class RefreshType: | ||||||
| FullRefresh = "FullRefresh" | ||||||
| IncrementalRefresh = "IncrementalRefresh" | ||||||
|
|
||||||
| # A user should never create this item. It should only ever be created by a return call from | ||||||
| # a REST API. Possibly we could be more vigilant in hiding the constructor | ||||||
| def __init__(self, id, schedule_id, priority, refresh_type, item_type, item_id): | ||||||
| super(ExtractRefreshTaskItem, self).__init__(id, schedule_id) | ||||||
| self.priority = priority | ||||||
| self.type = type | ||||||
| self.refresh_type = refresh_type | ||||||
| self.item_type = item_type | ||||||
| self.item_id = item_id | ||||||
|
|
||||||
| @property | ||||||
| def priority(self): | ||||||
| return self._priority | ||||||
|
|
||||||
| @priority.setter | ||||||
| @property_is_int(range=(1, 100)) | ||||||
| def priority(self, value): | ||||||
| self._priority = value | ||||||
|
|
||||||
| @property | ||||||
| def refresh_type(self): | ||||||
| return self._refresh_type | ||||||
|
|
||||||
| @refresh_type.setter | ||||||
| @property_is_enum(RefreshType) | ||||||
| def refresh_type(self, value): | ||||||
| self._refresh_type = value | ||||||
|
|
||||||
| @property | ||||||
| def item_type(self): | ||||||
| return self._item_type | ||||||
|
|
||||||
| @item_type.setter | ||||||
| def item_type(self, value): | ||||||
| # Check that it is Datasource or Workbook | ||||||
| if not (value in [ItemTypes.Datasource, ItemTypes.Workbook]): | ||||||
|
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. suggestion (code-quality): Simplify logical expression using De Morgan identities (
Suggested change
|
||||||
| error = "Invalid value: {0}. item_type must be either ItemTypes.Datasource or ItemTypes.Workbook".format(value) | ||||||
| raise ValueError(error) | ||||||
| self._item_type = value | ||||||
|
|
||||||
| @property | ||||||
| def item_id(self): | ||||||
| return self._item_id | ||||||
|
|
||||||
| @item_id.setter | ||||||
| def item_id(self, value): | ||||||
| self._item_id = value | ||||||
|
|
||||||
|
|
||||||
| @classmethod | ||||||
| def from_response(cls, resp, schedule_id): | ||||||
| tasks_items = list() | ||||||
|
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. suggestion (code-quality): Replace
Suggested change
ExplanationThe most concise and Pythonic way to create a list is to use the[] notation.
This fits in with the way we create lists with elements, saving a bit of mental x = ["first", "second"]Doing things this way has the added advantage of being a nice little performance Here are the timings before and after the change: Similar reasoning and performance results hold for replacing |
||||||
| parsed_response = ET.fromstring(resp) | ||||||
| extract_tags = parsed_response.findall('.//t:extract', namespaces=NAMESPACE) | ||||||
| for extract_tag in extract_tags: | ||||||
| (id, priority, refresh_type, item_type, item_id) = cls._parse_element(extract_tag) | ||||||
|
|
||||||
| task = cls(id, schedule_id, priority, refresh_type, item_type, item_id) | ||||||
| tasks_items.append(task) | ||||||
| return tasks_items | ||||||
|
|
||||||
| @staticmethod | ||||||
| def _parse_element(extract_tag): | ||||||
| id = extract_tag.get('id') | ||||||
|
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. issue (code-quality): Don't assign to builtin variable ExplanationPython has a number ofbuiltin variables: functions and constants thatform a part of the language, such as list, getattr, and type(See https://docs.python.org/3/library/functions.html). It is valid, in the language, to re-bind such variables: list = [1, 2, 3]However, this is considered poor practice.
How can you solve this? Rename the variable something more specific, such as |
||||||
| priority = int(extract_tag.get('priority')) | ||||||
|
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. issue: Missing error handling for malformed XML attributes in _parse_element Add error handling for cases where priority is not a valid integer or other attributes are missing/malformed. |
||||||
| refresh_type = extract_tag.get('type') | ||||||
|
|
||||||
| datasource_tag = extract_tag.find('.//t:datasource', namespaces=NAMESPACE) | ||||||
| workbook_tag = extract_tag.find('.//t:workbook', namespaces=NAMESPACE) | ||||||
| if datasource_tag is not None: | ||||||
| item_type = ItemTypes.Datasource | ||||||
| item_id = datasource_tag.get('id') | ||||||
| elif workbook_tag is not None: | ||||||
| item_type = ItemTypes.Workbook | ||||||
| item_id = workbook_tag.get('id') | ||||||
| else: | ||||||
| error = "Missing workbook / datasource element for refresh task" | ||||||
| raise ResponseBodyError(error) | ||||||
|
|
||||||
| return id, priority, refresh_type, item_type, item_id | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| class ItemTypes: | ||
| Datasource = 'Datasource' | ||
| Workbook = 'Workbook' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| class TaskItem(object): | ||
|
|
||
| def __init__(self, id, schedule_id): | ||
| self._id = id | ||
| self._schedule_id = schedule_id | ||
|
|
||
| @property | ||
| def id(self): | ||
| return self._id | ||
|
|
||
| @property | ||
| def schedule_id(self): | ||
| return self._schedule_id |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| from .endpoint import Endpoint | ||
| from .. import PaginationItem, ExtractRefreshTaskItem | ||
| import logging | ||
|
|
||
| logger = logging.getLogger('tableau.endpoint.tasks') | ||
|
|
||
| class ExtractRefreshes(Endpoint): | ||
| def __init__(self, parent_srv): | ||
| super(ExtractRefreshes, self).__init__() | ||
| self.parent_srv = parent_srv | ||
|
|
||
| @property | ||
| def baseurl(self): | ||
| return "{0}/sites/{1}".format(self.parent_srv.baseurl, self.parent_srv.site_id) | ||
|
|
||
| def get_for_schedule(self, schedule_id, req_options=None): | ||
| url = "{0}/schedules/{1}/extracts".format(self.baseurl, schedule_id) | ||
| server_response = self.get_request(url, req_options) | ||
| pagination_item = PaginationItem.from_response(server_response.content) | ||
| tasks = ExtractRefreshTaskItem.from_response(server_response.content, schedule_id) | ||
| return tasks, pagination_item |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| from .endpoint import Endpoint | ||
| from extract_refreshes_endpoint import ExtractRefreshes | ||
| import logging | ||
|
|
||
| logger = logging.getLogger('tableau.endpoint.tasks') | ||
|
|
||
| class Tasks(Endpoint): | ||
| def __init__(self, parent_srv): | ||
| super(Tasks, self).__init__() | ||
| self.parent_srv = parent_srv | ||
| self.extract_refreshes = ExtractRefreshes(parent_srv) |
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.
issue (bug_risk): The 'type' parameter in init is assigned to self.type but never used elsewhere in the class
This unused assignment could lead to confusion and potential bugs. Consider removing it if not needed.