-
-
Notifications
You must be signed in to change notification settings - Fork 30
fix: Update Plex requests #226
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe PR adjusts Plex synchronization polling intervals and introduces randomized delays to reduce server load. Default polling interval increased from 10 to 120 seconds (range 60–300 seconds). Jittered sleep durations added in Server.scala and PlexUtils.scala; cache-buster UUID removed and paging size reduced from 300 to 100. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
🧹 Nitpick comments (2)
src/main/scala/plex/PlexUtils.scala (2)
86-91: Consider using cats-effectRandomfor consistency.This code uses
scala.util.Randomdirectly, whileServer.scalausescats.effect.std.Random.scalaUtilRandom[IO]. For consistency and referential transparency, consider aligning with the cats-effect approach:- for { - // We should be considerate with the Plex servers - randomSeconds <- EitherT.liftF(IO(Random.nextInt(11))) - _ <- EitherT.liftF(IO.sleep(3.seconds + randomSeconds.seconds)) - next <- getSelfWatchlist(config, client, containerStart + containerSize) - } yield next + for { + // We should be considerate with the Plex servers + rand <- EitherT.liftF(Random.scalaUtilRandom[IO]) + randomSeconds <- EitherT.liftF(rand.nextIntBounded(11)) + _ <- EitherT.liftF(IO.sleep(3.seconds + randomSeconds.seconds)) + next <- getSelfWatchlist(config, client, containerStart + containerSize) + } yield nextThe same applies to
getWatchlistIdsForUserat lines 187-192.
187-192: Delay logic is consistent withgetSelfWatchlist.The same delay pattern is applied here, which is good for consistency. Consider extracting the delay logic into a shared helper method to reduce duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/resources/config-template.yaml(1 hunks)src/main/scala/Server.scala(2 hunks)src/main/scala/configuration/ConfigurationUtils.scala(1 hunks)src/main/scala/plex/PlexUtils.scala(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-06-10T19:44:23.144Z
Learnt from: nylonee
Repo: nylonee/watchlistarr PR: 57
File: src/test/scala/configuration/ConfigurationUtilsSpec.scala:121-130
Timestamp: 2024-06-10T19:44:23.144Z
Learning: The expected result for the test case that verifies the splitting of multiple RSS watchlist feeds is based on the mock HTTP client's response, which may contain a single `RSSInfo` object, rather than directly using the `plexWatchlists` configuration provided. This understanding is crucial for accurately assessing the expected outcomes in test cases.
Applied to files:
src/main/scala/plex/PlexUtils.scala
🧬 Code graph analysis (2)
src/main/scala/configuration/ConfigurationUtils.scala (2)
src/main/scala/configuration/FileAndSystemPropertyReader.scala (1)
getConfigOption(52-58)src/main/scala/configuration/SystemPropertyReader.scala (1)
getConfigOption(4-4)
src/main/scala/Server.scala (3)
src/main/scala/PlexTokenSync.scala (2)
PlexTokenSync(12-99)run(16-57)src/main/scala/PlexTokenDeleteSync.scala (1)
run(16-47)src/main/scala/PingTokenSync.scala (1)
run(7-7)
🔇 Additional comments (5)
src/main/resources/config-template.yaml (1)
5-8: LGTM!The updated interval range (60-300 seconds) and default value (120 seconds) are consistent and align with the PR objective to be more considerate with Plex servers.
src/main/scala/configuration/ConfigurationUtils.scala (1)
31-31: LGTM!The default interval of 120 seconds is consistent with the config template update.
src/main/scala/plex/PlexUtils.scala (2)
73-73: LGTM!Reducing the container size to 100 combined with the pagination delays should be more considerate to Plex servers.
5-5: Verify ifparSequenceNimport is used.The
parSequenceNimport on line 5 doesn't appear in the visible code. Confirm whether this was added intentionally for future use or should be removed.src/main/scala/Server.scala (1)
52-57: LGTM - jittered sleep correctly implements the PR objective.The sleep duration ranges from
refreshIntervalto4 * refreshIntervalas specified. With the default 120-second interval, this means 2-8 minutes between RSS syncs.One minor optimization:
Random.scalaUtilRandom[IO]creates a newRandominstance on each iteration. Consider caching it, though the overhead is negligible for this use case.
Description
interval.secondsto120in the config templaterefreshInterval&4 * refreshIntervalcache_busterparameter (has no effect anymore - prevents a 302 redirect)100PS: The code was edited using an AI agent. Please point out any mistakes.
Checklist
sbt scalafmtAllRun (and optionallysbt scalafmtSbt)Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.