samples: credential helper, pagination fixes, and new jobs/subscriptions samples - #1843
Merged
Merged
Conversation
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.
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).
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.
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>
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
Updates the samples/ scripts to demonstrate safer credential handling, correct pagination patterns, and add new examples for jobs and subscriptions, without changing the core tableauserverclient library.
Changes:
- Introduces
samples/_shared.pyhelpers for common CLI args plus credential resolution from CLI/env/.env/interactive prompt. - Fixes several samples that used
.get()(first page only) by switching toTSC.Pager(...)(or iterable QuerySet) to traverse full result sets. - Adds new sample scripts for listing background jobs and managing subscriptions.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| samples/_shared.py | New shared credential + argparse helpers used by multiple samples |
| samples/login.py | Uses shared helpers; updates login flow to avoid secrets on CLI |
| samples/publish_workbook.py | Uses shared helpers; fixes project discovery to page through projects |
| samples/publish_datasource.py | Uses shared helpers; removes ad-hoc env reading and debug overrides |
| samples/refresh_tasks.py | Pages through tasks via TSC.Pager instead of first-page .get() |
| samples/move_workbook_sites.py | Pages through sites via TSC.Pager instead of first-page .get() |
| samples/update_workbook_data_freshness_policy.py | Uses TSC.Pager to list all workbooks |
| samples/extracts.py | Uses TSC.Pager to list all workbooks |
| samples/explore_workbook.py | Uses TSC.Pager for projects/workbooks/custom views paging correctness |
| samples/explore_webhooks.py | Uses TSC.Pager to list all webhooks |
| samples/explore_favorites.py | Uses TSC.Pager to list all workbooks/datasources |
| samples/explore_datasource.py | Uses TSC.Pager for projects/datasources paging correctness |
| samples/getting_started/3_hello_universe.py | Fixes incorrect endpoint (workbooks vs datasources) |
| samples/list_jobs.py | New sample demonstrating jobs listing/filtering and wait-for-job |
| samples/manage_subscriptions.py | New sample demonstrating list/create/delete subscriptions with paging |
Suppressed comments (3)
samples/publish_workbook.py:34
add_common_arguments()already reserves-ufor--username, so reusing-uhere causes argparse to raise a conflicting option error and the script won’t start.
group = parser.add_mutually_exclusive_group(required=False)
group.add_argument("--thumbnails-user-id", "-u", help="User ID to use for thumbnails")
group.add_argument("--thumbnails-group-id", "-g", help="Group ID to use for thumbnails")
samples/_shared.py:133
- This claims a
.envfile next to the sample is loaded automatically, but the implementation only checks the current working directory. If users runpython samples/<script>.pyfrom the repo root,samples/.envwill be ignored.
# Load `.env` file if one is requested or available.
env_file = getattr(args, "env_file", None)
if env_file:
_load_env_file(Path(env_file))
else:
default_env = Path.cwd() / ".env"
if default_env.is_file():
_load_env_file(default_env)
samples/_shared.py:149
resolve_credentialswill callinput()/getpass.getpass()even when stdin is not a TTY, which can hang non-interactive runs despite the docstring saying prompts happen only when stdin is a terminal.
if not allow_prompt:
return
# Prompt for what's still missing. We only prompt for the pieces we
# actually need: server URL, and one of token or username/password.
if not getattr(args, "server", None):
args.server = input("Tableau server URL: ").strip()
💡 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.
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.
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).
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.
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>
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>
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>
4 tasks
…provements # Conflicts: # CHANGELOG.md
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>
There was a problem hiding this comment.
🟡 Changes recommended
There are a few confirmed runtime/correctness issues in the updated/new samples (auth-scope bug, small-site IndexError, and CLI flag inconsistency) that should be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 16/16 changed files
- Comments generated: 5
- Review effort level: Lite
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>
There was a problem hiding this comment.
🔵 Needs a closer look
The new/updated samples include confirmed runtime failures (unsupported SubscriptionItem.on_extract_refresh usage and favorites cleanup occurring after sign-out) that must be corrected before users can run them successfully.
Review details
Suppressed comments (8)
Previously missed (3) — in code that hasn't changed since the last review.
samples/manage_subscriptions.py:30
- The header comment includes a full
--on-extract-refreshexample, but the script now exits with an error if that flag is used (since the library doesn't expose this capability). Update the top-of-file docs so users don't follow an example that can't work.
This issue also appears in the following locations of the same file:
- line 70
- line 96
- line 147
samples/publish_workbook.py:35
- This comment says the
-Uchange avoids an argparse conflict error "at import", but the parser is built insidemain(), so the conflict would occur when the script runs (and builds the parser), not at import time. Updating the wording keeps the guidance accurate.
samples/update_workbook_data_freshness_policy.py:35 first_pageis assigned but never used; using_makes it explicit that onlypagination_itemis needed for the count line.
samples/manage_subscriptions.py:74
SubscriptionItem.on_extract_refresh(...)does not exist in this codebase, socreate --on-extract-refreshwill crash with AttributeError. Either remove this path or fail fast with a clear message until the underlying capability is added to the library.
if args.on_extract_refresh:
# Extract-refresh-triggered: the subscription fires when the referenced
# extract-refresh schedule finishes running the refresh. On Tableau
# Cloud this shows up as schedule type "On Extract Refresh" in the UI.
# `SubscriptionItem.on_extract_refresh` wires up schedule_id and the
samples/manage_subscriptions.py:97
- After removing the extract-refresh code path, this success message still branches on
args.on_extract_refresh, which is now misleading. It should print a single, accurate message for the supported scheduled-subscription flow.
trigger = "on-extract-refresh" if args.on_extract_refresh else "on-schedule"
print(f"Created {trigger} subscription {created.id} " f"for user {created.user_id} against {created.target}")
samples/manage_subscriptions.py:150
- The
--on-extract-refreshhelp text referencescreate_extract_refresh_subscription.py, but that file does not exist insamples/, and the flag is not currently supported by the library (see the failure-fast change inhandle_create). Update the help text so users aren't sent to a dead reference and know what to expect.
"Fire this subscription when the referenced extract-refresh schedule "
"completes, rather than on the schedule's time trigger. --schedule-id "
"must reference an extract-refresh schedule (see create_extract_refresh_"
"subscription.py for the fully worked example)."
samples/explore_favorites.py:66
- The favorite deletions run after the
with server.auth.sign_in(...)block exits, so the client is signed out when these requests are made (and they also assumemy_workbook/my_vieware non-None). Keep the cleanup calls inside the sign-in context and guard them so the sample doesn't crash on empty sites.
server.favorites.delete_favorite_workbook(user, my_workbook)
print(f"Workbook deleted from favorites. Workbook Name: {my_workbook.name}, Workbook ID: {my_workbook.id}")
server.favorites.delete_favorite_view(user, my_view)
print(f"View deleted from favorites. View Name: {my_view.name}, View ID: {my_view.id}")
samples/update_workbook_data_freshness_policy.py:20
- The ArgumentParser description still says this script "Creates sample schedules for each type of frequency", which doesn't match what the script actually does (workbook data freshness policy updates). This makes
--helpoutput misleading.
parser = argparse.ArgumentParser(description="Creates sample schedules for each type of frequency.")
add_common_arguments(parser)
- Files reviewed: 15/15 changed files
- Comments generated: 0 new
- Review effort level: Lite
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>
There was a problem hiding this comment.
🔵 Needs a closer look
The new samples include a couple of concrete credential-resolution and filtering behaviors that can mislead users or fail against some server versions and should be corrected before merging.
Review details
Suppressed comments (4)
Previously missed (3) — in code that hasn't changed since the last review.
samples/_shared.py:217
- If only
TABLEAU_TOKEN_VALUE(or--token-value) is set andtoken_nameis missing, this falls through to the "Fully unspecified" branch and re-prompts for the token value even though it’s already available. Handle the inverse partial PAT case by prompting only for the missing token name.
This issue also appears on line 218 of the same file.
samples/list_jobs.py:94
QuerySet.filter()ultimately serializes datetime values viaFilter(UTC + trailingZ). Passingcutoff.isoformat()bypasses that and can produce+00:00offsets that some Tableau Server versions reject, causing the--hoursfilter to return unexpected results. Pass the timezone-awaredatetimeobject directly.
samples/_shared.py:91- The
--jwthelp text says it’s mutually exclusive with token/username auth, but the parser does not enforce this (andbuild_auth()explicitly defines a precedence order). This is misleading in--help; either enforce mutual exclusion or describe the precedence.
samples/_shared.py:220
- If a password is supplied (env or CLI) without a username, this currently skips the partial-fill logic and drops into the PAT prompt path. Prompt for the missing username (non-secret) the same way you already prompt for the missing password.
if getattr(args, "username", None) and not getattr(args, "password", None):
args.password = getpass.getpass(f"Password for '{args.username}': ")
return
- Files reviewed: 15/15 changed files
- Comments generated: 0 new
- Review effort level: Lite
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>
…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>
There was a problem hiding this comment.
🔵 Needs a closer look
There is at least one confirmed runtime-crash bug in an updated sample (samples/explore_workbook.py uses a loop variable that may be undefined when there are no custom views) and a confirmed documentation mismatch in samples/publish_datasource.py.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
samples/explore_workbook.py:121
cis only defined inside the custom-views loop, but is used afterwards (update/export/delete). If the site has zero custom views, the loop never runs and this will crash with an UnboundLocalError/NameError. Guard the subsequent custom-view operations when no custom views exist (e.g., collect into a list and skip if empty, or use a sentinel and branch).
samples/publish_datasource.py:18- Header comment says this sample "uses personal access tokens" for sign-in, but the code now delegates to
build_auth()and supports JWT and username/password as well. Updating this comment will prevent users from missing supported auth modes.
- Files reviewed: 15/15 changed files
- Comments generated: 0 new
- Review effort level: Lite
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>
There was a problem hiding this comment.
🔵 Needs a closer look
There are a couple of concrete sample-script correctness bugs (notably populate_views() misuse and missing required CLI args) that should be fixed before merging.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
samples/explore_favorites.py:50
server.workbooks.populate_views(...)returns None and populatesmy_workbook.viewsin-place; assigning it toviewsmeans the view-favoriting branch never runs. Usemy_workbook.viewsafter calling populate_views().
samples/move_workbook_sites.py:29--workbook-nameand--destination-siteare optional, but the script unconditionally usesargs.workbook_nameand calls.lower()onargs.destination_site, which will crash when either flag is omitted. Mark both arguments required (and clarify that destination-site expects the site content URL).
- Files reviewed: 15/15 changed files
- Comments generated: 0 new
- Review effort level: Lite
jacalata
added a commit
that referenced
this pull request
Sep 18, 2026
Fold the inline sign-in argparse block into the shared helper landed in #1843. Users of this sample now get env / .env / interactive-prompt credential resolution, JWT + username/password auth in addition to PAT, and a clear ValueError when --server is missing under non-TTY, instead of an opaque 400/401 inside TSC. Also update the header's Python-version claim from 3.7 to 3.10 to match `pyproject.toml`'s `requires-python = ">=3.10"`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Sep 18, 2026
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.
Closes #1551.
Motivation
#1551 flagged three concerns with
samples/: credentials always on thecommand line, several samples using
.get()where they meant to page,and no examples for background jobs or subscriptions. Each of the three
gets one commit for reviewability.
Behavior change
Samples only -- no library changes. Users running the sample scripts
directly will see:
Credentials.
samples/_shared.pyaddsresolve_credentials(args)which fills missing sign-in values from env vars
(
TABLEAU_SERVER,TABLEAU_TOKEN_NAME, etc.), a plain.envfile, orgetpass.getpass(), in that precedence order. CLI args continue towork for CI use. Wired into
login.py,publish_workbook.py, andpublish_datasource.pyto establish the pattern; other samples leftalone to keep the diff surgical. Stdlib only, no new deps.
Short flags on the shared helper match tabcmd's canonical set:
-s--server,-t--site,-u--username,-p--password,-l--logging-level.--token-nameand--token-valueare long-only.An earlier iteration of this PR reassigned
-pto--token-name; thatwas corrected before merge -- see 54e51f5.
Pagination fixes. Several samples called
server.<endpoint>.get()and named the resultall_workbooks, whichonly returns the first page (default 100). Replaces those with
TSC.Pager(...)so every page is walked. Where a total count wasdisplayed we still
.get()once to grabtotal_available; that meansone extra request but preserves the count line.
Also fixes an unrelated bug in
getting_started/3_hello_universe.pywhere the "workbooks" section actually queried datasources.
New samples.
list_jobs.py-- background jobs (extract refreshes, publishes,flow runs) with
.filter()queryset API +wait_for_jobmanage_subscriptions.py-- list/create/delete subscriptions withSubscriptionItem/Targetand paginated listingNot exhaustive on coverage -- data alerts, metrics, tables, databases,
virtual connections still have no dedicated sample. Left for follow-up.
Test plan
samples/ has no automated tests; each check below is manual.
python samples/login.py --helpshows the new flags with updated help textTABLEAU_TOKEN_NAME/TABLEAU_TOKEN_VALUEin env and runningpython samples/login.py -s <server>signs in with no secrets on the CLIpython samples/list_jobs.py --hours 24lists recent jobs;--wait <job_id>blocks until completionpython samples/manage_subscriptions.py listprints existing subs;create+deleteround-trips cleanlyexplore_datasource.py,explore_workbook.py,extracts.py,update_workbook_data_freshness_policy.py, andpublish_workbook.pyreturns correct behavior on a >100-item site🤖 Generated with Claude Code