Config files #97
Config files #97
Conversation
|
Warning Rate Limit Exceeded@nylonee has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 28 minutes and 59 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe project has seen updates across configuration management, build dependencies, and Docker optimization. Notably, the handling of configuration settings has been refined with a new approach to reading from both system properties and files, improving flexibility and reliability. Additionally, a dependency on SnakeYAML has been introduced for YAML parsing, and the Dockerfile has been optimized for a more efficient build process. Changes
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.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
src/main/resources/config-template.yamlis excluded by:!**/*.yaml
Files selected for processing (6)
- .gitignore (1 hunks)
- build.sbt (2 hunks)
- docker/Dockerfile (1 hunks)
- src/main/scala/Server.scala (2 hunks)
- src/main/scala/configuration/FileAndSystemPropertyReader.scala (1 hunks)
- src/main/scala/configuration/SystemPropertyReader.scala (1 hunks)
Additional comments: 4
src/main/scala/configuration/SystemPropertyReader.scala (1)
- 4-4: The implementation of
getConfigOptioncorrectly overrides the method fromConfigurationReaderand safely wraps the system property lookup in anOption, which is a good practice for handling potentially null values from Java APIs in Scala.build.sbt (1)
- 19-25: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [22-44]
The addition of SnakeYAML version 2.0 as a dependency in
build.sbtaligns with the PR's objectives of enhancing configuration management through YAML file parsing. Ensure that this version is compatible with the project's Scala version and other dependencies.src/main/scala/Server.scala (2)
- 4-4: Replacing
SystemPropertyReaderwithFileAndSystemPropertyReaderin the import statement aligns with the PR's objectives of enhancing configuration management by reading from both system properties and YAML files.- 21-30: The changes to directly return
IO[Configuration]and update method parameters to acceptRef[IO, Configuration]are good practices in functional programming. They simplify the code and improve the handling of real-time configuration updates, aligning with the PR's objectives.
| private lazy val data: Map[String, String] = { | ||
| val yaml = new Yaml() | ||
| val configDirPath = "config" | ||
| val configFile = new File(s"$configDirPath/config.yaml") | ||
|
|
||
| try { | ||
| // Ensure parent config folder exists | ||
| val parentDir = configFile.getParentFile | ||
| if (!parentDir.exists()) parentDir.mkdirs() | ||
|
|
||
| if (!configFile.exists()) { | ||
| val resourceStream = getClass.getClassLoader.getResourceAsStream("config-template.yaml") | ||
| if (resourceStream != null) { | ||
| try { | ||
| Files.copy(resourceStream, Paths.get(configFile.toURI), StandardCopyOption.REPLACE_EXISTING) | ||
| logger.info(s"Created config file in ${configFile.getPath}") | ||
| } finally { | ||
| resourceStream.close() | ||
| } | ||
| } else { | ||
| logger.debug("config-template.yaml resource not found") | ||
| } | ||
| } | ||
|
|
||
| if (configFile.exists()) { | ||
| val inputStream = new FileInputStream(configFile) | ||
| val result = yaml.load[util.Map[String, Object]](inputStream).asScala | ||
| inputStream.close() | ||
| flattenYaml(Map.from(result)) | ||
| } else { | ||
| Map.empty[String, String] | ||
| } | ||
| } catch { | ||
| case e: Exception => | ||
| logger.debug(s"Failed to read from config.yaml: ${e.getMessage}") | ||
| Map.empty[String, String] | ||
| } | ||
| } |
There was a problem hiding this comment.
The implementation of configuration reading from YAML files and system properties in FileAndSystemPropertyReader is well-structured and aligns with the PR's objectives. Consider using Scala's Using trait or a similar pattern for resource management to ensure that resources like file streams are always properly closed, even in the case of exceptions.
- val inputStream = new FileInputStream(configFile)
- val result = yaml.load[util.Map[String, Object]](inputStream).asScala
- inputStream.close()
+ Using.resource(new FileInputStream(configFile)) { inputStream =>
+ val result = yaml.load[util.Map[String, Object]](inputStream).asScala
+ // Processing continues here
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| private lazy val data: Map[String, String] = { | |
| val yaml = new Yaml() | |
| val configDirPath = "config" | |
| val configFile = new File(s"$configDirPath/config.yaml") | |
| try { | |
| // Ensure parent config folder exists | |
| val parentDir = configFile.getParentFile | |
| if (!parentDir.exists()) parentDir.mkdirs() | |
| if (!configFile.exists()) { | |
| val resourceStream = getClass.getClassLoader.getResourceAsStream("config-template.yaml") | |
| if (resourceStream != null) { | |
| try { | |
| Files.copy(resourceStream, Paths.get(configFile.toURI), StandardCopyOption.REPLACE_EXISTING) | |
| logger.info(s"Created config file in ${configFile.getPath}") | |
| } finally { | |
| resourceStream.close() | |
| } | |
| } else { | |
| logger.debug("config-template.yaml resource not found") | |
| } | |
| } | |
| if (configFile.exists()) { | |
| val inputStream = new FileInputStream(configFile) | |
| val result = yaml.load[util.Map[String, Object]](inputStream).asScala | |
| inputStream.close() | |
| flattenYaml(Map.from(result)) | |
| } else { | |
| Map.empty[String, String] | |
| } | |
| } catch { | |
| case e: Exception => | |
| logger.debug(s"Failed to read from config.yaml: ${e.getMessage}") | |
| Map.empty[String, String] | |
| } | |
| } | |
| private lazy val data: Map[String, String] = { | |
| val yaml = new Yaml() | |
| val configDirPath = "config" | |
| val configFile = new File(s"$configDirPath/config.yaml") | |
| try { | |
| // Ensure parent config folder exists | |
| val parentDir = configFile.getParentFile | |
| if (!parentDir.exists()) parentDir.mkdirs() | |
| if (!configFile.exists()) { | |
| val resourceStream = getClass.getClassLoader.getResourceAsStream("config-template.yaml") | |
| if (resourceStream != null) { | |
| try { | |
| Files.copy(resourceStream, Paths.get(configFile.toURI), StandardCopyOption.REPLACE_EXISTING) | |
| logger.info(s"Created config file in ${configFile.getPath}") | |
| } finally { | |
| resourceStream.close() | |
| } | |
| } else { | |
| logger.debug("config-template.yaml resource not found") | |
| } | |
| } | |
| if (configFile.exists()) { | |
| Using.resource(new FileInputStream(configFile)) { inputStream => | |
| val result = yaml.load[util.Map[String, Object]](inputStream).asScala | |
| // Processing continues here | |
| } | |
| } else { | |
| Map.empty[String, String] | |
| } | |
| } catch { | |
| case e: Exception => | |
| logger.debug(s"Failed to read from config.yaml: ${e.getMessage}") | |
| Map.empty[String, String] | |
| } | |
| } |
| @@ -0,0 +1,40 @@ | |||
| ## Watchlistarr Configuration | |||
There was a problem hiding this comment.
Let's add more meat into this, and trim down the README
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
src/main/resources/config-template.yamlis excluded by:!**/*.yaml
Files selected for processing (1)
- README.md (3 hunks)
Files skipped from review due to trivial changes (1)
- README.md
We want to change up how we provide config in Watchlistarr. This paves the way for:
Summary by CodeRabbit
Summary by CodeRabbit
.gitignoreto exclude theconfig/directory from version control.