371 - clarify javadoc on config.getConfigSources

[Emily-Jiang]

Signed-off-by: Emily Jiang emijiang6@googlemail.com

[Emily-Jiang]

@jbee @radcortez @ljnelson please review

[jbee]

I find the wording very vague and easy to misunderstand.

Ideas you could (wrongly) get:

• not alterable means you will get errors when trying the remove() method?
• not alterable means every call might return the very same Iterable instance
• which may be either static or dynamic means the iterated elements (the length and order of the backing collection) may or may not change

Ideas you might not get but should:

• every call returns a new Iterable instance but the elements iterated never change (when are they pinned down? bootstrapping, point of first use?)

[Emily-Jiang]

Good point @jbee! I tried a few variants, and then settle on the current statement.

every call returns a new Iterable instance but the elements iterated never change (when are they pinned down? bootstrapping, point of first use?)

The elements never change is problematic. The elements are config sources. Some config sources might change as they might be dynamic.

How about just saying:

The Iterable contains a fixed number of config sources, determined at application start time, and the config sources may be static or dynamic.

[Emily-Jiang]

@jbee I thought it further and altered the above comments. Can you double check?

[jbee]

The Iterable contains a fixed number of config sources, determined at application start time, and the config sources may be static or dynamic.

More clear. Go for it.

[ljnelson]

Well, now, wait a minute: can't a ConfigSourceProvider supply a set of ConfigSources? So if a Config uses that set for its own ConfigSources, then the set's "staticness" cannot be guaranteed, right? Nor, presumably, can the iteration behavior be guaranteed? Is it written somewhere that a ConfigSourceProvider must provide a snapshot?

[ljnelson]

Yep; just checked the ConfigSourceProvider javadocs and there is nothing specified about how this must or must not behave. Nor is there anything specified about how Config must represent its output. So that may need to change before we make a guarantee here.

[radcortez]

Hum, I believe that we need to change other parts of the specification if we want to make this change.

From the spec:

MicroProfile Config also allows to implement and register own configuration sources in a portable way, e.g. for reading configuration values from a shared database in an application cluster.

From my understanding, there might be non-portable ways to register Config Sources, including after the Config object is initialized. In that particular case, then getConfigSources may be dynamic as well. If it needs to be determined at Config startup, then the spec must also change (or provide clarification), to Config Sources can only be registered / discovered on Config init.

[Emily-Jiang]

I have made the similar javadocs on ConfigSourceProvider @ljnelson. Please check to see whether you have any further comments.

[Emily-Jiang]

fixes #371

[ljnelson]

How does the ConfigSourceProvider know when "application start time" is? Do you just mean that the method is called once? Or maybe you mean that the method must be deterministic, i.e. must return whatever it returns whenever it is invoked?

[radcortez]

I've also stated that adding a restriction on the sources determined at start time, removes the ability to register additional sources later on. For instance, SmallRyeConfig provides an API to do just that.

I'm not against this restriction, but then I propose that the spec is also updated to clarify that sources may only be discovered on Config initialization.

[radcortez]

How does the ConfigSourceProvider know when "application start time" is?

I think the most correct timeline is on Config initialization. Remember that MP Config may be used outside of application containers, where the notion of "application start time" is not well defined.

Even on application container, due to the programmatic API, nothings stops you to access the Config before the application starts, so you need to have the Config instance ready.

[ljnelson]

I don't understand the focus on lifecycle concerns. Isn't this proposed restriction just saying: if the method returns sources A, B and C the first time it is invoked, then it must return sources A, B and C the second time it is invoked, and the third, and the nth? Or, much more clearly and succinctly: isn't this just saying the method must be deterministic?

[radcortez]

It is what it says, yes.

My point is, once you get a Config instance, you are locked in the registered ConfigSources. Right now, the spec did not specify that you couldn't add additional sources after getting a hold of the Config. With this change it effectively locks the Config. Is this a reasonable assumption?

If so, I'm only requesting that we clarify in the actual spec documentation the proper behaviour, instead of just being in the java docs. This seems to be somehow important to only be referenced in the java docs.

[Emily-Jiang]

added more clarification on the spec.

[ljnelson]

@Emily-Jiang Did this comment ever make it into the spec? If not, it would probably be best to put it here.

[Emily-Jiang]

@ljnelson it is pretty much common sense implementation. I prefer not to put it here as getPropertyNames() could give you a different list depending when you call it since some configsources are dynamic. This issue is about we ensure the number of configsources are determined and not dynamic while the config sources might be dynamic.

[ljnelson]

IMHO specifications should never ever rely on "common sense". "Common sense" is "undefined behavior".

Here, it is best to specify what the behavior of Config#getPropertyNames() is with respect to the behavior of ConfigSource#getPropertyNames(), because it does not seem to be specified anywhere at the moment. If you do not want to specify it, then an explicit statement to that effect would be important I think. Obviously if you think it should go in a separate issue that's fine too; I just don't want this to get lost.

[ljnelson]

One other question that may end up being its own issue:

1. If I understand right, a ConfigSourceProvider must return Iterators from its iterator() method that always iterate over the same number of elements. That is, if I call ConfigSourceProvider#getConfigSources() and it returns me an Iterator that iterates over three elements, and if I then call ConfigSourceProvider#getConfigSources() n more times, it is a requirement (unenforceable of course) that each such invocation will return me an Iterator that also iterates over the same three elements. Fine.
2. If I also understand right, any ConfigSource may be dynamic, i.e. may contain any number of configuration properties, and that number may change in any way over time. So at time 0 a ConfigSource could have a=b, and at time 1 it could have c=d and a=b, and at time 2 it could be empty, and at time 3 it could have x=y and a=c, and so on. Fine.
3. If I also understand right, I can always implement a ConfigSourceProvider's functionality in a single ConfigSource: i.e. it is simple, if tedious and boring, to wrap n ConfigSource implementations inside a single ConfigSource "façade" implementation. Fine.

My question: why does the restriction defined by (1) exist if (3) can work around it? If there's a simple answer, feel free to respond here, otherwise I'll open an issue.

[jbee]

why does the restriction defined by (1) exist

I want to believe for the same reason any limitation exist, namely (quoting Paul Phillips)

Every increase in expressiveness brings an increased burden on all who care to understand the message

Or as Paul put it more bluntly

Unnecessary expressiveness is the enemy

He reminded us that we easily fall for the "more power is better" fallacy and forget about being on the receiving end of it,

especially being on the receiving end of the possibility of it

Which causes the paradox of...

...all the things not done, because it was too complicated, because of all the things you could have done

He concludes:

The real power is... not having the power

;)

Remains the question of a changing list of sources being unnecessary? I'd say the TL;DR; is yes, you pretty much gave the answer. Like you suggest, if you have dynamically appearing sources through some vendor specific mechanism this can be represented as single source hooked in from the beginning which wraps the actual dynamic mechanism and acts as the façade for the properties they export. These dynamics allowed at the level of properties within a source make it unnecessary to additionally have dynamics on the level or sources. As a nice side effect it makes something visible that otherwise would be invisible. The existence of a DynamicSomethingSomethingSource in the list of sources will tell you that under certain circumstances more properties might occur though a certain mechanism. But more importantly being able to rely on a fixed list of sources simplifies any bit of code build on top or around sources as you will not have to deal with the possibility of more sources appearing or sources disappearing.

[ljnelson]

I'm not sure I understood all that but there were some eloquent parts!

Answering my own question: I remember now it is for thread safety: you can safely iterate over a fixed set of ConfigSources, whereas additional specification language would be required if the set were constantly growing and shrinking.

[Emily-Jiang]

I think we are now settled. Can I have an approval @jbee @ljnelson so that we can move on?