Deprecated api remove #196
Conversation
remove deprecated
update deprecated api
WalkthroughThe changes involve modifications to 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
src/main/scala/sonarr/SonarrUtils.scala (1)
85-88: Consider extracting common URL construction logicThe URL construction logic is identical between RadarrUtils and SonarrUtils. Consider extracting this into a shared utility trait to promote DRY principles.
trait ArrBaseUtils { protected def buildUrlWithParams(baseUrl: Uri, endpoint: String, params: Map[String, Any]): Uri = params.foldLeft(baseUrl / "api" / "v3" / endpoint) { case (url, (key, value)) => url.withQueryParam(key, value.toString) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/main/scala/radarr/RadarrUtils.scala(2 hunks)src/main/scala/sonarr/SonarrUtils.scala(2 hunks)
🔇 Additional comments (4)
src/main/scala/radarr/RadarrUtils.scala (3)
68-73: Clean implementation of dynamic query parameters
The implementation is well-structured and type-safe. Good use of foldLeft for building the URL with query parameters.
75-79: LGTM: Error handling remains robust
The response handling and decoding logic maintains proper error handling while accommodating the new URL construction.
28-28: Verify pagination behavior with pageSize=-1
While using pageSize=-1 to fetch all records in one request is convenient, it could impact performance with large datasets. Verify that:
- The new API endpoint supports this usage
- There are no memory constraints when dealing with large exclusion lists
✅ Verification successful
Let's examine the actual usage of pageSize=-1 in both files to understand the pattern better.
Let's check if there are any other pagination patterns or pageSize usages in the codebase to understand if this is a consistent approach.
Let's check the implementation of getToArr to understand how it handles the response and if there are any safeguards for large datasets.
Let's try to find the getToArr implementation using ripgrep instead.
Based on the complete context gathered, I can now generate the final response:
Using pageSize=-1 with paged endpoints appears to be a consistent pattern
The codebase shows this is an established pattern used consistently in both Radarr and Sonarr implementations:
- Both services use identical pagination approach for their exclusion endpoints
- The
getToArrimplementation handles responses safely usingEitherTfor error handling - The response is processed lazily through IO monad, which helps manage memory efficiently
- This pattern aligns with the API's v3 specification for both services
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other instances of pageSize=-1 to understand if this is a common pattern
rg -l "pageSize.*-1"
Length of output: 102
Script:
#!/bin/bash
# Show the context of pageSize=-1 usage in both files
rg "pageSize.*-1" -B 2 -A 2
Length of output: 986
Script:
#!/bin/bash
# Search for other pagination patterns and pageSize usage
rg "pageSize.*\d+" -B 2 -A 2
# Also check for any paged endpoint usage
rg "/paged" -B 2 -A 2
Length of output: 1966
Script:
#!/bin/bash
# Search for getToArr implementation
ast-grep --pattern 'def getToArr'
Length of output: 35
Script:
#!/bin/bash
# Search for getToArr implementation using ripgrep with context
rg "getToArr" -B 3 -A 10
Length of output: 5627
src/main/scala/sonarr/SonarrUtils.scala (1)
27-27: Consistent with RadarrUtils implementation
The change to use the paginated endpoint with pageSize=-1 mirrors the RadarrUtils implementation. Consider the same performance considerations mentioned in RadarrUtils.
misscopypasted Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
src/main/scala/sonarr/SonarrUtils.scala (1)
85-88: Add type safety for query parameter valuesThe current implementation calls
toStringon any value type, which could produce unexpected results for complex types. Consider adding type constraints or validation for parameter values.- )(baseUrl: Uri, apiKey: String, endpoint: String, params: Map[String, Any] = Map.empty): EitherT[IO, Throwable, T] = { + )(baseUrl: Uri, apiKey: String, endpoint: String, params: Map[String, String] = Map.empty): EitherT[IO, Throwable, T] = { val urlWithParams = params.foldLeft(baseUrl / "api" / "v3" / endpoint) { - case (url, (key, value)) => url.withQueryParam(key, value.toString) + case (url, (key, value)) => url.withQueryParam(key, value) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/main/scala/sonarr/SonarrUtils.scala(2 hunks)
🔇 Additional comments (2)
src/main/scala/sonarr/SonarrUtils.scala (2)
85-94: LGTM! Clean and functional implementation
The updated getToArr method implementation with dynamic query parameter support is well-structured and maintains good functional programming practices.
27-27: Consider pagination handling for large datasets
While using pageSize=-1 fetches all exclusions in one request, this approach might cause performance issues or timeouts with large datasets. Consider implementing proper pagination handling.
Let's check if there are any timeout configurations or similar patterns in the codebase:
| @@ -24,7 +24,7 @@ trait SonarrUtils extends SonarrConversions { | |||
| if (bypass) { | |||
| EitherT.pure[IO, Throwable](List.empty[SonarrSeries]) | |||
| } else { | |||
| getToArr[List[SonarrSeries]](client)(baseUrl, apiKey, "importlistexclusion") | |||
| getToArr[List[SonarrSeries]](client)(baseUrl, apiKey, "importlistexclusion/paged", Map("pageSize" -> "-1")) | |||
There was a problem hiding this comment.
💡 Codebase verification
Based on the test specifications and usage patterns, I can now generate the final response:
API endpoint change appears to be inconsistent
The code changes the endpoint from importlistexclusion to importlistexclusion/paged, but the test specifications consistently use /api/v3/importlistexclusion without the /paged suffix. This inconsistency suggests the change might not be compatible with the API specification.
src/test/scala/sonarr/SonarrUtilsSpec.scalauses/api/v3/importlistexclusionsrc/test/scala/PlexTokenSyncSpec.scalauses/api/v3/importlistexclusion- The test data in
importlistexclusion.jsonis structured for the non-paged endpoint
🔗 Analysis chain
Verify API endpoint compatibility
Please ensure that the new importlistexclusion/paged endpoint is part of the official Sonarr API v3 specification.
Let me gather more information about the API endpoint by checking the test data and any documentation files.
Let me check the test specifications to understand how the API endpoint is being used in tests.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any other occurrences of the old or new endpoint
rg -i "importlistexclusion"
Length of output: 941
Script:
#!/bin/bash
# Check the test JSON file to understand the API structure
cat src/test/resources/importlistexclusion.json
# Look for any API documentation files
fd -e md -e txt -e yaml -e json . | grep -i "api\|swagger\|openapi" || true
Length of output: 197
Script:
#!/bin/bash
# Check the test specifications for API endpoint usage
cat src/test/scala/sonarr/SonarrUtilsSpec.scala
cat src/test/scala/PlexTokenSyncSpec.scala
Length of output: 12144
Description
I've updated work with *arr to non deprecated api
Checklist
sbt scalafmtAllRun (and optionallysbt scalafmtSbt)Summary by CodeRabbit
New Features
Bug Fixes
Documentation