Add scoped library schedule modes - #3466
Conversation
|
Docker image pushed: |
badja-dev
left a comment
There was a problem hiding this comment.
Two things to possible settle before merge, neither causing the biggest headache:
- Schedule list is evaluated twice (config.py at load time, library.py at run time) with two different current_time sources. Worth consolidating to one evaluation
- Scope enforcement is spread across three call sites in the Plex/builder code. Would like to see this run against a real library with a list-based collection (imdb_list, trakt_list, etc) on a scoped schedule to confirm nothing outside scheduled_item_keys sneaks through
There was a problem hiding this comment.
library._all_items is accessed directly here to build the cache snapshot, reaching into a "private" attribute on the Plex subclass from kometa.py. Works fine since Plex is the only Library subclass, but might be worth a small accessor method on Library instead, for cleanliness
There was a problem hiding this comment.
Addressed. I added Library.get_cached_items() as the public accessor for the unscoped per-run snapshot, and replaced the direct _all_items access in kometa.py.
There was a problem hiding this comment.
filter_and_save_items guards with getattr(self, "library", None) before checking schedule scope. Is there a real path where a builder has no library set, or was this just to satisfy a test fixture?
If it's just for the test, might be cleaner to fix the fixture instead of adding a runtime guard for a case that can't happen in a real run
There was a problem hiding this comment.
There is a real no-library path: playlist builders. I replaced the generic runtime guard with the explicit not self.playlist condition, and fixed the non-playlist test fixture so it supplies the required library.
There was a problem hiding this comment.
The schedule list gets evaluated here to decide skip_library (using current_time), then evaluated again in library._schedule_item_keys (using a fresh datetime.now()) to pick the actual mode.
Basically, can we calculate the mode once and pass it through, so there is one source of truth?
There was a problem hiding this comment.
Addressed. Config loading now selects the first matching schedule mode using the existing current_time, stores it in params["schedule_mode"], and Library only consumes that selected value. It no longer re-evaluates schedules with a separate clock source.
There was a problem hiding this comment.
Scope is enforced in get_all, search, exact_search, and filter_and_save_items.
With how cumbersome builder.py is, is there a fetch path (direct rating-key lookup, list-based collection sources, etc) that could pull in an item outside scheduled_item_keys and skip these checks?
There was a problem hiding this comment.
Added a regression test for the direct ratingKey path used after list builders resolve IDs. It fetches an item outside scheduled_item_keys and verifies filter_and_save_items excludes it. This is the shared final path for list-derived IDs, including IMDb/Trakt list results. I haven’t run it against a live Plex library from this environment.
There was a problem hiding this comment.
Added a regression test for the direct ratingKey path used after list builders resolve IDs. It fetches an item outside scheduled_item_keys and verifies filter_and_save_items excludes it. This is the shared final path for list-derived IDs, including IMDb/Trakt list results. I haven’t run it against a live Plex library from this environment.
… into library-filters # Conflicts: # CHANGELOG.md
|
@badja-dev to re-review please |
What type of PR is this?
Description
Adds library schedule modes using an explicit YAML list of
scheduleandmodemappings.Schedule modifiers
.notweekly.not(monday)matches every day except Monday.Modes
fulladded(days)diffdiff_episodeindex(A-F)index(A-F#)Example configuration
Related Issues [optional]
Have you updated the Documentation to reflect changes (if necessary)?
Have you updated the JSON Schema files (if necessary)?
Have you updated the CHANGELOG.md?