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

For load-config :carica/json, directly open an io/reader on resource #4

Merged

Conversation

ddillinger
Copy link
Collaborator

.openStream is a java.net.URL method, which means, if you pass "something io/reader could
handle, but not a URL" you get a "No matching field found: openStream" exception.

For example, a java.net.URI, which io/reader handles fine, threw that exception. Basically,
the assumption was that load-config was only ever called on a resource directly returned
from io/resource (which returns java.net.URL because that is what ClassLoader/getResource
returns), but that's not necessarily always true for anyone calling load-config with json
from a java.net.URI, or a java.io.File, or even a string -- "(carica/load-config
"/path/to/some.json")" would also throw.

.openStream is a java.net.URL method, which means, if you pass "something io/reader could
handle, but not a URL" you get a "No matching field found: openStream" exception.

For example, a java.net.URI, which io/reader handles fine, threw that exception. Basically,
the assumption was that load-config was only ever called on a resource directly returned
from io/resource (which returns java.net.URL because that is what ClassLoader/getResource
returns), but that's not necessarily always true for anyone calling load-config with json
from a java.net.URI, or a java.io.File, or even a string -- "(carica/load-config
"/path/to/some.json")" would also throw.
@ddillinger ddillinger added the bug label Aug 3, 2022
@ddillinger ddillinger self-assigned this Aug 3, 2022
Copy link
Owner

@leathekd leathekd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ddillinger ddillinger merged commit c1014db into leathekd:master Aug 3, 2022
@ddillinger ddillinger deleted the better-json-resource-handling branch August 3, 2022 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants