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

Ability to add "default" converters #155

Open
StFS opened this issue Feb 10, 2016 · 9 comments
Open

Ability to add "default" converters #155

StFS opened this issue Feb 10, 2016 · 9 comments

Comments

@StFS
Copy link
Contributor

StFS commented Feb 10, 2016

Hi.

I'm going to need to add a couple of custom converters (duration and byte size, as in https://github.com/typesafehub/config/blob/master/HOCON.md#units-format).

I was wondering whether there was any way to add converters other than using the ConverterClass annotation?

For example, if I created a DurationConverter for converting strings like "10 minutes" into a java.time.Duration instance, I would like to avoid having to specify the converter annotation every time.

My thought was to be able to do something like the following:

    // register a converter to convert values that have a return type of java.time.Duration
    ConfigFactory.registerConverter(DurationConverter.class, java.time.Duration.class);
    MyConfig cfg = ConfigFactory.create(MyConfig.class);
    Duration dur = cfg.getSomeInterval();    

I would suggest that this be added to the core implementation but unfortunately java.time.Duration was only added in Java 7 and I believe OWNER is trying to maintain compatibility with 1.5 syntax.

So that's another question/comment on this, there is the owner-java8 module, but I'm having a difficult time understanding what the code in that module actually does. It would be good to be able to add converters there and register them automagically somehow so that if you had both owner (core) and owner-java8 artifacts as dependencies on your project you'd get the extra converters (such as the DurationConverter). Is this possible as is maybe?

@lviggiano
Copy link
Collaborator

I see your point and it's possible.

The owner-java8 module is meant to allow java8 feature without breaking the java5 full compatibility (for instance in java8 you have default methods in interfaces and owner-java8 module allows that).
I am thinking to some mechanism to automatically load converters from the classpath, so that just having some jars in the classpath can be sufficient.

@StFS
Copy link
Contributor Author

StFS commented Feb 12, 2016

Regarding adding a ByteUnit converter. That would either require defining all the classess needed for that inside the owner project to maintain the "no dependency" rule or find a library and add a dependency.

I'm not quite sure what the rules are here, the owner-java8 module is allowed to have Java8 dependencies, but what about the owner-extras? Is that Java5 only? Can it have dependencies on external libraries?

Writing the ByteUnit classes probably wouldn't be that much of a task but I have found a very small library that does exactly that (and only that): https://github.com/JakeWharton/byteunits

There are of course other implementations of this but they mostly seem to be embedded into larger software projects such as these:

So in my mind there are only the two options: 1) use the JakeWharton/byteunits library or 2) implement/copy the classes into owner.

@lviggiano what is your preference regarding this if I do another pull request for ByteSize config options?

@lviggiano
Copy link
Collaborator

owner-extras, I think, it's less critical than the main module, so from my point of view it can include dependencies as long as we mark them as "optional" (so people can import the module but use just some classes without having necessarily to import dependencies)

Just be patient some more days, since recently I had to stay away from work for (serious) health reasons, and it will take some time for me to get back into the project. But I really want to get into this asap.

@lviggiano
Copy link
Collaborator

And thanks for all the work you have done.

@StFS
Copy link
Contributor Author

StFS commented Feb 12, 2016

No worries. Hope you get well soon.

I'll create a pull request for the ByteSize types and use the byteunits library for it, at least to begin with.

@lviggiano
Copy link
Collaborator

Thanks again.

@lviggiano
Copy link
Collaborator

Also, have a look at the documentation (owner-site module): adding features without documenting, reduces the effectiveness of the code since users don't know the potential.

@StFS
Copy link
Contributor Author

StFS commented Feb 12, 2016

will do

@StFS
Copy link
Contributor Author

StFS commented Feb 15, 2016

Just took a quick look at the documentation, the main problem with adding documentation at this point is that I'm not quite sure how this will eventually be used. The main questions I have is, will the DurationConverter class be automatically assigned to configuration interface methods that return java.time.Duration (as long as the owner-java8 jar will be present in the classpath) as discussed in this issue, or will people have to manually specify the ConverterClass annotation and point to it?

So basically I'm thinking whether I should add a section on converter classes that are shipped with different modules of owner and can be used via the ConverterClass annotation, or whether I should rather focus on documenting how different modules will (automatically) add some custom ConverterClass implementations automatically if you include them in your runtime classpath (such as the DurationConverter if you include owner-java8). So I think I'll wait a bit on creating the documentation until we've discussed how this will be.

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