Skip to content
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

Allow parsing empty files without throwing error #340

Closed
andrew-smartnews opened this issue Aug 18, 2022 · 11 comments
Closed

Allow parsing empty files without throwing error #340

andrew-smartnews opened this issue Aug 18, 2022 · 11 comments

Comments

@andrew-smartnews
Copy link

I'd like to be able to pass in empty files and not have the yml parser throw, such as:

Caused by: java.lang.IllegalArgumentException: Expected document start at  in 'reader', line 1, column 1:
    
    ^
	at com.sksamuel.hoplite.yaml.YamlParser.load(YamlParser.kt:38)
	at com.sksamuel.hoplite.sources.ConfigFilePropertySource$node$1.invoke(ConfigFilePropertySource.kt:36)
	at com.sksamuel.hoplite.sources.ConfigFilePropertySource$node$1.invoke(ConfigFilePropertySource.kt:36)
	at com.sksamuel.hoplite.fp.Validated$Companion.ap(Validated.kt:70)
	at com.sksamuel.hoplite.sources.ConfigFilePropertySource.node(ConfigFilePropertySource.kt:36)
	at com.sksamuel.hoplite.NodeParser.parseNode(NodeParser.kt:29)
	at com.sksamuel.hoplite.ConfigLoader.loadConfig(ConfigLoader.kt:136)
	at com.smartnews.cg.configuration.BuildConfigKt.<clinit>(BuildConfig.kt:169)
	... 3 more

What do you think?

@sksamuel
Copy link
Owner

Sounds like a good idea.

@andrew-smartnews
Copy link
Author

Awesome I'll post up a PR (on my personal account, this is my work account)

@andrew-smartnews
Copy link
Author

Looks like I can't post PRs here, but I think I got most of the way there - got a bit stuck with the generics, but would love your opinion on how to close this out. I added the following:

  1. In ConfigFailure.kt:
data class EmptySource(val source: String) : ConfigFailure {
  override fun description(): String = "Source $source is empty"
}
  1. In ConfigFilePropertySource.kt, changed the node method to:
override fun node(context: PropertySourceContext): ConfigResult<Node> {
    val parser = context.parsers.locate(config.ext())
    return Validated.ap(parser, config.open()) { a, b -> if (b.available() > 0) b.use { a.load(it, config.describe()) } else ConfigFailure.EmptySource(config.describe()).invalid() }
      .mapInvalid { ConfigFailure.MultipleFailures(it) }
      .flatRecover { if (optional) Undefined.valid() else it.invalid() }
  }

Won't compile to a generics issue, but I think it's pretty close. Thoughts?

@sksamuel
Copy link
Owner

What's the error ?

@sksamuel
Copy link
Owner

I am ready to release 2.6.0
If you want to finish this I can include it, or I can add it myself if you wish.
@andrew-smartnews

@andrew-smartnews
Copy link
Author

I'd love to get this added so I can begin using it right away (all my config files have "THIS FILE CANNOT BE LEFT BLANK" right now for the ones that aren't yet filled in, which is not ideal!)

The error my change is giving me is:

Type mismatch.
Required:
ConfigResult<Node> /* = Validated<ConfigFailure, Node> */
Found:
Validated<ConfigFailure.MultipleFailures, Any>

My change is returning Validated<ConfigFailure.MultipleFailures, Any> vs. the expected Validated<ConfigFailure, Node>, and I can't quite figure out how to make it happy. Since it doesn't seem like I'm able to push up PRs, it's probably best if you do it (but I'm keen on seeing how you do it after digging into the code in detail yesterday!)

@andrew-smartnews
Copy link
Author

if you copy paste those two changes into the files as described, you should be able to reproduce easily - (or you can give me perms to push up a PR)

@sksamuel
Copy link
Owner

On github the way you do PRs for public projects is you fork the repo, push to your fork, and PR from fork to repo.
This avoids every project needing to give permissions to the public.
But I can just as easily do the work.

@andrew-smartnews
Copy link
Author

oh - thanks for the tip - I'll try that out

@andrew-smartnews
Copy link
Author

andrew-smartnews commented Aug 19, 2022

See PR: #342 @sksamuel

@sksamuel
Copy link
Owner

This now works in 2.6.0

ConfigLoaderBuilder.default().allowEmptySources....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants