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

Add avaje-config-json #171

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add avaje-config-json #171

wants to merge 1 commit into from

Conversation

Mechite
Copy link
Contributor

@Mechite Mechite commented Sep 3, 2024

An experiment, created a simple parser with jsonb dependency as a separate module.

Did this because I saw #94 was closed, and this seemed like the first simple thing that never got done, adding rudimentary JSON support like another one of the languages here seems quite simple?

If we want to do more advanced stuff, like destructure a more nested JSON structure into key/value with . to seperate the levels. for example:

{
    "postgresql": {
        "host": "127.0.0.1"
    }
}

to be destructured into
"postgresql.host" = "127.0.0.1"
in the Map<String, String>,

this pull request does not achieve that. It will only support a flat JSON structure.
I didn't check what the YAML parser does.

Go ahead and make changes to this PR if you think it's a good idea, close it if not

Signed-off-by: Mechite <[email protected]>
@Mechite Mechite changed the title Add avaje-config-json Add avaje-config-json & avaje-config-toml Sep 3, 2024
@Mechite
Copy link
Contributor Author

Mechite commented Sep 3, 2024

EDIT - Moved to #172
(this PR originally brought #172 in along with it. It has been separated out)

@SentryMan
Copy link
Collaborator

I'm not totally sold on json as a config, @rbygrave what do you think?

@Mechite Mechite changed the title Add avaje-config-json & avaje-config-toml Add avaje-config-json Sep 6, 2024
@Mechite Mechite mentioned this pull request Sep 6, 2024
@rbygrave
Copy link
Contributor

rbygrave commented Sep 8, 2024

For myself, I'm personally extremely unlikely to use json for configuration of a Java application (where we have properties files which are so simple).

I'm happy to merge and include this if someone is going to say that they will actually use it? If no one has any immediate use case for this then I'm also happy for this PR to sit here for a bit and see what comments it gets.

@agentgt
Copy link

agentgt commented Sep 20, 2024

@SentryMan @rbygrave @Mechite

For me the challenge in config is not so much formats (parsing) but where the key values are coming from and if they have to be transformed. See #175

That is the source from where it comes from more often dictates what its format is anyway so I just don't find JSON config useful either.

@Mechite
Copy link
Contributor Author

Mechite commented Sep 20, 2024

@agentgt Absolutely agreed. I think even for an extremely large set of configurations, migrating formats would be more than trivial, even with special types like datetime being used with formats that support it.

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

Successfully merging this pull request may close these issues.

4 participants