Conversation
The Tableau REST API supports a `refreshExtractTriggered="true"` attribute on subscription payloads that makes the subscription fire when its referenced schedule's extract refresh completes, rather than on the schedule's time trigger. On Tableau Cloud, this is the wire form of an "On Extract Refresh" subscription. TSC never exposed this attribute; users trying to create these subscriptions were passing `schedule_id=None` and hitting a confusing wire error deep in the endpoint layer. Changes: - `SubscriptionItem.on_extract_refresh(...)` classmethod factory constructs a subscription with an extract-refresh schedule id and the flag set. - `refresh_extract_triggered` exposed as a property with a docstring covering the two ways the server surprises callers (server rejects True with a non-extract schedule; server silently clears the flag when a schedule change is included in an update). - `Subscriptions.create()` and `.update()` now raise `ValueError` up front when `schedule_id` is missing, so the wire error becomes an actionable client-side message. - `create_req` emits `refreshExtractTriggered="true"` only when set; `update_req` emits both true and false so callers can turn the flag off on an existing subscription. - `_parse_element` reads the attribute back into the property; parse continues to accept inline-schedule responses (schedule_id=None). Tests cover: factory sets flag + schedule id; default false; create_req emit-when-set/omit-when-false; update_req always emits; parse round-trip for both true and missing; parse of inline-schedule responses; create() and update() reject missing schedule_id. Related to #1658. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds first-class support for Tableau REST API’s refreshExtractTriggered subscription attribute (Tableau Cloud “On Extract Refresh” subscriptions) by exposing it on SubscriptionItem, emitting/parsing it in XML requests/responses, and improving client-side validation/errors when schedule_id is missing.
Changes:
- Introduces
SubscriptionItem.refresh_extract_triggeredplusSubscriptionItem.on_extract_refresh(...)factory for extract-refresh-triggered subscriptions. - Updates subscription request XML generation/parsing to handle
refreshExtractTriggered(create emits only when true; update always emits true/false). - Adds endpoint-level validation for missing
schedule_idand expands test coverage for the new behavior and regression cases.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
test/test_subscription.py |
Adds tests for factory, defaults, XML emit/omit behavior, parsing, and create/update validation. |
tableauserverclient/server/request_factory.py |
Emits refreshExtractTriggered in create/update subscription request payloads. |
tableauserverclient/server/endpoint/subscriptions_endpoint.py |
Validates schedule_id presence for create/update and raises clearer ValueErrors. |
tableauserverclient/models/subscription_item.py |
Adds property + factory, documents behavior, and parses refreshExtractTriggered from responses. |
Suppressed comments (1)
tableauserverclient/server/endpoint/subscriptions_endpoint.py:75
- update() uses a truthiness check for schedule_id, which won’t reject whitespace-only values and is slightly inconsistent with update_req (which treats schedule_id as optional and would emit a without an id). Making the validation explicit for None/empty/whitespace keeps failures predictable and avoids generating invalid XML.
if not subscription_item.schedule_id:
# A subscription round-tripped from an inline-schedule response
# (Cloud/TOL) has schedule_id=None. Updating it in that state
# sends <schedule/> with no id and hits the same wire-layer error
# that create() guards against. See tableau/server-client-python#1658.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Docstring on `refresh_extract_triggered` now warns about the manual- build update() footgun: because every subscriptions.update() payload carries the attribute, a caller who builds a fresh SubscriptionItem locally, stamps _id, and updates will silently flip an existing on-extract-refresh subscription off. Fetch first. - Soften create()'s "schedule_id is required" error so someone who just forgot to set schedule_id on a time-based subscription doesn't get steered exclusively toward SubscriptionItem.on_extract_refresh(...); the factory is now mentioned as a conditional pointer. - __init__'s schedule_id parameter is now typed str | None, matching the real state: _parse_element sets it to None on inline-schedule responses. Drop the two `# type: ignore` markers in test/test_subscription.py that were papering over the earlier lie. - create_req asserts schedule_id non-None to satisfy mypy after the parameter widening; subscriptions.create() already guards this path before request emission. - Add samples/create_extract_refresh_subscription.py demonstrating the full flow: sign in, resolve view/workbook and user by name, pick an extract-refresh schedule from the schedules list, build the subscription via on_extract_refresh(), post it. Highest-leverage discoverability artifact for callers searching "on extract refresh". - CHANGELOG entry.
jacalata
added a commit
that referenced
this pull request
Aug 18, 2026
Three fresh-eyes findings: - Remove the "Subscriptions cannot use On Extract Refresh" entry. The REST API's refreshExtractTriggered attribute does support this, and TSC support is landing in #1861 (which closes the tracker #1658). The entry is about to be factually wrong on both counts. - Soften the intro: it promised "follow the appropriate issues" for every item, but the vf_-silently-dropped entry has no tracker filed. New wording says "where filed" so the doc doesn't overpromise. - Reword the daily-schedules note to describe behavior neutrally ("currently runs daily schedules hourly instead") rather than calling it a "bug", which reads punchy on a public docs page.
jacalata
added a commit
that referenced
this pull request
Aug 20, 2026
* docs: add Known server-side limitations section Add a new user-facing section to docs/api-ref.md that lists behaviors users may hit that are constrained by the Tableau Server REST API rather than by TSC. Each entry links its tracking issue and notes where the fix is expected to land. Cross-link the new section from the top-of-page note, and point at the `Server-Side Enhancement` label as the live tracker maintained by this repo. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: drop internal work-item IDs and per-bullet fix-scope tag Public docs on gh-pages will be indexed, so remove the internal work-item numbers (W-...) from the Known server-side limitations section along with the paragraph that described what they were. Also strip the redundant per-bullet "Fix expected: server-side" tag: the section title already scopes the whole list to server-side gaps. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Drop On Extract Refresh entry, soften intro and daily-schedule wording Three fresh-eyes findings: - Remove the "Subscriptions cannot use On Extract Refresh" entry. The REST API's refreshExtractTriggered attribute does support this, and TSC support is landing in #1861 (which closes the tracker #1658). The entry is about to be factually wrong on both counts. - Soften the intro: it promised "follow the appropriate issues" for every item, but the vf_-silently-dropped entry has no tracker filed. New wording says "where filed" so the doc doesn't overpromise. - Reword the daily-schedules note to describe behavior neutrally ("currently runs daily schedules hourly instead") rather than calling it a "bug", which reads punchy on a public docs page. * Drop the vf_ exact-match limitation entry Opus-5 review found the entry's tracker link (#1431) points at a closed-not-planned issue; the underlying gap was filed with the Tableau REST API docs team internally and there's no realistic timeline for it being tracked back in the TSC repo. Rather than carry an untracked entry indefinitely, drop it. The behavior claim (vf_ is exact-match / OR-list only) is now covered by the expanded docstring on RequestOptions.vf() in #1840. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jacalata
added a commit
that referenced
this pull request
Aug 21, 2026
Round of fixes for the sample-scripts refactor after fresh-eyes review. Blocker: publish_workbook.py reused `-u` for --thumbnails-user-id while _shared.py add_common_arguments already binds `-u` to --username, so argparse raised ArgumentError on module load and the script would not start. Renamed to `-U`. Real bugs: - _shared.py .env search now checks cwd, samples/, and repo root (in that order) so the docstring stops lying about "next to the sample or cwd." - resolve_credentials now gates input()/getpass on sys.stdin.isatty() as the docstring already promised, so piped/CI invocations no longer hang forever. - manage_subscriptions.py --attach-image switched to argparse.BooleanOptionalAction so users can actually pass --no-attach-image; the previous store_true+default=True made the flag a permanent True. - Header docstring in _shared.py no longer claims "no existing command line breaks" (which was false: -p migrated from --token-name to --password in an earlier commit). Documented the tabcmd-aligned short flags instead. - Corrected Python-version headers on login.py, list_jobs.py, manage_subscriptions.py, publish_workbook.py, refresh_tasks.py, move_workbook_sites.py, publish_datasource.py, and update_workbook_data_freshness_policy.py -- repo floor is 3.10 per pyproject.toml. - list_jobs._wait_for_job: reordered excepts so JobCancelledException (a subclass of JobFailedException) is caught first, otherwise cancelled jobs were reported as failed with the wrong exit code. - login.py sign-in banner now branches on JWTAuth as well, so JWT logins no longer print "Username: None". Header env-var list updated to include TABLEAU_JWT / TABLEAU_JWT_FILE. New JWT support: _shared.py add_common_arguments now exposes --jwt and --jwt-file, resolves TABLEAU_JWT / TABLEAU_JWT_FILE from env, reads a JWT file path into args.jwt during resolve_credentials, and returns TSC.JWTAuth from build_auth when a JWT is present. JWT takes priority over PAT and username/password. Extract-refresh subscription: manage_subscriptions.py create now accepts --on-extract-refresh, which calls SubscriptionItem.on_extract_refresh() to construct a subscription that fires when the referenced extract-refresh schedule completes (the flow introduced in #1861). Rebased this branch onto jac/subscription-refresh-extract-triggered so the flag lands on top of the new API without conflicts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- update() now falls back to subscription.schedule.id when schedule_id is None. Fixes Cloud fetch-then-update (schedule_id is always None on Cloud inline-schedule responses per #1875), which the property docstring recommended but the guard rejected. - request_factory create_req: replace assert with if-raise so python -O and direct-import callers get an actionable error instead of an unhelpful ElementTree TypeError. - Add regression tests: (a) manually-constructed SubscriptionItem through update() emits the intended refreshExtractTriggered value, and (b) fetch-parse-mutate-update round-trip preserves the flag. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Inline-schedule responses can fail during parsing and lack a usable schedule identifier for the documented update flow.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Review effort level: Lite
Comment on lines
+80
to
+83
| if schedule_id is None and subscription_item.schedule is not None: | ||
| schedule_id = subscription_item.schedule.id | ||
| if schedule_id is not None: | ||
| subscription_item.schedule_id = schedule_id |
jacalata
added a commit
that referenced
this pull request
Sep 17, 2026
Restore subscription_item, subscriptions_endpoint, request_factory, and test_subscription to origin/development state, and drop the matching "On Extract Refresh subscriptions" bullet from CHANGELOG's Unreleased section. That work is being landed via #1861 so it does not need to ride along in this samples-focused PR. Leaves this PR as a pure samples/CHANGELOG-free contribution: shared credential resolver, pagination fixes, list_jobs, manage_subscriptions, and the small samples cleanups already staged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jacalata
added a commit
that referenced
this pull request
Sep 17, 2026
samples/create_extract_refresh_subscription.py depends on SubscriptionItem.on_extract_refresh(), which is added by #1861 and was already removed from this PR's diff along with the rest of the refreshExtractTriggered work. #1861's branch already carries the same sample byte-identical. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jacalata
added a commit
that referenced
this pull request
Sep 17, 2026
Copilot round-2 (2026-09-17) findings: - _shared.py: build_auth now validates args.server so non-TTY callers hit a clear ValueError instead of TSC.Server(None, ...) downstream. - explore_favorites.py: favorite-delete cleanup moved inside the `with server.auth.sign_in(...)` block; each delete guarded by `if my_workbook is not None:` etc. to match the add-side. - update_workbook_data_freshness_policy.py: all_workbooks[2] -> [0] with a follow-up comment; argparse description corrected. Fresh-eyes findings this pass caught: - manage_subscriptions.py: drop the --on-extract-refresh path entirely (docstring, code branch, argparse flag). That relies on SubscriptionItem.on_extract_refresh which lands with #1861 and is not present on this branch after the earlier subscription revert. - extracts.py: `all_workbooks[3]` -> `[0]`; guard the create/delete branches against `wb is None` so `--datasource ... --create` no longer AttributeErrors on `wb.name`; --workbook/--datasource made mutually exclusive to match how the sample is meant to be used. - publish_datasource.py: raise a clear "no project named X" error when the project filter matches zero; fix a swapped-argument print so the datasource id no longer prefixes the "Datasource published" message with the timestamp reading as the id. - refresh_tasks.py: subparsers marked required=True so running the sample with no subcommand prints usage instead of AttributeError. Not fixed in this PR (pre-existing, flagged for follow-up): - explore_workbook.py:120-149 has three latent bugs (missing `=` on `changed`, `c` referenced outside its loop, `--delete` not in this script's argparse). This PR only adds the _shared import; the bugs pre-date it and belong in a separate cleanup PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jacalata
added a commit
that referenced
this pull request
Sep 18, 2026
…ons samples (#1843) * samples: add shared credential resolver that avoids the command line Introduces samples/_shared.py with resolve_credentials(args), which fills missing sign-in values from env vars (TABLEAU_SERVER, TABLEAU_TOKEN_NAME, etc.) or a .env-style file, and falls back to interactive getpass so secrets never touch shell history. CLI args still work for CI use. Wires the new helper into login.py, publish_workbook.py, and publish_datasource.py to establish the pattern; the remaining samples still accept the same CLI args and continue to work as before. Addresses #1551 item 1. * samples: fix mispagination in samples that treated a single page as all Several samples called `server.<endpoint>.get()` and named the result `all_workbooks`, `all_datasources`, etc. This only returns the first page (default 100 items); if the item of interest was not on that page it was silently missed and the sample failed with a "not found" message. Replace those calls with `TSC.Pager(server.<endpoint>)` so every page is walked. Where a total count was being displayed we still make one plain `.get()` up front so the total_available field is available without paging through the whole site twice. Also corrects an unrelated typo in getting_started/3_hello_universe.py where the "workbooks" section actually queried datasources. Addresses #1551 item 2 (and #1531). * samples: add list_jobs and manage_subscriptions for coverage gaps The existing samples cover workbooks, datasources, schedules, extracts, projects, users, groups, favorites, and webhooks, but there was no sample for two frequently asked-about endpoints: * list_jobs.py -- lists background jobs (extract refreshes, publishes, flow runs, etc.), demonstrating the .filter() queryset with date/status/type filters and the wait_for_job helper. * manage_subscriptions.py -- list/create/delete site subscriptions, demonstrating the SubscriptionItem + Target pattern and paginated listing with TSC.Pager. Both samples use the new samples/_shared.py credential resolver so the sign-in pattern matches the rest of the samples. Addresses #1551 item 3. * samples: align sign-in short flags with tabcmd Restore -t for --site, -u for --username, -p for --password; drop short flags on --token-name and --token-value. This matches tabcmd's canonical short flags in tabcmd/execution/parent_parser.py so users running both tools have one convention to remember. The initial refactor picked new short flags without noticing that the old samples/login.py already followed tabcmd's convention (-p was --password, -t was --site). Reassigning -p to --token-name meant `python login.py -p <password>` silently sent the password as a token name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: expose refreshExtractTriggered on SubscriptionItem (#1658) The Tableau REST API supports a `refreshExtractTriggered="true"` attribute on subscription payloads that makes the subscription fire when its referenced schedule's extract refresh completes, rather than on the schedule's time trigger. On Tableau Cloud, this is the wire form of an "On Extract Refresh" subscription. TSC never exposed this attribute; users trying to create these subscriptions were passing `schedule_id=None` and hitting a confusing wire error deep in the endpoint layer. Changes: - `SubscriptionItem.on_extract_refresh(...)` classmethod factory constructs a subscription with an extract-refresh schedule id and the flag set. - `refresh_extract_triggered` exposed as a property with a docstring covering the two ways the server surprises callers (server rejects True with a non-extract schedule; server silently clears the flag when a schedule change is included in an update). - `Subscriptions.create()` and `.update()` now raise `ValueError` up front when `schedule_id` is missing, so the wire error becomes an actionable client-side message. - `create_req` emits `refreshExtractTriggered="true"` only when set; `update_req` emits both true and false so callers can turn the flag off on an existing subscription. - `_parse_element` reads the attribute back into the property; parse continues to accept inline-schedule responses (schedule_id=None). Tests cover: factory sets flag + schedule id; default false; create_req emit-when-set/omit-when-false; update_req always emits; parse round-trip for both true and missing; parse of inline-schedule responses; create() and update() reject missing schedule_id. Related to #1658. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Address fresh-eyes review on refreshExtractTriggered subscriptions - Docstring on `refresh_extract_triggered` now warns about the manual- build update() footgun: because every subscriptions.update() payload carries the attribute, a caller who builds a fresh SubscriptionItem locally, stamps _id, and updates will silently flip an existing on-extract-refresh subscription off. Fetch first. - Soften create()'s "schedule_id is required" error so someone who just forgot to set schedule_id on a time-based subscription doesn't get steered exclusively toward SubscriptionItem.on_extract_refresh(...); the factory is now mentioned as a conditional pointer. - __init__'s schedule_id parameter is now typed str | None, matching the real state: _parse_element sets it to None on inline-schedule responses. Drop the two `# type: ignore` markers in test/test_subscription.py that were papering over the earlier lie. - create_req asserts schedule_id non-None to satisfy mypy after the parameter widening; subscriptions.create() already guards this path before request emission. - Add samples/create_extract_refresh_subscription.py demonstrating the full flow: sign in, resolve view/workbook and user by name, pick an extract-refresh schedule from the schedules list, build the subscription via on_extract_refresh(), post it. Highest-leverage discoverability artifact for callers searching "on extract refresh". - CHANGELOG entry. * samples: add shared credential resolver that avoids the command line Introduces samples/_shared.py with resolve_credentials(args), which fills missing sign-in values from env vars (TABLEAU_SERVER, TABLEAU_TOKEN_NAME, etc.) or a .env-style file, and falls back to interactive getpass so secrets never touch shell history. CLI args still work for CI use. Wires the new helper into login.py, publish_workbook.py, and publish_datasource.py to establish the pattern; the remaining samples still accept the same CLI args and continue to work as before. Addresses #1551 item 1. * samples: fix mispagination in samples that treated a single page as all Several samples called `server.<endpoint>.get()` and named the result `all_workbooks`, `all_datasources`, etc. This only returns the first page (default 100 items); if the item of interest was not on that page it was silently missed and the sample failed with a "not found" message. Replace those calls with `TSC.Pager(server.<endpoint>)` so every page is walked. Where a total count was being displayed we still make one plain `.get()` up front so the total_available field is available without paging through the whole site twice. Also corrects an unrelated typo in getting_started/3_hello_universe.py where the "workbooks" section actually queried datasources. Addresses #1551 item 2 (and #1531). * samples: add list_jobs and manage_subscriptions for coverage gaps The existing samples cover workbooks, datasources, schedules, extracts, projects, users, groups, favorites, and webhooks, but there was no sample for two frequently asked-about endpoints: * list_jobs.py -- lists background jobs (extract refreshes, publishes, flow runs, etc.), demonstrating the .filter() queryset with date/status/type filters and the wait_for_job helper. * manage_subscriptions.py -- list/create/delete site subscriptions, demonstrating the SubscriptionItem + Target pattern and paginated listing with TSC.Pager. Both samples use the new samples/_shared.py credential resolver so the sign-in pattern matches the rest of the samples. Addresses #1551 item 3. * samples: align sign-in short flags with tabcmd Restore -t for --site, -u for --username, -p for --password; drop short flags on --token-name and --token-value. This matches tabcmd's canonical short flags in tabcmd/execution/parent_parser.py so users running both tools have one convention to remember. The initial refactor picked new short flags without noticing that the old samples/login.py already followed tabcmd's convention (-p was --password, -t was --site). Reassigning -p to --token-name meant `python login.py -p <password>` silently sent the password as a token name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * samples: fix argparse blocker + 7 bugs, add JWT + on-extract-refresh Round of fixes for the sample-scripts refactor after fresh-eyes review. Blocker: publish_workbook.py reused `-u` for --thumbnails-user-id while _shared.py add_common_arguments already binds `-u` to --username, so argparse raised ArgumentError on module load and the script would not start. Renamed to `-U`. Real bugs: - _shared.py .env search now checks cwd, samples/, and repo root (in that order) so the docstring stops lying about "next to the sample or cwd." - resolve_credentials now gates input()/getpass on sys.stdin.isatty() as the docstring already promised, so piped/CI invocations no longer hang forever. - manage_subscriptions.py --attach-image switched to argparse.BooleanOptionalAction so users can actually pass --no-attach-image; the previous store_true+default=True made the flag a permanent True. - Header docstring in _shared.py no longer claims "no existing command line breaks" (which was false: -p migrated from --token-name to --password in an earlier commit). Documented the tabcmd-aligned short flags instead. - Corrected Python-version headers on login.py, list_jobs.py, manage_subscriptions.py, publish_workbook.py, refresh_tasks.py, move_workbook_sites.py, publish_datasource.py, and update_workbook_data_freshness_policy.py -- repo floor is 3.10 per pyproject.toml. - list_jobs._wait_for_job: reordered excepts so JobCancelledException (a subclass of JobFailedException) is caught first, otherwise cancelled jobs were reported as failed with the wrong exit code. - login.py sign-in banner now branches on JWTAuth as well, so JWT logins no longer print "Username: None". Header env-var list updated to include TABLEAU_JWT / TABLEAU_JWT_FILE. New JWT support: _shared.py add_common_arguments now exposes --jwt and --jwt-file, resolves TABLEAU_JWT / TABLEAU_JWT_FILE from env, reads a JWT file path into args.jwt during resolve_credentials, and returns TSC.JWTAuth from build_auth when a JWT is present. JWT takes priority over PAT and username/password. Extract-refresh subscription: manage_subscriptions.py create now accepts --on-extract-refresh, which calls SubscriptionItem.on_extract_refresh() to construct a subscription that fires when the referenced extract-refresh schedule completes (the flow introduced in #1861). Rebased this branch onto jac/subscription-refresh-extract-triggered so the flag lands on top of the new API without conflicts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * samples: address remaining fresh-eyes review followups (#1843) Follow-up round of fixes on top of the fresh-eyes review pass. Each change maps to a specific finding from that review. Migrate stragglers to _shared (M4). Eight samples still had their own inline argparse and inline PersonalAccessTokenAuth construction: explore_{datasource,favorites,webhooks,workbook}.py, extracts.py, move_workbook_sites.py, refresh_tasks.py, and update_workbook_data_freshness_policy.py. All now call _shared.add_common_arguments and _shared.build_auth so the tabcmd-aligned short-flag convention (-s -t -u -p -l) applies uniformly and any future credential-handling fix lives in one place. Skip getting_started/3_hello_universe.py: intentionally a hardcoded starter with no argparse, aimed at teaching new users to edit the source directly. Different pedagogy from the CLI samples. Fix explore_favorites empty-site handling (L8). The favorite-datasource add and delete calls used to run unconditionally with my_datasource initialized to None, so on an empty site the sample failed partway. Both calls are now guarded (add inside the existing `if all_datasource_items:` block, delete under a new `if my_datasource is not None:` check). Drop verify=False TLS bypass (L11). Removed http_options={"verify": False} from publish_workbook.py and the equivalent server.add_http_options({"verify": False}) pattern from extracts.py and update_workbook_data_freshness_policy.py. A sample teaching users to bypass TLS validation is the wrong first impression; TSC defaults to verify=True, which is what a paved-path deployment expects. Users on self-signed dev servers can still set the option at their own call site. Delete dead _shared.sign_in() helper (L9). It was not called by any migrated sample: they all use resolve_credentials + build_auth + `with server.auth.sign_in(auth):` for the auto-signout context manager. The helper did not compose with `with` because it returned a Server object rather than a context manager. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Defer subscription refreshExtractTriggered to #1861 Restore subscription_item, subscriptions_endpoint, request_factory, and test_subscription to origin/development state, and drop the matching "On Extract Refresh subscriptions" bullet from CHANGELOG's Unreleased section. That work is being landed via #1861 so it does not need to ride along in this samples-focused PR. Leaves this PR as a pure samples/CHANGELOG-free contribution: shared credential resolver, pagination fixes, list_jobs, manage_subscriptions, and the small samples cleanups already staged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Drop create_extract_refresh_subscription sample, defer to #1861 samples/create_extract_refresh_subscription.py depends on SubscriptionItem.on_extract_refresh(), which is added by #1861 and was already removed from this PR's diff along with the rest of the refreshExtractTriggered work. #1861's branch already carries the same sample byte-identical. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Address Copilot + fresh-eyes review on samples Copilot round-2 (2026-09-17) findings: - _shared.py: build_auth now validates args.server so non-TTY callers hit a clear ValueError instead of TSC.Server(None, ...) downstream. - explore_favorites.py: favorite-delete cleanup moved inside the `with server.auth.sign_in(...)` block; each delete guarded by `if my_workbook is not None:` etc. to match the add-side. - update_workbook_data_freshness_policy.py: all_workbooks[2] -> [0] with a follow-up comment; argparse description corrected. Fresh-eyes findings this pass caught: - manage_subscriptions.py: drop the --on-extract-refresh path entirely (docstring, code branch, argparse flag). That relies on SubscriptionItem.on_extract_refresh which lands with #1861 and is not present on this branch after the earlier subscription revert. - extracts.py: `all_workbooks[3]` -> `[0]`; guard the create/delete branches against `wb is None` so `--datasource ... --create` no longer AttributeErrors on `wb.name`; --workbook/--datasource made mutually exclusive to match how the sample is meant to be used. - publish_datasource.py: raise a clear "no project named X" error when the project filter matches zero; fix a swapped-argument print so the datasource id no longer prefixes the "Datasource published" message with the timestamp reading as the id. - refresh_tasks.py: subparsers marked required=True so running the sample with no subcommand prints usage instead of AttributeError. Not fixed in this PR (pre-existing, flagged for follow-up): - explore_workbook.py:120-149 has three latent bugs (missing `=` on `changed`, `c` referenced outside its loop, `--delete` not in this script's argparse). This PR only adds the _shared import; the bugs pre-date it and belong in a separate cleanup PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Address Copilot rounds 3/4 remaining findings Round 3 nits: - publish_workbook.py: `-U` comment now says the conflict would happen when the parser is built at run time inside main(), not "at import". - update_workbook_data_freshness_policy.py: `first_page` was assigned but unused; renamed to `_`. Round 4 (after last push): - _shared.py: added the two missing partial-credential branches so a user who supplies TABLEAU_TOKEN_VALUE without TABLEAU_TOKEN_NAME is prompted for the (non-secret) name, and one who supplies a password without a username is prompted for the username. Previously both fell through to the "fully unspecified" PAT prompt. - _shared.py: `--jwt` help text now describes the JWT > PAT > username/password precedence build_auth actually implements, rather than claiming a mutual exclusion that argparse doesn't enforce. - list_jobs.py: `--hours` now passes the tz-aware datetime directly to QuerySet.filter(created_at__gte=...) rather than `.isoformat()`. TSC serializes it as UTC with a trailing Z; the raw isoformat string could produce `+00:00` offsets that older Tableau Server versions reject. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix publish_workbook empty-projects guard + move_workbook_sites help text publish_workbook.py: mirror the empty-projects guard that landed in publish_datasource.py earlier this PR. A --project filter that matches zero results would have slipped past `if len(projects) > 1` and hit `projects[0].id` with an IndexError; now raises a clear ValueError. move_workbook_sites.py: argparse description used implicit string concatenation with missing spaces at the boundaries, so --help printed "...from thedefault project of the default site tothe default project of another site." Reflowed as a parenthesized single-string so the sentence reads correctly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Update publish_datasource header comment for build_auth Comment claimed the sample "uses personal access tokens" for sign-in, but the file now delegates to build_auth() which supports JWT, PAT, and username/password. Reword so users see the full auth surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot flagged two issues in SubscriptionItem._parse_element and the subscriptions.update() fallback path (2026-09-11 review): 1. `ScheduleItem.from_element(...)` returns a list, but the parser assigned it directly to `subscription.schedule`, so any downstream code doing `subscription.schedule.id` (including the update() fallback added in this PR) crashed with AttributeError on any real inline-schedule response. Unwrap the list in SubscriptionItem._parse_element: pick the first ScheduleItem — subscriptions carry at most one <schedule> element — so callers see a proper ScheduleItem instance. 2. Even after normalizing to a ScheduleItem, the inline-schedule response covered by test_parse_response_with_inline_schedule_no_id has no `id` attribute, so ScheduleItem.id is also None and the update()'s fallback still can't fill schedule_id. Tighten the ValueError message on the endpoint to document exactly this case (Cloud response inlined schedule with no id) and point callers at `server.schedules.get(...)` as the workaround. Update the refresh_extract_triggered property docstring with the same caveat so users see it before hitting the runtime error. Add two tests: - Strengthen test_parse_response_with_inline_schedule_no_id to assert `.schedule` is a ScheduleItem instance (guards the list-unwrap fix). - New test_update_raises_clear_error_when_inline_schedule_has_no_id exercises the endpoint's error path end-to-end and pins the message fragments users are expected to search. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🔵 Needs a closer look
A couple of small but concrete issues remain (notably inconsistent schedule_id validation in RequestFactory and a typo in an error string) that should be corrected before approval.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
tableauserverclient/server/request_factory.py:1364
- create_req only raises for schedule_id is None, but an empty-string schedule_id ("") will still serialize as when RequestFactory is used directly. This is inconsistent with Subscriptions.create(), and it makes the new error message less reliable for callers bypassing the endpoint.
tableauserverclient/server/endpoint/subscriptions_endpoint.py:44 - Typo in error message: "Susbcription" should be "Subscription".
- Files reviewed: 6/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
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
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.
Summary
Adds support for the REST API's
refreshExtractTriggered="true"attribute on subscriptions -- the wire form of Tableau Cloud's "On Extract Refresh" subscriptions. When True, the subscription fires when the referenced schedule's extract refresh completes, rather than on the schedule's time trigger.Closes #1658.
What changes
SubscriptionItem.on_extract_refresh(subject, extract_refresh_schedule_id, user_id, target)classmethod factory -- the recommended way to construct these subscriptions.refresh_extract_triggeredis now a boolean property onSubscriptionItem. Its docstring covers the two ways the server can surprise callers:schedule_idcause the server to silently clear the flag to False on that same call; converting an existing time-based subscription requires two updates.Subscriptions.create()and.update()now raiseValueErrorup front ifschedule_idis missing, so what used to be a confusing wire-layer error becomes an actionable client-side message.create_reqemitsrefreshExtractTriggered="true"only when set.update_reqemits both"true"and"false"unconditionally, so callers can turn the flag off on an existing subscription (the server retains the prior value when the attribute is absent)._parse_elementreads the attribute back into the property.Compatibility note
Every
subscriptions.update()payload now carriesrefreshExtractTriggered="true|false". Older servers that don't know the attribute should ignore unknown attrs on the<subscription>element per the schema's<xs:anyAttribute processContents="skip"/>; the attribute has been on the server since long before TSC's current minimum version.Test plan
pytest test/test_subscription.py -q-- 16 pass, including 12 new tests covering the factory, defaults,create_reqemit-when-set / omit-when-false,update_reqalways-emits in both directions, parse round-trip with and without the attribute, parse of inline-schedule responses (noschedule_id), andcreate()/update()rejection of missing schedule_id.on_extract_refresh(), confirm it appears as "On Extract Refresh" in the web UI, and confirm it fires when the referenced extract refresh runs.🤖 Generated with Claude Code