fix: parse Tableau Cloud's inlined subscription <schedule> (#1627) - #1875
Open
jacalata wants to merge 1 commit into
Open
fix: parse Tableau Cloud's inlined subscription <schedule> (#1627)#1875jacalata wants to merge 1 commit into
jacalata wants to merge 1 commit into
Conversation
Tableau Server references a schedule by id inside a <subscription>:
<schedule id="cfb2..." name="Weekday mornings"/>
Tableau Cloud inlines the schedule instead -- no id attribute, but
a `frequency`, `nextRunAt` and a nested `<frequencyDetails>`:
<schedule frequency="Daily" nextRunAt="2026-08-29T16:55:00-0700">
<frequencyDetails start="16:55:00" end="16:55:00">
<intervals>
<interval hours="24"/>
<interval weekDay="Saturday"/>
</intervals>
</frequencyDetails>
</schedule>
Previously `SubscriptionItem` only pulled `id`/`name` off `<schedule>`,
so on Cloud every subscription came back with `schedule_id == None`
and no structured way to see what the API sent. Client code filtering
`[s for s in subs if s.schedule_id == target]` silently returned `[]`
on Cloud. This is the bug described in issue #1627.
Model changes:
* `SubscriptionItem` now populates a `schedule: ScheduleItem` attribute
for both shapes. On Server, `schedule.id`/`.name` are set (and
`schedule_id` remains populated for back-compat). On Cloud,
`schedule.frequency`, `schedule.next_run_at`, and
`schedule.interval_item` are set. `schedule_id` is `None` on Cloud --
the API does not send one, unavoidable. Docstring calls out the
Cloud-vs-Server discriminator and warns callers who filter by
`schedule_id`. Fixes a latent bug where the previous Cloud branch
assigned a list (return of `ScheduleItem.from_element`) to
`sub.schedule` instead of a single item.
* `ScheduleItem` gains a `frequency` property. The XML attribute was
already being read to select the interval type but was discarded;
now it's exposed so callers can distinguish the Cloud shape without
reaching into the interval object. The class `Attributes` docstring
is rewritten to cover every public property and to note that only
`frequency` / `next_run_at` / `interval_item` are populated when the
item comes from an inlined Cloud subscription schedule.
* `_parse_interval_item` is defensive against malformed Cloud data:
a `<frequencyDetails>` without a `start` attribute no longer crashes
on `strptime(None, ...)`, and an out-of-range `<interval hours="3"/>`
(or unknown weekDay / monthDay) no longer raises `ValueError` out of
`IntervalItem` and poisons sibling schedules in the same page. Bad
data degrades to `interval_item = None` with a warning log.
Datetime plumbing:
* `parse_datetime` learns the Tableau Cloud `%Y-%m-%dT%H:%M:%S%z` form
(e.g. `2026-08-29T16:55:00-0700`) in addition to the Server `...Z`
form. Unparseable input still returns `None` on the read path
(preserving the pre-change contract that a malformed server-side
date cannot crash a page-through of unrelated data).
* `property_is_datetime` now raises `ValueError` when `parse_datetime`
returns `None` for a non-empty str value. Bad *user* input is
surfaced at the assignment site instead of silently nulling the
attribute.
* `TABLEAU_CLOUD_DATE_FORMAT` is public, matching the existing
`TABLEAU_DATE_FORMAT`.
Tests:
* New `test/assets/subscription_get_cloud.xml` mirroring the shape
observed on `stage-dp1` (API 3.29) and matching parse tests for
both Cloud and Server shapes.
* Three edge-case Cloud fixtures + tests: no `<frequencyDetails>`,
empty `<intervals/>`, out-of-set `<interval hours="3"/>`.
* Existing subscription tests extended to cover sub 2 and
`get_subscription_by_id`.
* New `test/test_datetime_helpers.py` with direct coverage of
`parse_datetime` (None / "" / garbage / Server / Cloud / colon-offset
/ microseconds) and `property_is_datetime` (valid / bad / non-str).
* `test/test_schedule.py::test_get` asserts the new
`ScheduleItem.frequency` property on the standard /schedules endpoint.
Refs: #1627
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
A few API/docs/type-safety inconsistencies were introduced (notably around Cloud schedules where schedule_id can be None), and should be corrected before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the client’s subscription and schedule parsing to correctly handle Tableau Cloud’s inline <schedule> shape (no schedule id, but frequency, nextRunAt, and nested <frequencyDetails>), while preserving Tableau Server behavior and improving datetime parsing/validation.
Changes:
- Populate a structured
SubscriptionItem.schedulefor both Server (id/name reference) and Cloud (inline frequency/next-run/intervals) schedule shapes. - Expose
ScheduleItem.frequencyand make interval parsing degrade gracefully (warn +interval_item=None) on malformed Cloud interval payloads. - Expand
parse_datetimeto accept Cloud offset timestamps and makeproperty_is_datetimesetters raise on unparseable strings; add comprehensive fixtures and tests.
File summaries
| File | Description |
|---|---|
| test/test_subscription.py | Adds coverage for Cloud inline schedule parsing and malformed interval degradation, plus asserts structured schedule parsing for Server shape. |
| test/test_schedule.py | Asserts the new ScheduleItem.frequency property on /schedules responses. |
| test/test_datetime_helpers.py | New direct tests for parse_datetime formats and property_is_datetime strict setter behavior. |
| test/assets/subscription_get_cloud.xml | Cloud-shaped subscription fixture with inline schedule and frequency details. |
| test/assets/subscription_get_cloud_no_frequency_details.xml | Cloud fixture with schedule missing <frequencyDetails>. |
| test/assets/subscription_get_cloud_empty_intervals.xml | Cloud fixture with empty <intervals/>. |
| test/assets/subscription_get_cloud_bad_hours.xml | Cloud fixture with invalid interval hours that should degrade without breaking siblings. |
| tableauserverclient/models/subscription_item.py | Adds schedule attribute parsing and documents Server vs Cloud schedule shapes. |
| tableauserverclient/models/schedule_item.py | Adds frequency property and makes interval parsing resilient with warning logs on malformed data. |
| tableauserverclient/models/property_decorators.py | Makes datetime setter decorator strict by raising on unparseable strings. |
| tableauserverclient/datetime_helpers.py | Extends datetime parsing to accept Cloud %z offsets while staying lenient for server-response parsing. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
73
to
+74
| self.user_id = user_id | ||
| self.schedule = None | ||
| self.schedule: Optional[ScheduleItem] = None |
Comment on lines
127
to
128
| Because we return everything with Z as the timezone, we assume everything is in UTC and create | ||
| a timezone aware datetime. |
Comment on lines
+36
to
+40
| The reliable Cloud-vs-Server discriminator on a parsed ``SubscriptionItem`` | ||
| is ``schedule.interval_item is not None`` (Cloud inlines the interval | ||
| detail; Server only sends an ``id``/``name`` reference and this field will | ||
| always be ``None`` there). ``schedule.id is None`` also identifies Cloud | ||
| but only in combination with ``schedule is not None`` -- see below. |
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.
Motivation
Fixes #1627. Tableau Server references a schedule by id inside a
<subscription>:Tableau Cloud inlines the schedule instead — no
id, but afrequency,nextRunAt, and nested<frequencyDetails>.SubscriptionItemonly parsed the Server shape, so on Cloud every subscription came back withschedule_id is Noneand no structured access to the schedule metadata. Client code doing[s for s in subs if s.schedule_id == target]silently returned[]on Cloud.Behavior change
Read side (server response → model):
SubscriptionItem.schedule(new) is populated for both shapes with aScheduleItem. On Server:schedule.idand.nameare set. On Cloud:schedule.frequency,schedule.next_run_at, andschedule.interval_itemare set;schedule_idremainsNoneon Cloud because the API doesn't send one.ScheduleItem.frequency(new property): the underlying XML attribute was already being read to pick the interval type but was discarded — now exposed.parse_datetimeaccepts Cloud's%Y-%m-%dT%H:%M:%S%zform (e.g.2026-08-29T16:55:00-0700) in addition to Server's...Z. Unparseable input still returnsNone.startattribute, or<interval hours="3"/>outsideIntervalItem.VALID_INTERVALS) no longer crashessubscriptions.get(); the offending schedule degrades tointerval_item = Nonewith a warning log, sibling schedules are unaffected.Write side (user assignment → model):
property_is_datetime(setter decorator) now raisesValueErrorwhen a str value fails to parse. Previously the attribute was silently set toNone. This affects direct user assignment to typed datetime properties on model classes; server-response parsing is unchanged.Testing Run
test/assets/subscription_get_cloud.xmlmirrors the shape observed on a live Cloud site (API 3.29). Companion parse tests verify Server and Cloud paths._parse_interval_item's graceful-degradation paths (missing<frequencyDetails>, empty<intervals/>,<interval hours="3"/>).get_subscription_by_id.test/test_datetime_helpers.pydirectly coversparse_datetime(None / "" / garbage / Server / Cloud / colon-offset / microseconds) andproperty_is_datetime(valid / bad / non-str inputs).test/test_schedule.py::test_getasserts the newfrequencyproperty on the standard/schedulesendpoint.main-windows.tsi.lan(API 3.31, Server shape) andstage-dp1(API 3.29, Cloud shape) — matches the two fixture shapes.black --checkandmypyclean.