-
Notifications
You must be signed in to change notification settings - Fork 448
Cleanup duplications #87
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
Cleanup duplications #87
Conversation
t8y8
left a comment
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.
🚀
Minor requests, I trust a ship it after they're addressed
|
|
||
| class Auth(Endpoint): | ||
| def __init__(self): | ||
| super(Auth, self).__init__(None) |
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.
Keyword argument? I don't know what None means it kinda makes me think it's passing some fake value
| def _check_status(server_response): | ||
| if server_response.status_code not in Success_codes: | ||
| raise ServerResponseError.from_response(server_response.content) | ||
| def _make_headers(token, content_type): |
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.
_make_tableau_headers or _make_server_headers or something to indicate it's not a totally general purpose function, it really is Tableau REST API specific headers
| self._check_status(server_response) | ||
|
|
||
| # This check is to determine if the response is a text response (xml or otherwise) | ||
| # so that we do not attempt to log bytes and other binary data. |
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.
Thank you!!!
| retval['content-type'] = content_type | ||
|
|
||
| def get_unauthenticated_request(self, url, request_object=None): | ||
| def _make_request(self, method, url, content=None, request_object=None, token=None, content_type=None): |
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.
call it auth_token.
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.
Okay
LGraber
left a comment
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.
🚀
…ableau#89) When Cross Database Joins were introduced in 10 all connections became "federated" by default, even if they weren't actually joined to anything else. In the file format that means they get represented as named-connection/connection elements. Expressed under one top-level connection element with a class of 'federated'. Except for 'sqlproxy' connections (Published Data Sources) -- they stay in the old connection style as a top level connection element. We need to, when in a 10.0 or greater workbook, get all federated connections (named-connection) plus go back and find any sqlproxy connections as well.
…ableau#89) When Cross Database Joins were introduced in 10 all connections became "federated" by default, even if they weren't actually joined to anything else. In the file format that means they get represented as named-connection/connection elements. Expressed under one top-level connection element with a class of 'federated'. Except for 'sqlproxy' connections (Published Data Sources) -- they stay in the old connection style as a top level connection element. We need to, when in a 10.0 or greater workbook, get all federated connections (named-connection) plus go back and find any sqlproxy connections as well.
Cleaning up the Endpoint base object and removing the duplicate ctors across all endpoints.