From my point of view there is a bunch of misinformation in some of the
comments on this thread. Here's what I think:
0) The config admin spec has a meaning for factory pid and pid. If you create
a factory configuration, you use a factory pid, but you cannot control the pid
for the configuration, that is implementation defined. Most CA implementations
seem to try to help you out by using <factoryPid>-<number> but it could just as
well be a UID or other generated unique meaningless string.
1) master now at least looks self-consistent: for factory pids, the input xml
has an identifier of the form <id>-<factoryPid> and this is replicated in the
identifier used by karaf put into the configuration under the key CONFIG_KEY =
"org.apache.karaf.features.configKey". I didn't examine the code thoroughly
before the change, but from the diff it looks like it was not consistent in
that an incoming xml id <a>-<b> got turned into a config key of <b>-<a> which
is weird.
2) The current code seriously abuses "pid" for when there is a factory pid. In
this case, the "pid" is just some random string the user picked for their own
convenience and has nothing whatsoever to do with a CA pid which is used
internally to uniquely identify configurations.
3) This code from the "pre" in the diff is just a bad idea if there's something
called "factoryPid" lying around
> return configurationAdmin.createFactoryConfiguration(pid, null);
Anything that fixes that is an improvement.
4) To make the code clearer, I would change parsePid(String unparsed) to return
an object like
class PidInfo {
String pid;
String factoryPid;
String userId;
}
where the parser fills in either pid, or factoryPid and userId.
5) I think it's weird to have <userId>-<factoryPid>, and apparently this
differs from fileInstall and the old karaf behavior. I'd change it to
<factoryPid>-<userId> and change the config key values. This is a backwards
incompatible change, but I doubt configurations can actually be usefully
persisted across karaf upgrades.
thanks
david jencks
On Oct 7, 2014, at 3:32 PM, John Ellinwood <[email protected]> wrote:
> So did the original error end up getting fixed? I couldn't tell from the
> commits, jira, or this thread whether you reverted back to the original
> behavior or not.
>
> Gareth was correct in the original post for this thread, 3.0.1 is broken.
> We just upgraded from Karaf 3.0.0 to Karaf 3.0.1 and all of feature file
> based configs are broken now. Let me stress, everybody's feature based
> config deployments just broke between 3.0.0 and 3.0.1.
>
> For all of karaf history, including Karaf 3.0.0, the config name's value was
> parsed as <factoryPid>-<configName>.
>
> If you had a feature file like:
>
> <features name="features">
> <feature name="feature>
> <config name="my.factory-config1">
> prop1=value1
> </config>
> </feature>
> </features>
>
> I'd end up with a ConfigAdmin config like:
> service.pid: my.factory-123412341234342
> service.factoryPid: my.factory
> prop1: value1
>
> Now, with Karaf 3.0.1 that same feature file results in:
> service.pid: config1-12312312312312123
> service.factoryPid: config1
> prop1: value1
>
> FileInstall always followed the original convention as well, where a
> filename would be factoryPid-configName.cfg.
>
> I see commit
> https://github.com/apache/karaf/commit/21089127c89ddae275d495607c422dda8ffec474
> modified
> features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureConfigInstaller.java:
> - return configurationAdmin.createFactoryConfiguration(pid, null);
> + return configurationAdmin.createFactoryConfiguration(factoryPid, null);
>
> and it doesn't appear to be fixed in
> https://github.com/apache/karaf/blob/master/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureConfigInstaller.java
>
> Why would you make such a drastic change to the way that a core part of
> karaf works??
>
>
>
> --
> View this message in context:
> http://karaf.922171.n3.nabble.com/Problems-With-Factory-Configurations-In-Karaf-3-0-1-tp4033921p4035676.html
> Sent from the Karaf - User mailing list archive at Nabble.com.
