Skip to content

feat: consolidate and export public type aliases (HyperAction, FilePath, AddResponse, IDP types)#1825

Open
jacalata wants to merge 8 commits into
developmentfrom
jac/export-public-types
Open

feat: consolidate and export public type aliases (HyperAction, FilePath, AddResponse, IDP types)#1825
jacalata wants to merge 8 commits into
developmentfrom
jac/export-public-types

Conversation

@jacalata

@jacalata jacalata commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • HyperAction, HyperActionTable, HyperActionRow, HyperActionCondition — needed to annotate the actions argument to datasources.update_hyper_data()
  • FilePath, FileObject, FileObjectR, FileObjectW, PathOrFile, PathOrFileR, PathOrFileW — needed to annotate publish()/download() arguments across datasources, workbooks, flows, and custom views; were duplicated across 4 endpoint files
  • AddResponse — return type of schedules.add_to_schedule(), also used by the same method on datasources, workbooks, and flows
  • IDPAttributes, IDPProperty, HasIdpConfigurationID — needed to annotate arguments to oidc.get_by_id() and oidc.delete_configuration()

All types are now defined in tableauserverclient/types.py and exported from tableauserverclient/__init__.py. The endpoint files import from types.py instead of defining them locally.

Test plan

  • 849 existing tests pass (pytest test/)
  • Types importable directly: from tableauserverclient import HyperAction, FilePath, AddResponse, HasIdpConfigurationID

🤖 Generated with Claude Code

jacalata and others added 8 commits July 21, 2026 22:57
Add Read the Docs configuration file for documentation build
Added configuration settings for Sphinx documentation. Direct copy from example conf file.
Updated project information to load from pyproject.toml.
- Add docs optional-dependencies group (sphinx, tomli) to pyproject.toml
- Wire up .readthedocs.yaml to install .[docs] extra
- Fix conf.py: correct pyproject.toml path, use importlib.metadata for
  version, switch to alabaster theme, use tomllib/tomli compat import
- Add minimal docs/index.rst
- Fix RST docstring errors in connection_item, site_item, job_item,
  task_item that caused Sphinx build warnings/errors

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
On push to master, builds Sphinx HTML and opens a PR from
docs-update into gh-pages so the generated API reference
can be reviewed before going live.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ient.types

HyperAction*, FilePath, FileObject*, PathOrFile*, AddResponse, IDPAttributes,
IDPProperty, and HasIdpConfigurationID were defined locally in endpoint files
(in some cases duplicated across 4 files) but not importable from the top-level
package. Users annotating calls to update_hyper_data(), publish(), download(),
add_to_schedule(), or OIDC methods had to import from internal modules.

Move all shared type aliases to tableauserverclient/types.py and re-export from
tableauserverclient/__init__.py.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Coverage

Coverage Report
FileStmtsMissCoverMissing
tableauserverclient
   __init__.py60100% 
   config.py150100% 
   datetime_helpers.py2511 96%
   exponential_backoff.py200100% 
   filesys_helpers.py310100% 
   namespace.py2633 88%
   types.py200100% 
tableauserverclient/bin
   __init__.py20100% 
   _version.py358212212 41%
tableauserverclient/helpers
   __init__.py10100% 
   logging.py20100% 
   strings.py3111 97%
tableauserverclient/models
   __init__.py460100% 
   collection_item.py4177 83%
   column_item.py553232 42%
   connection_credentials.py351111 69%
   connection_item.py941414 85%
   custom_view_item.py1442121 85%
   data_acceleration_report_item.py5411 98%
   data_alert_item.py15844 97%
   data_freshness_policy_item.py1551515 90%
   database_item.py2073636 83%
   datasource_item.py3001212 96%
   dqw_item.py10455 95%
   exceptions.py40100% 
   extensions_item.py13244 97%
   extract_item.py4444 91%
   favorites_item.py6988 88%
   fileupload_item.py190100% 
   flow_item.py1491010 93%
   flow_run_item.py710100% 
   group_item.py8966 93%
   groupset_item.py4977 86%
   interval_item.py1823232 82%
   job_item.py1871010 95%
   linked_tasks_item.py7911 99%
   location_item.py2922 93%
   metric_item.py1291313 90%
   oidc_item.py6333 95%
   pagination_item.py3411 97%
   permissions_item.py1111212 89%
   project_item.py2073131 85%
   property_decorators.py1001818 82%
   reference_item.py2622 92%
   revision_item.py5911 98%
   schedule_item.py20966 97%
   server_info_item.py3777 81%
   site_item.py6361313 98%
   subscription_item.py10122 98%
   table_item.py1191818 85%
   tableau_auth.py612525 59%
   tableau_types.py2711 96%
   tag_item.py150100% 
   target.py60100% 
   task_item.py5622 96%
   user_item.py3101818 94%
   view_item.py2201616 93%
   virtual_connection_item.py6488 88%
   webhook_item.py6911 99%
   workbook_item.py3621616 96%
tableauserverclient/server
   __init__.py90100% 
   exceptions.py40100% 
   filter.py2111 95%
   pager.py3311 97%
   query.py1431515 90%
   request_factory.py1335195195 85%
   request_options.py38655 99%
   server.py1882323 88%
   sort.py60100% 
tableauserverclient/server/endpoint
   __init__.py350100% 
   auth_endpoint.py771111 86%
   custom_views_endpoint.py1471212 92%
   data_acceleration_report_endpoint.py210100% 
   data_alert_endpoint.py942323 76%
   databases_endpoint.py1113030 73%
   datasources_endpoint.py3303535 89%
   default_permissions_endpoint.py4433 93%
   dqw_endpoint.py451616 64%
   endpoint.py1871818 90%
   exceptions.py7766 92%
   extensions_endpoint.py310100% 
   favorites_endpoint.py942222 77%
   fileuploads_endpoint.py510100% 
   flow_runs_endpoint.py6299 85%
   flow_task_endpoint.py2122 90%
   flows_endpoint.py2125555 74%
   groups_endpoint.py12699 93%
   groupsets_endpoint.py7277 90%
   jobs_endpoint.py6799 87%
   linked_tasks_endpoint.py370100% 
   metadata_endpoint.py881414 84%
   metrics_endpoint.py5566 89%
   oidc_endpoint.py3911 97%
   permissions_endpoint.py4433 93%
   projects_endpoint.py1572424 85%
   resource_tagger.py1273535 72%
   schedules_endpoint.py1181111 91%
   server_info_endpoint.py361010 72%
   sites_endpoint.py1302727 79%
   subscriptions_endpoint.py561414 75%
   tables_endpoint.py1103636 67%
   tasks_endpoint.py6366 90%
   users_endpoint.py18388 96%
   views_endpoint.py15099 94%
   virtual_connections_endpoint.py1131010 91%
   webhooks_endpoint.py5499 83%
   workbooks_endpoint.py3492424 93%
TOTAL11992142788% 

@bcantoni

bcantoni commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Claude Code posted this on my behalf after review:

Overview

The PR title/description says this consolidates five families of previously-duplicated type aliases (HyperAction*, FilePath/FileObject*/PathOrFile*, AddResponse, IDP types) into a new tableauserverclient/types.py, exported from __init__.py. That part is real and well-executed. However, the actual diff (gh pr diff 1825) is much larger than the description implies — it also includes:

  • A new Sphinx/ReadTheDocs setup (.github/workflows/docs.yml, .readthedocs.yaml, docs/conf.py, docs/index.rst, pyproject.toml's new docs extra)
  • ~600 lines of new e2e tests (test_e2e/test_publisher.py, test_e2e/test_site_admin.py) plus binary fixture assets and conftest.py fixture/marker changes
  • Unrelated docstring fixes (job_item.py, task_item.py: id_id; site_item.py tier-capacity wording; connection_item.py XML sample removed from a docstring)

This looks like squashed/rebased history from other branches (commit messages like "Create Sphinx configuration file for documentation," "add more e2e tests," "change theme" appear in git log for this PR head but aren't attributed to any other open/merged PR). Before merging, confirm with the author whether development is missing commits this branch already has, or whether this branch accidentally bundled unrelated work — as-is, reviewing "type alias consolidation" together with a docs pipeline and a large e2e suite makes it hard to review each concern on its own merits, and a revert of one would be entangled with the others.

What's good about the core type-consolidation change

  • tableauserverclient/types.py is a clean leaf module — no internal imports, so no circular-import risk from moving these definitions out of per-endpoint files and out of TYPE_CHECKING-guarded blocks.
  • Confirmed real, not cosmetic: AddResponse was previously defined in schedules_endpoint.py and imported via TYPE_CHECKING-only forward references in datasources_endpoint.py, workbooks_endpoint.py, and flows_endpoint.py — a real duplication smell now fixed.
  • IDPAttributes/IDPProperty/HasIdpConfigurationID and the Hyper-action TypedDicts move cleanly with no behavior change.
  • Verified locally: black --check, mypy (with the repo's CI flags), and pytest test -n auto all pass — 849 passed, 1 skipped, matching the PR's stated test plan.

Issues found

  1. Dead imports introduced across nearly every touched endpoint file. Several endpoint files now import names from tableauserverclient.types that are unused in that file (confirmed via pyflakes):

    • custom_views_endpoint.py: FilePath, FileObject, FileObjectR, FileObjectW all unused (only PathOrFileR/PathOrFileW are used).
    • datasources_endpoint.py: FileObject, FileObjectR, PathOrFile, PathOrFileW, HyperActionCondition, HyperActionRow, HyperActionTable all unused.
    • workbooks_endpoint.py: FileObject, FileObjectR, PathOrFileW unused.
    • flows_endpoint.py: FilePath, FileObjectR, FileObjectW unused.
    • oidc_endpoint.py: IDPAttributes, IDPProperty unused (only HasIdpConfigurationID is used).

    To be fair, most of these were already dead as locally-defined-but-unused type aliases before this PR (mypy/black don't flag unused module-level assignments the way they flag unused imports), so this PR doesn't regress runtime behavior — but converting them to imports makes the dead code visible to linters like pyflakes/ruff for the first time, and it's a quick cleanup to just drop the unused names from each from tableauserverclient.types import (...) block while you're already touching every one of these import lines.

  2. No test coverage for the new public export surface. The PR description's "test plan" mentions manually verifying from tableauserverclient import HyperAction, FilePath, AddResponse, HasIdpConfigurationID works, but there's no automated test (e.g., under test/models/ or a new test/test_types.py) asserting these names are importable from the top-level package and stay in __all__. Since this PR's whole purpose is establishing a public API surface, a regression test that imports every new name from tableauserverclient (not just tableauserverclient.types) would guard against someone later reordering __init__.py or renaming something in types.py.

  3. No CHANGELOG entry. Per repo convention (CHANGELOG.md gets a flat bullet per user-visible PR, ideally with the PR number), this newly-exported public API surface should get an entry — this is the kind of change users may rely on afterward (e.g. for type-checking their own update_hyper_data() calls), so it's worth documenting explicitly rather than leaving users to discover it from source.

  4. Scope/hygiene concerns (see Overview):

    • The e2e test additions (test_publisher.py, test_site_admin.py) and the two new binary fixtures (.twbx, .tdsx) add real repo size/maintenance surface and aren't mentioned in the PR description at all — worth a sanity check that they're intentional here and not meant for PR test(e2e): add e2e tests for workbooks, datasources, views, pagination, jobs, tagging, projects, favorites, metadata, site-admin #1823 (jac/e2e-tests), which appears to cover similar/overlapping ground (workbooks, datasources, site-admin, favorites, tagging, metadata).
    • The Sphinx/docs pipeline is a meaningful new CI surface (a workflow with contents: write/pull-requests: write permissions that pushes to gh-pages via peter-evans/create-pull-request) that deserves its own dedicated review, ideally separate from a type-refactor PR.
    • The docstring tweaks to job_item.py/task_item.py/site_item.py/connection_item.py are harmless improvements but have nothing to do with type consolidation; worth splitting out if this PR gets re-scoped.

Security

Nothing security-sensitive in the type-consolidation change itself. The new docs.yml workflow grants contents: write and pull-requests: write and runs on push: branches: [master] — this is fine since it only ever builds and PRs into gh-pages, but it's a new automated write path worth a second look if the docs work is reviewed independently.

Recommendation

The type-consolidation refactor itself is sound (verified by tests/mypy/black) and worth merging, but I'd ask the author to confirm the diff scope before approval — specifically, whether this branch should be rebased to drop the docs/e2e/docstring commits so this PR reviews cleanly as "consolidate and export public type aliases," matching its title and description.

@bcantoni

Copy link
Copy Markdown
Contributor

@jacalata see the things claude code pointed out above -- in particular are we sure the "docs" changes should be in here? (Docs are still in gh-pages branch.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants