Allow multiple localhost types to try for sonarr/radarr config #114
Conversation
WalkthroughThe recent updates enhance URL validation and selection logic for Sonarr and Radarr base URLs by introducing a method to find the correct URLs from a list of local hosts. These changes aim to improve connectivity and resolve issues related to default or incorrect URL settings, ensuring smoother operations for the container with Sonarr and Radarr services. Changes
Possibly related issues
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (3)
- src/main/scala/configuration/ConfigurationUtils.scala (3 hunks)
- src/main/scala/configuration/test.sc (1 hunks)
- src/test/scala/configuration/ConfigurationUtilsSpec.scala (1 hunks)
Additional Context Used
Learnings (1)
src/test/scala/configuration/ConfigurationUtilsSpec.scala (1)
User: nylonee" PR: nylonee/watchlistarr#57 File: src/test/scala/configuration/ConfigurationUtilsSpec.scala:121-130 Timestamp: 2023-12-17T22:23:45.326Z 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.
Additional comments not posted (4)
src/main/scala/configuration/test.sc (1)
3-9: Consider adding a comment explaining the purpose of including URLs with and without the "http://" prefix to clarify the intent for future maintainers.src/test/scala/configuration/ConfigurationUtilsSpec.scala (1)
204-215: The mock HTTP requests for Sonarr and Radarr API health checks are correctly set up. Consider adding comments explaining the choice ofIO.pure(Right(Json.Null))as the return value to simulate a successful but empty response, which might not be immediately clear to future maintainers.src/main/scala/configuration/ConfigurationUtils.scala (2)
21-27: The list of possible local hosts includes URLs with and without the "http://" prefix. Adding a comment explaining the inclusion of both formats could clarify the intent for future maintainers, ensuring they understand the purpose of this variety.
84-117: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [105-151]
The modifications to the
getSonarrConfigandgetRadarrConfigmethods to utilize thefindCorrectUrlmethod for dynamic URL selection are well-implemented. Consider adding comments explaining how these methods now dynamically select the most appropriate base URL for connecting to Sonarr and Radarr services, which enhances the application's flexibility and reliability.
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (4)
- CONFIGURATION.md (1 hunks)
- README.md (2 hunks)
- src/main/resources/config-template.yaml (2 hunks)
- src/main/scala/configuration/ConfigurationUtils.scala (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/main/scala/configuration/ConfigurationUtils.scala
Additional comments not posted (8)
src/main/resources/config-template.yaml (2)
18-18: LGTM! The update from "127.0.0.1:8989" to "localhost:8989" for Sonarr'sbaseUrlincreases flexibility in handling localhost URLs.
33-33: LGTM! The update from "127.0.0.1:7878" to "localhost:7878" for Radarr'sbaseUrlenhances the application's ability to connect under various configurations.README.md (2)
29-29: The clarification that the feature is disabled by default improves transparency and user understanding.
45-45: Redirecting users toCONFIGURATION.mdfor Docker and Java setup instructions helps keep the README concise and focused on high-level information.CONFIGURATION.md (4)
4-42: The Docker configuration instructions are clear and provide comprehensive options for setting up Watchlistarr, enhancing user flexibility.
28-42: The instructions for enabling debug mode in Docker are straightforward and provide clear guidance for users.
118-133: The Docker Environment Variable Configuration section offers a clear alternative for users, enhancing the accessibility of the application's configuration.
144-216: The Java configuration instructions are detailed and provide comprehensive guidance for setting up Watchlistarr in a Java environment, enhancing user understanding and flexibility.
There was a problem hiding this comment.
Actionable comments posted: 2
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (2)
- CONFIGURATION.md (1 hunks)
- src/main/scala/configuration/FileAndSystemPropertyReader.scala (1 hunks)
Additional comments not posted (10)
src/main/scala/configuration/FileAndSystemPropertyReader.scala (1)
17-17: Consider adding a comment explaining the default behavior when theconfigPathsystem property is not set, for clarity.CONFIGURATION.md (9)
9-9: Consider rephrasing to: "Note: If you are adding new environment variables or other configurations, add them before thenylonee/watchlistarrpart."
11-11: Consider using "can" for conciseness: "In most consoles, you can add a \ to denote a breakline..."
20-20: Consider rephrasing for clarity: "For the rest of the guide, we will use breaklines. Please ensure your console is compatible with the \ breakline character."
29-29: Consider using a more formal tone: "To enable debug mode in Docker, you may add the following code to your command before thenylonee/watchlistarrpart:"
57-57: Simplify for clarity: "To use the config.yaml configuration, add a new line to your Docker command:"
94-94: Simplify for clarity: "To make a configuration change, uncomment the specific line and change its value."
107-107: Consider using "can" for conciseness: "Note: You can simplify your Docker command if you wish to provide the API keys and Plex token within the configuration instead of via environment variables."
143-143: Clarify the sentence: "If you are unsure about what a configuration does, the best place to look is the config.yaml template file."
202-202: Add a comma for clarity: "However, since we are running the application natively, we need to provide this manually:"
| @@ -0,0 +1,233 @@ | |||
| # Getting Started With Configuration | |||
| There are two main ways to run Watchlistarr. The recommended (and simpler way) is to use Docker. For Windows, this is called Docker for Windows, and comes with a nice UI to manage your docker applications, including starting and stopping multiple applications. | |||
There was a problem hiding this comment.
Consider correcting the spelling mistake: "configuration" to "Configuration".
| This tells Docker that you have a file called `config` on your computer, that you'd like to point to the application's `app/config` location. | ||
| If this file doesn't yet exist on your computer, Watchlistarr will automatically create it with a nice template for you to use. | ||
|
|
||
| Once this command runs successfully, you will see the `config.yaml` file in the same directory as where you ran the command from. Open the `config.yaml` file using your favourite text editor, and let's look at some of the contents: |
There was a problem hiding this comment.
Correct the spelling to American English for consistency: "favourite" to "favorite".
No description provided.