194 fix namespace issue#219
194 fix namespace issue#219graysonarts merged 21 commits intotableau:developmentfrom graysonarts:194-fix-namespace-issue
Conversation
tableauserverclient/namespace.py
Outdated
| if self._detected: | ||
| return | ||
|
|
||
| lines = (n for n in xml.split('\n') if n.startswith('<tsResponse')) |
There was a problem hiding this comment.
There's breaks all ova' them tests because of str/bytes stuff.
I think we can reduce the parsing overhead a bit as well, by parsing the namespace outta the root element.
Something like this:
>>> root = ET.fromstring(server_response.content)
>>> root.tag
'{http://tableau.com/api}tsResponse'
And then run the find over just that smaller string:
re.findall(r'\{.*\}', root.tag)
>>> ['{http://tableau.com/api}']
There was a problem hiding this comment.
hrm. Weird, I even tested wth python 3.6 on my mac. yeah I think I like this solution better.
| parameters['data'] = content | ||
|
|
||
| server_response = method(url, **parameters) | ||
| self.parent_srv._namespace.detect(server_response.content) |
There was a problem hiding this comment.
I see, the first request, of any kind, checks and caches itself, then we shortcut on subsequent requests.
I like the deisgn.
There was a problem hiding this comment.
yup. I think the only place where we might get in trouble is if the first request isn't the use server version workflow. At this point, I want to make that the default behavior. @LGraber any objections to always determining the server version to help solve this issue?
There was a problem hiding this comment.
Why would it be a problem if that wasn't the first call? I am not sure I see how this only happens on the first call?
| def baseurl(self): | ||
| return "{0}/api/{1}".format(self._server_address, str(self.version)) | ||
|
|
||
| @property |
There was a problem hiding this comment.
Is 'xml_namespace' a better name? (No strong opinion here)
There was a problem hiding this comment.
I doubt anyone outside of TSC contributors will use that property. I don't have a strong opinion either way.
There was a problem hiding this comment.
It feels SO GOOD to remove that giant package global from all those imports.
It's actually now trivial to officially support back to 9.0
EDIT: Ok, it's not trivial, since some very convenient features were added in 10 (like get workbooks). We could shim some of those but maybe it's not worth it. Still, I love this, it's so much cleaner for the future.
| matches = NAMESPACE_RE.match(root.tag) | ||
| if matches: | ||
| detected_ns = matches.group(1) | ||
| if detected_ns in (OLD_NAMESPACE, NEW_NAMESPACE): |
There was a problem hiding this comment.
Is there a reason we don't throw here if we don't know what it is? Won't we fail in an odd way later?
There was a problem hiding this comment.
Not every request necessarily returns xml (deletes for example).
This just skips it an checks on the next request. Once it succeeds we cache the result and don't check again.
We default to the new namespace so nothing should fail
There was a problem hiding this comment.
no real reason. I'll add a new exception
| from .. import ConnectionItem, DatasourceItem,\ | ||
| GroupItem, PaginationItem, ProjectItem, ScheduleItem, SiteItem, TableauAuth,\ | ||
| UserItem, ViewItem, WorkbookItem, TaskItem, NAMESPACE | ||
| UserItem, ViewItem, WorkbookItem, TaskItem, namespace |
There was a problem hiding this comment.
Why is this the only one lowercase?
There was a problem hiding this comment.
That should have been removed. It's not needed anymore
|
🚀 |
3 participants
Introduce detection of namespace url for backwards compatibility.