Allow Configuration to carry its context name

[lalloni]

In a system where a big Configuratin is read from various sources and then different sections are detached and provided to components for internal configuration.

This components need to report configuration errors but, as is, can't report correctly the full path since the detached config doesn't carry the "path" from where it was detached.

So it would be nice if when you detach a configuration it would keep a string with de parameter passed to detach itself and expose it through a simple read-only property.

val configuration = Configuration(....)
val detached = configuration.detach("some.config.section")
// then:
assert detached.contextName == "some.config.section"

I gave not much thought to the contextName name.

If this makes general sense, I can offer a patch.

[paradigmatic]

I really like it. I am not sure about the method name: contextName is ok, but perhaps contextPath or rootPath are more explicit. What do you think ?

If you have some free time, please submit a patch. If you don't, I will take care of it next week probably.

[lalloni]

Getting back to the name... in Configuration's API there's no mention of "path" of keys and keys are just "key", they have no explicit "name".

Also in Configuration.detach(..) and Configuration.attach(..) (relevant API here, I think) the parameter referring to the concept under discussion is named "prefix", so I think that could be a consistent and appropriate name. I'm going to use that for the time being...

[lalloni]

Ok, here is the simplest patch I could think of.

Generally ok, but implies a broken previous invariant because after this change this equality does not hold anymore:

config.attach("x", config1).detach("x") == config1

This breaks because with this patch detached configs would always carry a prefix != None included in equals() comparison logic (because case class implications).

There is one test case I had to fix because of this, the fix was just comparing the data instead of the complete Configuration objects.

If you are not happy with this I think we could have the prefix as a val not included in the case class constructor, defaulted to None and overridden in the detach() method, with the added uglyness of subclassing for the overriding. Other option would be implementing equals() and hashCode() instead of relying on the case class defaults to avoid comparing on prefix. Neither nice.

[paradigmatic]

I'm convinced by the name of the properties. It makes the API consistent.

I would propose to solve the default value problem by using an empty string instead of an option. So a top level configuration has just an empty prefix:

case class Configuration( data: Map[String,String], prefix: String = "" ) {

I am also in favor of removing default arguments from the primary constructor, but it makes the code compatible with preceding versions.

If you want to change your patch, send me a pull request and I will merge it.

Thanks for the work.

[lalloni]

I agree completely about avoiding default arguments on primary constructors, but, as you said, for backwards compatibility I used that.

Having said that, I can't see which would be "the default value problem" (to be solved using an empty string) you mention?

Honestly, I'm not convinced about using an empty string to signal the absence of a prefix... of course if you want it that way, it will be that way, but...

Wouldn't it introduce potential ambiguities?

What's the practical benefit over using Option[String]?

I think None is more clearly representing the intent of the absence of prefix than "" which could be interpreted as an empty prefix as well the absence of prefix, which are not the same thing.

If you still believe it should be "" then I'll fix the commit no problem, just wanted to understand.

[lalloni]

2012/8/8 Paradigmatic notifications@github.com

My bad, I misunderstood the invariant problem you mentioned, so I fully agree with the option.

For the default parameter, I would be in favor of having an auxilliary constructor with the default and a deprecation tag:

case class Configuration( data: Map[String,String], prefix: Option[String] ) {

Agreed, I'll do that.

The auxiliary constructor would have only the "data" parameter and call the principal passing None for the prefix.

[paradigmatic]

Thanks, since the project is more than one year long. I plan to release the 1.0 version very soon and I will include your patch (+ binaries for scala 2.10 release candidate)

[lalloni]

Talking post 1.0...

Going on with the use of Configrity in this app we're building I've come to think that being able to report precisely the source and line number of a setting would be desirable, and while I see it quite feasible to implement, it would require changing drastically the simple way currently Configuration keeps its state (a map) to something more elaborate like a map or set of "Setting" (pretty much sbt like) which besides the value holds a source (url maybe?) and a position/line number so we can ask Configuration for a setting which carries where it came from.

Of course there are many rough edges like what to do with "Map" source like when using the default constructor.

How does this sound to you?

2012/8/8 Paradigmatic notifications@github.com

Thanks, since the project is more than one year long. I plan to release the 1.0 version very soon and I will include your patch (+ binaries for scala 2.10 release candidate)

— Reply to this email directly or view it on GitHubhttps://github.com/paradigmatic/Configrity/issues/14#issuecomment-7595395.

[lalloni]

Ok, maybe the "source" should be to Configuration some kind of opaque "tag" set by the IO layer on the settings it produces.

2012/8/8 Pablo Lalloni plalloni@gmail.com

Talking post 1.0...

Going on with the use of Configrity in this app we're building I've come to think that being able to report precisely the source and line number of a setting would be desirable, and while I see it quite feasible to implement, it would require changing drastically the simple way currently Configuration keeps its state (a map) to something more elaborate like a map or set of "Setting" (pretty much sbt like) which besides the value holds a source (url maybe?) and a position/line number so we can ask Configuration for a setting which carries where it came from.

Of course there are many rough edges like what to do with "Map" source like when using the default constructor.

How does this sound to you?

2012/8/8 Paradigmatic notifications@github.com

Thanks, since the project is more than one year long. I plan to release the 1.0 version very soon and I will include your patch (+ binaries for scala 2.10 release candidate)

— Reply to this email directly or view it on GitHubhttps://github.com/paradigmatic/Configrity/issues/14#issuecomment-7595395.

[paradigmatic]

Very interesting idea, I need to think about it. You introduce another issue for this one, so we can discuss without polluting the present issue.

[lalloni]

Created pull request for this.

[paradigmatic]

I just merged it. Thanks for your contribution.