docs+feat: VirtualConnections docstrings, tag parsing, id workaround - #1858
Open
jacalata wants to merge 3 commits into
Open
docs+feat: VirtualConnections docstrings, tag parsing, id workaround#1858jacalata wants to merge 3 commits into
jacalata wants to merge 3 commits into
Conversation
Adds a class-level docstring plus 15 method docstrings (14 public methods + the private _get_virtual_database_connections is unchanged). Content ported from api-ref.md's Virtual Connections section on gh-pages so once the Sphinx pipeline in #1832 is live the generated output will cover what the handwritten page does today. Virtual Connections was the largest remaining chunk in the needs_docstring bucket per the api-ref migration audit (14 of the 41 remaining public endpoint methods without docstrings, after the 15 Favorites methods in #1855). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- add_permissions and delete_permission grow proper type hints (VirtualConnectionItem, list[PermissionsRule], PermissionsRule) so the docstring claims and the signatures agree. - Rename docstring/signature params to `virtual_connection` throughout the permissions methods; `item`, `resource`, `capability_item` were inconsistent with the rest of the class and with each other. - Rename `delete_permission`'s second param from `capability_item` to `permission_rule` (which is what it actually is). - Class docstring's REST API line now uses the same RST link style as every method's REST API line. - update_tags docstring documents WHY it's not implemented: the REST API's virtual-connection response schema doesn't include tags, so there's no way to populate _initial_tags on the item and no diff basis for the mixin's update_tags. Tags exist server-side and are manipulated via the add-tags / delete-tag endpoints that add_tags / delete_tags already wrap. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `VirtualConnections` REST endpoints have carried tags on their response bodies since Tableau Server 2026.2 / Cloud April 2026 (server commit 2e687548e10, W-21424436, shipping in v262 with REST API 3.30+). The public REST docs still don't reflect this (filed W-23806318); `VirtualConnectionItem` previously didn't parse the <tags> element. Changes: - `VirtualConnectionItem.__init__` gains `tags: set[str]` and `_initial_tags: set[str]` matching every other taggable item. - `VirtualConnectionItem.from_xml` parses <tags><tag label="..."/></tags> via the existing `TagItem.from_xml_element` helper. `_initial_tags` is a shallow copy of `tags` (strings are immutable so copy.copy is sufficient). - `VirtualConnections.update_tags` drops the NotImplementedError override and delegates to `TaggingMixin.update_tags`. Bumped to `@api(version="3.30")` since older server responses don't carry <tags>, meaning `_initial_tags` would be empty and every locally-set tag would be treated as new -- silent no-op on removes. - `VirtualConnections.get_by_id` stamps the id back onto the returned item. The `Get Virtual Connection` server response element omits the `id` attribute (separate server-side bug filed as W-23806343); downstream calls that need result.id (add_tags, delete_tags, update_tags) would fail with 'ID not found.' Client-side workaround until the server fix ships. - Fixed pre-existing "Workbook item must be populated with permissions first" copy-paste error in the `permissions` property error message. - Test fixture `virtual_connections_get.xml` grew a matching empty `<tags/>` element to reflect the current server response shape. - `test_from_xml` and `test_virtual_connection_get_by_id` assert on the new tags/id shape. - New `test_from_xml_populated_tags` covers the tag-parse path including no-back-propagation from `tags` to `_initial_tags`. - New `test_update_tags_diff_round_trip` mocks the PUT/DELETE calls to verify the diff-based mixin end-to-end. - `test_tagging.py` server version bumped from 3.28 to 3.30 so the parametrized virtual_connections update_tags case exercises the real code path. Live-verified end-to-end against a Tableau server (build 2026-08-09, REST API 3.30): fetch a VC, mutate .tags locally, call update_tags, re-fetch, server state matches the local edit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jacalata
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1569.
Motivation
Virtual Connections is the largest remaining chunk in the
needs-docstring bucket per the api-ref migration audit: 14 of the 27
remaining public endpoint methods without docstrings, once the 15
Favorites methods from #1855 land.
While backfilling the docstrings I confirmed via server code + a live
test that the REST endpoint now returns tags on virtual-connection
responses (added server-side in W-21424436, shipping in Tableau Server
2026.2 / Cloud April 2026 / REST API 3.30).
VirtualConnectionItemwasn't parsing them, so the diff-based
update_tagscouldn't work andhad a
NotImplementedErroroverride. Extending the PR to close thatgap now that the server side is available.
Also uncovered two adjacent server-side gaps while implementing this;
both filed on the server team:
describe the pre-2026.2 response schema, without
<tags>.Get Virtual ConnectionREST responseomits the
idattribute on the<virtualConnection>element eventhough
List Virtual Connectionsincludes it. Working aroundclient-side in this PR by stamping the id back from the request path.
Behavior change
For users:
VirtualConnectionsmethods now have numpy-style docstringsported from
api-ref.md, plus a class-level docstring. Content isunchanged; the generated Sphinx output (docs: set up Sphinx + ReadTheDocs pipeline for auto-generated API reference #1832) will cover what the
handwritten
api-ref.mdpage does today.VirtualConnectionItemnow parses<tags>from responses intotags: set[str](populated) and_initial_tags: set[str](immutable copy for diff purposes). On pre-2026.2 servers the
response omits
<tags>and both attributes are empty sets.VirtualConnections.update_tags(vc)works for the standardmutate-and-push pattern: fetch a VC, add/remove entries from
vc.tags, callupdate_tags. Gated on@api(version="3.30")soolder servers (where
_initial_tagswould always be empty andremoves would silently no-op) fail loudly at the version check
instead.
VirtualConnections.get_by_idnow returns an item with.idseteven though the server response omits it. Downstream calls
(
add_tags,delete_tags,update_tags) require the id.Also bundled (small):
add_permissionsanddelete_permissiongrow proper type hints sothe docstring claims and the signatures agree.
virtual_connectionthroughout the class.
delete_permission's second parameter fromcapability_itemto
permission_rule, matching what it actually is.permissionsproperty errormessage said "Workbook item must be populated..." instead of
"Virtual connection item...".
Test plan
test/test_virtual_connection.py15 passed. New tests:test_from_xml_populated_tags(multi-item fixture, verifies parse_initial_tagsisolation fromtags) andtest_update_tags_diff_round_trip(mocks PUT/DELETE calls, assertscorrect add/delete pattern from a synthetic diff).
test/test_tagging.py134 passed after bumping theparametrized-test server version to 3.30.
REST API 3.30): fetch a VC by id,
add_tags(vc, ['a','b','c']),re-fetch, verify
tags == {a,b,c}and_initial_tags == {a,b,c}.Then locally
vc.tags.discard('b'); vc.tags.add('d'); update_tags(vc). Re-fetch confirms server state is{a,c,d}.Cleanup via
delete_tags(vc, list(vc.tags))succeeds.🤖 Generated with Claude Code