GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||
GeezerVison is showing github.com — Proposed: Remove pre-8.3 XML namespace fallback by jacalata · Pull Request #1863 · tableau/server-client-python Conversation
|
| File | Description |
|---|---|
| tableauserverclient/server/server.py | Removes per-instance namespace detector and introduces fixed namespace constants/map. |
| tableauserverclient/server/endpoint/endpoint.py | Removes the XML-response namespace detection hook in _make_request. |
| tableauserverclient/server/endpoint/auth_endpoint.py | Removes namespace detection during sign-in and site switching flows. |
| tableauserverclient/namespace.py | Deletes the legacy namespace detection module and exported symbols. |
| tableauserverclient/init.py | Updates DEFAULT_NAMESPACE re-export to come from the new namespace source. |
| CHANGELOG.md | Documents the breaking removal of legacy namespace fallback and module exports. |
tableauserverclient/server/endpoint/auth_endpoint.py:159
find(...).get(...) raise AttributeError. Adding an explicit check and raising ServerResponseError produces a clearer, user-facing failure mode for unsupported namespaces/servers. parsed_response = fromstring(server_response.content)
site_id = parsed_response.find(".//t:site", namespaces=self.parent_srv.namespace).get("id", None)
site_url = parsed_response.find(".//t:site", namespaces=self.parent_srv.namespace).get("contentUrl", None)
tableauserverclient/server/endpoint/auth_endpoint.py:86
find(...).get(...) raise an AttributeError, which is hard to diagnose. It’s better to explicitly check that the expected elements are present and raise ServerResponseError with a message that points to a likely namespace / server-version mismatch.This issue also appears on line 157 of the same file.
parsed_response = fromstring(server_response.content)
site_id = parsed_response.find(".//t:site", namespaces=self.parent_srv.namespace).get("id", None)
site_url = parsed_response.find(".//t:site", namespaces=self.parent_srv.namespace).get("contentUrl", None)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
…hared state Copilot flagged that Server.namespace returned the module-level _NAMESPACE_MAP directly, so a caller mutating the returned dict would have polluted every other Server instance in the process. Old namespace- handling code returned a per-instance dict, so no cross-instance pollution was possible. Wrap in dict(...) to restore that behavior.
Two adjustments from a post-merge fresh-eyes pass: - endpoint.py: the http -> https address-promotion match now compares hostnames case-insensitively (RFC 3986) and normalizes http://host vs http://host:80 so a same-host promotion doesn't silently drop. Split into two comparisons: current-vs-next is hostname-only (schemes differ so default port differs, comparing raw netloc would spuriously mismatch); old-address-vs-current is same-scheme and uses (hostname, effective port) so explicit-vs-implicit port compares equal. Expanded the auth-material comment to acknowledge that sign_in itself carries raw credentials in the POST body, not only the issued token on subsequent calls. - test_redirect_handling.py: added a docstring on test_all_supported_redirect_codes_preserve_post_body naming the RFC 7231 6.4.4 deviation on 303 -- if a future refactor "helpfully" converts 303 to GET, the parametrized test fails with a clear intent statement. Also considered a sign_in namespace-detect hedge for pre-8.3 Tableau responses (Copilot flagged this on #1848); dropped as theoretical because TSC's minimum_supported_server_version = 2.3 (Tableau 10.0, 2016) is eight years past the namespace change, and #1863 removes the whole subsystem anyway.
…e-old-namespace # Conflicts: # tableauserverclient/server/endpoint/auth_endpoint.py
Cosmetic import-graph cleanup on top of #1863's move of `NAMESPACE` from `namespace.py` into `server/server.py`. Before this change, `tableauserverclient/__init__.py:2` did `from tableauserverclient. server.server import NAMESPACE`, which forces `server.server` (and its transitive endpoint tree) to load just to read a string constant. Copilot flagged the circular-import path this creates: `server. server` -> `server.endpoint.endpoint` -> `from tableauserverclient import ...` -> `__init__` -> `server.server`. Python resolves the chain today because the reentrant lookups pull submodules whose own load doesn't back-reference the partially-initialized parent, but the shape is fragile and adding a new top-level import to `__init__.py` before line 2 could easily break it. Replaced the import with the string literal. Kept in sync with the `NAMESPACE` constant in `server/server.py`. Also removes one of the two entry points into `server.server` at package load, though everything else in `__init__.py` still transitively pulls the endpoint tree via the models block below, so no real startup savings -- this is defensive, not a perf fix.
CHANGELOG.md: development now points to the GitHub Releases page, so the changelog entry for this change is dropped (the breaking-change note moves to the PR description). tableauserverclient/namespace.py: deletion kept; development only added docstrings to the removed module. Add test/test_namespace.py pinning TSC.DEFAULT_NAMESPACE to server.NAMESPACE and checking Server.namespace returns an independent copy. Written with Claude Code
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
No active review issues are identified, and the change consistently removes obsolete namespace detection while preserving the documented public interfaces and defensive mapping behavior.
No open issues 🎉
Review effort: Lite
Sorry, something went wrong.
There was an error while loading. Please reload this page.
At least 1 approving review is required to merge this pull request.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
Stop supporting old namespaces
There was an error while loading. Please reload this page.