-
-
Notifications
You must be signed in to change notification settings - Fork 14
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
Run a full sync every 19 minutes #165
Conversation
WalkthroughThe changes update documentation for the Plex Watchlist RSS Feeds, which currently only displays the most recent 50 entries and is noted as broken. The Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
README.md (2)
Line range hint
63-63
: Missing language specification in fenced code block.It's important for clarity and proper rendering on platforms like GitHub to specify the language for code blocks. This block should specify
bash
as it contains shell commands.- ``` + ```bashTools
LanguageTool
[style] ~19-~19: Consider a shorter alternative to avoid wordiness. (IN_ORDER_TO_PREMIUM)
Context: ... | In order to fully integrate with Plex Watchlists, W...
Line range hint
69-69
: Missing language specification in fenced code block.Similar to the previous comment, specifying the language for code blocks enhances readability and proper syntax highlighting.
- ``` + ```bashTools
LanguageTool
[style] ~19-~19: Consider a shorter alternative to avoid wordiness. (IN_ORDER_TO_PREMIUM)
Context: ... | In order to fully integrate with Plex Watchlists, W...
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- README.md (1 hunks)
- src/main/scala/PlexTokenSync.scala (1 hunks)
- src/main/scala/Server.scala (3 hunks)
- src/test/scala/PlexTokenSyncSpec.scala (1 hunks)
Additional context used
LanguageTool
README.md
[style] ~19-~19: Consider a shorter alternative to avoid wordiness. (IN_ORDER_TO_PREMIUM)
Context: ... | In order to fully integrate with Plex Watchlists, W...
Markdownlint
README.md
44-44: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
63-63: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
69-69: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
Additional comments not posted (7)
src/main/scala/Server.scala (4)
2-2
: Updated import statement to support additional parallel operations.The change from
catsSyntaxTuple3Parallel
tocatsSyntaxTuple4Parallel
reflects the addition of another parallel operation in therun
method. This is a necessary change to accommodate the newplexFullSync
function.
28-30
: Addition ofplexFullSync
to the tuple of parallel operations.The inclusion of
plexFullSync
in the parallel operations tuple ensures that the full synchronization process runs concurrently with the other sync processes. This is in line with the PR's objective to run a full sync every 19 minutes.
46-56
: Refactoring and renaming ofplexTokenSync
toplexRssSync
.The function
plexRssSync
has been refactored from the previousplexTokenSync
to focus on RSS-based synchronization. The logic within the function remains largely the same, now explicitly settingrunFullSync
tofalse
for RSS syncs, which is a logical change given the context of the function's renaming and purpose.
57-65
: Introduction of theplexFullSync
function for full synchronization.This function is a new addition and is crucial for achieving the PR's goal of performing a full sync every 19 minutes. It correctly sets
runFullSync
totrue
, ensuring that a full sync is performed, and recursively calls itself after a 19-minute interval, aligning with the PR's objectives.src/main/scala/PlexTokenSync.scala (1)
16-34
: Refactoredrun
function to support conditional full synchronization.The function now handles a
runFullSync
boolean to decide between fetching a complete self watchlist or returning an empty set, which is a significant change to support conditional logic based on full sync requirements. The detailed logging and conditional checks enhance the function's clarity and functionality, aligning with the PR's goals for more reliable synchronization.README.md (1)
15-15
: Updated documentation for Plex Watchlist RSS Feeds.The documentation now correctly reflects that the Plex Watchlist RSS Feeds method is currently broken and only displays the most recent 50 movies/shows. This update is crucial for setting the right expectations for users.
src/test/scala/PlexTokenSyncSpec.scala (1)
26-26
: Updated test case to reflect changes inPlexTokenSync.run
.The test case has been updated to use the
runFullSync
parameter, ensuring that the unit tests are aligned with the changes made in the main codebase. This is crucial for maintaining the reliability of the test suite.
for { | ||
config <- fetchLatestConfig(configRef) | ||
_ <- PlexTokenSync.run(config, httpClient, runFullSync = true) | ||
_ <- IO.sleep(19.minutes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot to add my reasoning for why 19 minutes specifically - it avoids accidental clustering (e.g. if a few servers in the world are set to restart at midnight every night, this gives enough randomness that they won't start hammering Plex every 20 mins)
@nylonee is the 19 min sync the case for plex premium users too? The example in readme says instant, but i cant seem to make it work. |
Description
Since Plex RSS feeds are really flaky, we run a full sync every 19 minutes so that users at least still get the benefits of Watchlistarr without needing to restart.
Checklist
sbt scalafmtAll
Run (and optionallysbt scalafmtSbt
)Summary by CodeRabbit
Documentation
Refactor
Tests