-
Notifications
You must be signed in to change notification settings - Fork 445
Add structural Protocol types for TSC item classes #1802
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
jacalata
wants to merge
13
commits into
development
Choose a base branch
from
jac/base-item-class
base: development
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
f87287e
feat: add BaseItem protocol for item type safety
jacalata c3a8043
feat: add OwnedItem and TaggableItem protocols to base_item.py
jacalata 14fa834
Fix review findings: name collision, MetricItem coverage, _initial_ta…
jacalata 3f3339b
Remove dead Taggable protocol; fix duplicate SiteOIDCConfiguration in…
jacalata 5389b6c
Declare OwnedItem.owner_id as read-only @property
jacalata 84a0e29
Add protocol tests and annotate _ResourceTagger methods
jacalata af7d955
style: add grouping comments to __all__ and hoist datetime import in …
jacalata 506ae8c
rename BaseItem -> TableauItem Protocol, replace Union in tableau_typ…
jacalata 020d610
Merge origin/development into jac/base-item-class
jacalata dab0c5b
docs+test: address adversarial review findings on Protocol types
jacalata 4e6ead1
docs: note TableauItem Union -> Protocol behavior change in CHANGELOG
jacalata 8066acc
Merge remote-tracking branch 'origin/development' into jac/base-item-…
jacalata 2c27bc6
Fix Copilot review: _TaggableWithInitial.id read-only + accurate CHAN…
jacalata File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| """Structural protocols for TSC item classes. | ||
|
|
||
| These protocols define the minimum interface shared across TSC resource items. | ||
| They use ``typing.Protocol`` (structural subtyping) rather than an ABC so that | ||
| existing classes do not need to modify their inheritance chain to satisfy the | ||
| contract. Any class that exposes the required attributes satisfies the | ||
| protocol automatically -- no explicit inheritance is required or desired. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import datetime | ||
| from typing import Protocol, runtime_checkable | ||
|
|
||
|
|
||
| @runtime_checkable | ||
| class TableauItem(Protocol): | ||
| """Structural interface satisfied by TSC resource item classes that carry an id and a name. | ||
|
|
||
| Structurally satisfied by the primary content classes (WorkbookItem, | ||
| DatasourceItem, ViewItem, FlowItem, MetricItem), user/group/project/schedule/ | ||
| site/webhook/custom-view/table/database/virtual-connection items, and any | ||
| other class exposing ``id`` and ``name``. Items without a ``name`` attribute | ||
| (e.g. TaskItem, DataAlertItem) do NOT satisfy this protocol. | ||
|
|
||
| Notes | ||
| ----- | ||
| ``id`` and ``name`` are declared as read-only ``@property`` so that concrete | ||
| classes with narrower return types (e.g. ``name: str``) satisfy the protocol | ||
| under mypy's covariant property checking. Plain writable instance attributes | ||
| also satisfy a read-only property protocol. | ||
|
|
||
| ``runtime_checkable`` enables ``isinstance(obj, TableauItem)`` checks at | ||
| runtime, but these only verify attribute *presence*, not types or | ||
| signatures. Full static checking requires a type checker such as mypy. | ||
| ``issubclass`` is NOT supported for data-attribute Protocols and will raise | ||
| ``TypeError`` -- use ``isinstance`` on an instance instead. | ||
|
|
||
| ``from_response`` is intentionally excluded from this protocol because the | ||
| primary content classes have divergent signatures (different ``resp`` | ||
| parameter types, extra parameters) that cannot be unified without widening | ||
| to ``Any``. | ||
| """ | ||
|
|
||
| @property | ||
| def id(self) -> str | None: ... | ||
|
|
||
| @property | ||
| def name(self) -> str | None: ... | ||
|
|
||
|
|
||
| @runtime_checkable | ||
| class OwnedItem(TableauItem, Protocol): | ||
| """Structural interface for TSC items that carry an owner reference. | ||
|
|
||
| Structurally satisfied by WorkbookItem, DatasourceItem, ViewItem, | ||
| FlowItem, ProjectItem, and MetricItem -- every item class that exposes | ||
| an ``owner_id`` attribute. Extends ``TableauItem``. | ||
|
|
||
| No concrete class needs to explicitly inherit from OwnedItem. Protocol | ||
| structural subtyping means any class that exposes the required attribute | ||
| satisfies the protocol implicitly. | ||
|
|
||
| ``owner_id`` is declared as a read-only ``@property`` so that ViewItem | ||
| (whose owner is determined by its parent workbook and is not independently | ||
| writable) satisfies the protocol. Plain writable instance attributes on | ||
| other item classes also satisfy a read-only property protocol. | ||
| """ | ||
|
|
||
| @property | ||
| def owner_id(self) -> str | None: ... | ||
|
|
||
|
|
||
| @runtime_checkable | ||
| class TaggableItem(TableauItem, Protocol): | ||
| """Structural interface for TSC items that carry a mutable tag set. | ||
|
|
||
| Structurally satisfied by WorkbookItem, DatasourceItem, ViewItem, | ||
| FlowItem, and MetricItem. ProjectItem is intentionally excluded because | ||
| it does not expose a ``tags`` attribute. | ||
| """ | ||
|
|
||
| tags: set[str] | ||
|
|
||
|
|
||
| @runtime_checkable | ||
| class ContentItem(OwnedItem, TaggableItem, Protocol): | ||
| """Extended interface for publishable content items. | ||
|
|
||
| Composes OwnedItem (carries ``owner_id``), TaggableItem (carries ``tags``), | ||
| and adds server-assigned timestamps. Structurally satisfied by | ||
| WorkbookItem, DatasourceItem, ViewItem, FlowItem, and MetricItem. | ||
|
|
||
| No concrete class needs to explicitly inherit from ContentItem. Protocol | ||
| structural subtyping means any class that exposes all required attributes | ||
| satisfies the protocol implicitly, avoiding mypy [override] errors that | ||
| arise when a Protocol with plain writable annotations is explicitly | ||
| subclassed by a class that implements them as read-only properties. | ||
| """ | ||
|
|
||
| created_at: datetime.datetime | None | ||
| updated_at: datetime.datetime | None |
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
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
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.