You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by aledsage <gi...@git.apache.org> on 2017/06/16 12:13:40 UTC
[GitHub] brooklyn-server pull request #734: Support ConfigKey deprecated names
GitHub user aledsage opened a pull request:
https://github.com/apache/brooklyn-server/pull/734
Support ConfigKey deprecated names
As discussed on the dev@brooklyn mailing list in the thread "[PROPOSAL] Deprecate @SetFromFlag".
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/aledsage/brooklyn-server configKey-deprecatedNames
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/734.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #734
----
commit 93b7e188fc391e00436459282b4d4595fe9b96b5
Author: Aled Sage <al...@gmail.com>
Date: 2017-06-08T13:59:04Z
Add LogWatcher.EventPredicates.containsMessages
commit 02b71ffb5f81abb6ad76095a64903335b9692fda
Author: Aled Sage <al...@gmail.com>
Date: 2017-06-08T13:58:46Z
Support configKey.deprecatedNames
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #734: Support ConfigKey deprecated names
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/734#discussion_r124584831
--- Diff: core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java ---
@@ -215,6 +216,16 @@ public AbstractLocation configure(Map<?,?> properties) {
config().removeKey(PARENT_LOCATION);
}
+ // allow config keys to be set by name (or deprecated name)
+ //
+ // Aled thinks it would be sensible to remove the consumed flags below (i.e. properties = ...).
+ // However, that caused ClockerDynamicLocationPatternTest to fail because there is a field of
+ // StubContainerLocation annotated with `@SetFromFlag("owner")`, as well as a config key with
--- End diff --
Not certain - I deleted the test that used it in #728. My concern is whether that test highlighted that we're supposed to be supporting such madness!
@ahgittin's comment was: "if it's not too hard to log a warning wherever we rely on it, however, that would be ideal. if that's not easy then fine to abandon.".
It's probably tricky to log.warn on it reliably (because `FlagUtils.setFieldsFromFlagsWithBag` and `FlagUtils.setAllConfigKeys` also does logic for setting config, so we can't easily tell how it's using the key).
We could try adding some new code, e.g. in `EntityDynamicType`, to detect it, and warn (or even fail) if we see the name being used in multiple places. I suggest we do that separately from this PR, and I suggest that we fail-fast.
I'd be happy with abandoning - i.e. change it to what I proposed in the comment, to not pass the used properties to `FlagUtils.setFieldsFromFlagsWithBag` or `FlagUtils.setAllConfigKeys`.
I'll go with abandoning, unless anyone feels strongly to the contrary?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #734: Support ConfigKey deprecated names
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/734#discussion_r124008032
--- Diff: core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyDeprecationTest.java ---
@@ -0,0 +1,328 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.core.config;
+
+import static org.testng.Assert.assertEquals;
+
+import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.api.entity.EntityInitializer;
+import org.apache.brooklyn.api.entity.EntityLocal;
+import org.apache.brooklyn.api.entity.EntitySpec;
+import org.apache.brooklyn.api.entity.ImplementedBy;
+import org.apache.brooklyn.api.location.LocationSpec;
+import org.apache.brooklyn.api.policy.PolicySpec;
+import org.apache.brooklyn.api.sensor.EnricherSpec;
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.enricher.AbstractEnricher;
+import org.apache.brooklyn.core.entity.AbstractEntity;
+import org.apache.brooklyn.core.entity.EntityInternal;
+import org.apache.brooklyn.core.entity.internal.ConfigUtilsInternal;
+import org.apache.brooklyn.core.location.AbstractLocation;
+import org.apache.brooklyn.core.policy.AbstractPolicy;
+import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
+import org.apache.brooklyn.core.test.entity.TestEntity;
+import org.apache.brooklyn.entity.stock.BasicEntity;
+import org.apache.brooklyn.test.LogWatcher;
+import org.apache.brooklyn.test.LogWatcher.EventPredicates;
+import org.apache.brooklyn.util.core.config.ConfigBag;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableMap;
+
+import ch.qos.logback.classic.spi.ILoggingEvent;
+
+public class ConfigKeyDeprecationTest extends BrooklynAppUnitTestSupport {
+
+ @SuppressWarnings("unused")
+ private static final Logger log = LoggerFactory.getLogger(ConfigKeyDeprecationTest.class);
+
+ public static final ConfigKey<String> KEY_1 = ConfigKeys.builder(String.class, "key1")
+ .deprecatedNames("oldKey1", "oldKey1b")
+ .build();
+
+ @Test
+ public void testUsingDeprecatedName() throws Exception {
+ EntityInternal entity = app.addChild(EntitySpec.create(MyBaseEntity.class)
+ .configure("oldSuperKey1", "myval"));
+ EntityInternal entity2 = app.addChild(EntitySpec.create(MyBaseEntity.class)
+ .configure("oldSuperKey1b", "myval"));
+ assertEquals(entity.config().get(MyBaseEntity.SUPER_KEY_1), "myval");
+ assertEquals(entity2.config().get(MyBaseEntity.SUPER_KEY_1), "myval");
+ }
+
+ @Test
+ public void testPrefersNonDeprecatedName() throws Exception {
+ EntityInternal entity = app.addChild(EntitySpec.create(MyBaseEntity.class)
+ .configure("superKey1", "myval")
+ .configure("oldSuperKey1", "mywrongval"));
+ assertEquals(entity.config().get(MyBaseEntity.SUPER_KEY_1), "myval");
+ }
+
+ @Test
+ public void testPrefersFirstDeprecatedNameIfMultiple() throws Exception {
+ EntityInternal entity = app.addChild(EntitySpec.create(MyBaseEntity.class)
+ .configure("oldSuperKey1", "myval1")
--- End diff --
Maybe nicer to order these as
```
.configure("oldSuperKey1b", "myval2")
.configure("oldSuperKey1", "myval1")
```
to demonstrate that the order in which they are configured here is not the determining factor; maybe even call `oldSuperKey1` something instead that is lexically greater than `oldSuperKey2` to demonstrate that it's not alphabetic either!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #734: Support ConfigKey deprecated names
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/734#discussion_r124574299
--- Diff: core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java ---
@@ -365,6 +384,7 @@ public boolean isValueValid(T value) {
}
/** @see ConfigKey#getNameParts() */
+ @Deprecated
--- End diff --
That's included in the super-type `ConfigKey.getNameParts()`, which says `@deprecated since 0.12.0; use {@link #getName()}`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #734: Support ConfigKey deprecated names
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/734#discussion_r123992282
--- Diff: core/src/main/java/org/apache/brooklyn/util/core/config/ConfigBag.java ---
@@ -518,6 +574,46 @@ protected Object getStringKey(String key, boolean markUsed) {
/**
* @return Unresolved configuration value. May be overridden to provide resolution - @see {@link ResolvingConfigBag#getStringKeyMaybe(String, boolean)}
*/
+ protected synchronized Maybe<Object> getKeyMaybe(ConfigKey<?> key, boolean markUsed) {
--- End diff --
There's a fair bit of duplication of code here with the method above, it would be nice to reduce that somehow.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #734: Support ConfigKey deprecated names
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/734#discussion_r123985528
--- Diff: core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java ---
@@ -215,6 +216,16 @@ public AbstractLocation configure(Map<?,?> properties) {
config().removeKey(PARENT_LOCATION);
}
+ // allow config keys to be set by name (or deprecated name)
+ //
+ // Aled thinks it would be sensible to remove the consumed flags below (i.e. properties = ...).
+ // However, that caused ClockerDynamicLocationPatternTest to fail because there is a field of
+ // StubContainerLocation annotated with `@SetFromFlag("owner")`, as well as a config key with
--- End diff --
You deleted that in https://github.com/apache/brooklyn-server/pull/728, does that mean you can get rid of these flags now?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #734: Support ConfigKey deprecated names
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/734#discussion_r124009325
--- Diff: core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyDeprecationTest.java ---
@@ -0,0 +1,328 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.core.config;
+
+import static org.testng.Assert.assertEquals;
+
+import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.api.entity.EntityInitializer;
+import org.apache.brooklyn.api.entity.EntityLocal;
+import org.apache.brooklyn.api.entity.EntitySpec;
+import org.apache.brooklyn.api.entity.ImplementedBy;
+import org.apache.brooklyn.api.location.LocationSpec;
+import org.apache.brooklyn.api.policy.PolicySpec;
+import org.apache.brooklyn.api.sensor.EnricherSpec;
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.enricher.AbstractEnricher;
+import org.apache.brooklyn.core.entity.AbstractEntity;
+import org.apache.brooklyn.core.entity.EntityInternal;
+import org.apache.brooklyn.core.entity.internal.ConfigUtilsInternal;
+import org.apache.brooklyn.core.location.AbstractLocation;
+import org.apache.brooklyn.core.policy.AbstractPolicy;
+import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
+import org.apache.brooklyn.core.test.entity.TestEntity;
+import org.apache.brooklyn.entity.stock.BasicEntity;
+import org.apache.brooklyn.test.LogWatcher;
+import org.apache.brooklyn.test.LogWatcher.EventPredicates;
+import org.apache.brooklyn.util.core.config.ConfigBag;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableMap;
+
+import ch.qos.logback.classic.spi.ILoggingEvent;
+
+public class ConfigKeyDeprecationTest extends BrooklynAppUnitTestSupport {
+
+ @SuppressWarnings("unused")
+ private static final Logger log = LoggerFactory.getLogger(ConfigKeyDeprecationTest.class);
+
+ public static final ConfigKey<String> KEY_1 = ConfigKeys.builder(String.class, "key1")
+ .deprecatedNames("oldKey1", "oldKey1b")
+ .build();
+
+ @Test
+ public void testUsingDeprecatedName() throws Exception {
+ EntityInternal entity = app.addChild(EntitySpec.create(MyBaseEntity.class)
+ .configure("oldSuperKey1", "myval"));
+ EntityInternal entity2 = app.addChild(EntitySpec.create(MyBaseEntity.class)
+ .configure("oldSuperKey1b", "myval"));
+ assertEquals(entity.config().get(MyBaseEntity.SUPER_KEY_1), "myval");
+ assertEquals(entity2.config().get(MyBaseEntity.SUPER_KEY_1), "myval");
+ }
+
+ @Test
+ public void testPrefersNonDeprecatedName() throws Exception {
+ EntityInternal entity = app.addChild(EntitySpec.create(MyBaseEntity.class)
+ .configure("superKey1", "myval")
+ .configure("oldSuperKey1", "mywrongval"));
+ assertEquals(entity.config().get(MyBaseEntity.SUPER_KEY_1), "myval");
+ }
+
+ @Test
+ public void testPrefersFirstDeprecatedNameIfMultiple() throws Exception {
+ EntityInternal entity = app.addChild(EntitySpec.create(MyBaseEntity.class)
+ .configure("oldSuperKey1", "myval1")
+ .configure("oldSuperKey1b", "myval2"));
+ assertEquals(entity.config().get(MyBaseEntity.SUPER_KEY_1), "myval1");
+ }
+
+ @Test
+ public void testInheritsDeprecatedKeyFromRuntimeParent() throws Exception {
+ EntityInternal entity = app.addChild(EntitySpec.create(TestEntity.class)
+ .configure("oldSuperKey1", "myval"));
+ EntityInternal child = entity.addChild(EntitySpec.create(MyBaseEntity.class));
+ assertEquals(child.config().get(MyBaseEntity.SUPER_KEY_1), "myval");
+ }
+
+ @Test
+ public void testInheritsDeprecatedKeyFromSuperType() throws Exception {
+ EntityInternal entity = app.addChild(EntitySpec.create(MySubEntity.class)
+ .configure("oldSuperKey1", "myval")
+ .configure("oldSuperKey2", "myval2")
+ .configure("oldSubKey2", "myval3")
+ .configure("oldInterfaceKey1", "myval4"));
+ assertEquals(entity.config().get(MySubEntity.SUPER_KEY_1), "myval");
+ assertEquals(entity.config().get(MySubEntity.SUPER_KEY_2), "myval2");
+ assertEquals(entity.config().get(MySubEntity.SUB_KEY_2), "myval3");
+ assertEquals(entity.config().get(MyInterface.INTERFACE_KEY_1), "myval4");
+ }
+
+ @Test
+ public void testUsingDeprecatedNameInLocation() throws Exception {
+ MyLocation loc = mgmt.getLocationManager().createLocation(LocationSpec.create(MyLocation.class)
+ .configure("oldKey1", "myval"));
+ assertEquals(loc.config().get(MyLocation.KEY_1), "myval");
+ }
+
+ @Test
+ public void testUsingDeprecatedNameInPolicy() throws Exception {
+ MyPolicy policy = app.policies().add(PolicySpec.create(MyPolicy.class)
+ .configure("oldKey1", "myval"));
+ assertEquals(policy.config().get(MyPolicy.KEY_1), "myval");
+ }
+
+ @Test
+ public void testUsingDeprecatedNameInEnricher() throws Exception {
+ MyEnricher enricher = app.enrichers().add(EnricherSpec.create(MyEnricher.class)
+ .configure("oldKey1", "myval"));
+ assertEquals(enricher.config().get(MyEnricher.KEY_1), "myval");
+ }
+
+ @Test
+ public void testUsingDeprecatedNameInEntityInitializer() throws Exception {
+ Entity entity = app.addChild(EntitySpec.create(BasicEntity.class)
+ .addInitializer(new MyEntityInitializer(ConfigBag.newInstance(ImmutableMap.of("oldKey1", "myval")))));
+ assertEquals(entity.config().get(MyEntityInitializer.KEY_1), "myval");
+ }
+
+ @Test
+ public void testSetConfigUsingDeprecatedName() throws Exception {
+ EntityInternal entity = app.addChild(EntitySpec.create(MyBaseEntity.class));
+
+ // Set using strongly typed key
+ entity.config().set(MyBaseEntity.SUPER_KEY_1, "myval");
+ assertEquals(entity.config().get(MyBaseEntity.SUPER_KEY_1), "myval");
+
+ // Set using correct string
+ entity.config().putAll(ImmutableMap.of("superKey1", "myval2"));
+ assertEquals(entity.config().get(MyBaseEntity.SUPER_KEY_1), "myval2");
+
+ // Set using deprecated name
+ entity.config().putAll(ImmutableMap.of("oldSuperKey1", "myval3"));
+ assertEquals(entity.config().get(MyBaseEntity.SUPER_KEY_1), "myval3");
+
+ // Set using pseudo-generated strongly typed key with deprecated name
+ entity.config().set(ConfigKeys.newConfigKey(Object.class, "oldSuperKey1"), "myval4");
+ assertEquals(entity.config().get(MyBaseEntity.SUPER_KEY_1), "myval4");
+ }
+
+ @Test
+ public void testSetAndGetDynamicConfigUsingDeprecatedName() throws Exception {
+ // Using BasicEntity, which doesn't know about KEY_1
+ EntityInternal entity = (EntityInternal) app.addChild(EntitySpec.create(BasicEntity.class));
+
+ // Set using deprecated name
+ entity.config().putAll(ImmutableMap.of("oldKey1", "myval3"));
+ assertEquals(entity.config().get(KEY_1), "myval3");
+
+ // Set using pseudo-generated strongly typed key with deprecated name
+ entity.config().set(ConfigKeys.newConfigKey(Object.class, "oldKey1"), "myval4");
+ assertEquals(entity.config().get(KEY_1), "myval4");
+ }
+
+ /*
+ // Setting the value of a "dynamic config key" when using the deprecated key will not overwrite
+ // any existing value that was set using the real config key name. I think this is because
+ // EntityDynamicType.addConfigKey() is not called when KEY_1 is first set, so it doesn't have
+ // access to the deprecated names on subsequent calls to set(String, val). It therefore stores
+ // in EntityConfigMap (backed by a ConfigBag) the original key-value and the deprecated key-value.
+ // When we subsequently call get(key), it retrieved the original key-value.
+ //
+ // Contrast this with EntityDynamicType.addSensorIfAbsent().
+ //
+ * However, it's (probably) not straight forward to just add and call a addConfigKeyIfAbsent. This
+ * is because config().set() is used for dynamic config declard in yaml, which the entity doesn't
--- End diff --
typo `declard`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #734: Support ConfigKey deprecated names
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-server/pull/734
Going to merge this, should have done so before now, sorry! Any further changes can be done as separate PRs.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #734: Support ConfigKey deprecated names
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/734#discussion_r124582410
--- Diff: core/src/main/java/org/apache/brooklyn/util/core/config/ConfigBag.java ---
@@ -518,6 +574,46 @@ protected Object getStringKey(String key, boolean markUsed) {
/**
* @return Unresolved configuration value. May be overridden to provide resolution - @see {@link ResolvingConfigBag#getStringKeyMaybe(String, boolean)}
*/
+ protected synchronized Maybe<Object> getKeyMaybe(ConfigKey<?> key, boolean markUsed) {
--- End diff --
Yeah, not sure how to remove that without an almost-equal amount of code! The difference is it needs to call a `getRawStringKeyMaybe(name, markUsed)` or `getStringKeyMaybe(name, markUsed)`. Thinking of how we could achieve that:
* We could pass another boolean to say whether raw is desired. But I don't really like that pattern.
* There's not a nice java interface for passing two parameters, so we'd end up declaring our own interface to pass the different getter impl.
Do you think either of those is worth it @geomacy?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #734: Support ConfigKey deprecated names
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/734#discussion_r124580007
--- Diff: core/src/main/java/org/apache/brooklyn/util/core/config/ConfigBag.java ---
@@ -507,6 +525,44 @@ protected Object getStringKey(String key, boolean markUsed) {
return getStringKeyMaybe(key, markUsed).orNull();
}
+ private synchronized Maybe<Object> getRawKeyMaybe(ConfigKey<?> key, boolean markUsed) {
+ Maybe<Object> val = getRawStringKeyMaybe(key.getName(), markUsed);
+
+ String firstDeprecatedName = null;
+ Maybe<Object> firstDeprecatedVal = null;
+ for (String deprecatedName : key.getDeprecatedNames()) {
+ Maybe<Object> deprecatedVal = getRawStringKeyMaybe(deprecatedName, markUsed);
+ if (deprecatedVal.isPresent()) {
+ if (val.isPresent()) {
+ if (!Objects.equal(val.get(), deprecatedVal.get())) {
+ log.warn("Conflicting value for key "+key+" from deprecated name '"+deprecatedName+"'; "
+ + "using value from preferred name "+key.getName());
+ } else {
+ log.warn("Duplicate value for key "+key+" from deprecated name '"+deprecatedName+"'; "
+ + "using same value from preferred name "+key.getName());
+ }
+ } else if (firstDeprecatedVal.isPresent()) {
--- End diff --
Good spot! I've added a failing test, and then fixed the NPE.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #734: Support ConfigKey deprecated names
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/734#discussion_r124586077
--- Diff: core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyDeprecationRebindTest.java ---
@@ -0,0 +1,373 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.core.config;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.List;
+
+import org.apache.brooklyn.api.entity.Application;
+import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.api.entity.EntitySpec;
+import org.apache.brooklyn.api.entity.ImplementedBy;
+import org.apache.brooklyn.api.objs.BrooklynObjectType;
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.enricher.AbstractEnricher;
+import org.apache.brooklyn.core.entity.AbstractApplication;
+import org.apache.brooklyn.core.entity.EntityInternal;
+import org.apache.brooklyn.core.feed.AbstractFeed;
+import org.apache.brooklyn.core.location.AbstractLocation;
+import org.apache.brooklyn.core.mgmt.rebind.AbstractRebindHistoricTest;
+import org.apache.brooklyn.core.policy.AbstractPolicy;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Joiner;
+import com.google.common.base.Predicates;
+import com.google.common.collect.Iterables;
+
+public class ConfigKeyDeprecationRebindTest extends AbstractRebindHistoricTest {
+
+ @Test
+ public void testUsingDeprecatedName() throws Exception {
+ MyApp entity = app().addChild(EntitySpec.create(MyApp.class)
+ .configure("oldKey1", "myval"));
+ assertEquals(entity.config().get(MyApp.KEY_1), "myval");
+
+ rebind();
+
+ Entity newEntity = mgmt().getEntityManager().getEntity(entity.getId());
+ assertEquals(newEntity.config().get(MyApp.KEY_1), "myval");
+
+ // Expect the persisted state of the entity to have used the non-deprecated name.
+ String allLines = getPersistanceFileContents(BrooklynObjectType.ENTITY, newEntity.getId());
+ assertFalse(allLines.contains("oldKey1"), "contains 'oldKey1', allLines="+allLines);
+ assertTrue(allLines.contains("key1"), "contains 'key1', allLines="+allLines);
+ }
+
+ /**
+ * Created with:
+ * <pre>
+ * {@code
+ * Entity app = mgmt().getEntityManager().createEntity(EntitySpec.create(MyApp.class)
+ * .configure("oldKey1", "myval"));
+ * }
+ * </pre>
+ */
+ @Test
+ public void testEntityPersistedWithDeprecatedKeyName() throws Exception {
+ String appId = "pps2ttgijb";
+ addMemento(BrooklynObjectType.ENTITY, "config-deprecated-key", appId);
+ rebind();
+
+ Entity newApp = mgmt().getEntityManager().getEntity(appId);
+ assertEquals(newApp.config().get(MyApp.KEY_1), "myval");
+
+ // Expect the persisted state to have been re-written with the new key value.
+ switchOriginalToNewManagementContext();
+ rebind();
+
+ String allLines = getPersistanceFileContents(BrooklynObjectType.ENTITY, appId);
+ assertFalse(allLines.contains("oldKey1"), "should not contain 'oldKey1', allLines="+allLines);
+ assertTrue(allLines.contains("<key1>"), "should contain '<key1>', allLines="+allLines);
+ }
+
+ /**
+ * Created with:
+ * <pre>
+ * {@code
+ * Entity app = mgmt().getEntityManager().createEntity(EntitySpec.create(MyApp.class)
+ * .configure("oldFlagKey2", "myval"));
+ * }
+ * </pre>
+ */
+ @Test
+ public void testEntityPersistedWithSetFromFlagNameOnKey() throws Exception {
+ String appId = "ug77ek2tkd";
+ addMemento(BrooklynObjectType.ENTITY, "config-deprecated-flagNameOnKey", appId);
+ rebind();
+
+ Entity newApp = mgmt().getEntityManager().getEntity(appId);
+ assertEquals(newApp.config().get(MyApp.KEY_2), "myval");
+
+ // Expect the persisted state to have been re-written with the new key value.
+ switchOriginalToNewManagementContext();
+ rebind();
+
+ String allLines = getPersistanceFileContents(BrooklynObjectType.ENTITY, appId);
+ assertFalse(allLines.contains("oldFlagKey2"), "should not contain 'oldFlagKey2', allLines="+allLines);
--- End diff --
Yeah, it's a bit of a weird test. It was sensible to decide to write it - I generated the persisted state (as per the javadoc) with:
```
Entity app = mgmt().getEntityManager().createEntity(EntitySpec.create(MyApp.class)
.configure("oldFlagKey2", "myval"));
```
That turns out not to contain `oldFlagKey2` because it was transformed at the time of writing the persisted state (even before any changes in this PR).
I think it's important that we record that this upgrade-path works. I could delete the test, and replace it with just a comment in the test class to say why we're not explicitly testing this scenario.
Or I could leave in the test, and add to the javadoc an explanation of why it's weird.
Thoughts @geomacy?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #734: Support ConfigKey deprecated names
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/734
@geomacy comments addressed (except where I responded explicitly to your comment). I've added two commits on top of the original - could you review those please?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #734: Support ConfigKey deprecated names
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-server/pull/734
hi @aledsage sorry missed the update above last week, will look at that presto.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #734: Support ConfigKey deprecated names
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/734#discussion_r125617562
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java ---
@@ -153,19 +153,13 @@ public AbstractEntityAdjunct configure(Map flags) {
}
}
- // Allow config keys to be set by name (or deprecated name).
+ // Allow config keys to be set by name (or deprecated name)
//
- // Aled thinks it would be sensible to remove the consumed flags below (i.e. flags = ...).
- // However, that causes PolicyConfigTest.testConfigFlagsPassedInAtConstructionIsAvailable to fail.
- // However, tht looks mad - do we really need to support it?!
- // The policy is defined with one key using a name in SetFromFlag, and another key using the same name.
- // It expects both of the config keys to have been set.
- // @SetFromFlag("strKey")
- // public static final ConfigKey<String> STR_KEY = new BasicConfigKey<String>(String.class, "akey", "a key");
- // public static final ConfigKey<String> STR_KEY_WITH_DEFAULT = new BasicConfigKey<String>(String.class, "strKey", "str key", "str key default");
- // I've preserved that behaviour (for now).
- ConfigUtilsInternal.setAllConfigKeys(flags, getAdjunctType().getConfigKeys(), this);
+ // The resulting `flags` will no longer contain the keys that we matched;
+ // we will not also use them to for `SetFromFlag` etc.
+ flags = ConfigUtilsInternal.setAllConfigKeys(flags, getAdjunctType().getConfigKeys(), this);
+ // Must call setFieldsFromFlagsWithBag, even if properites is empty, so defaults are extracted from annotations
--- End diff --
shouldn't this read `// Must call setFieldsFromFlags,` as per 164 below?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #734: Support ConfigKey deprecated names
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/734#discussion_r122915954
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java ---
@@ -151,6 +153,19 @@ public AbstractEntityAdjunct configure(Map flags) {
}
}
+ // Allow config keys to be set by name (or deprecated name).
+ //
+ // Aled thinks it would be sensible to remove the consumed flags below (i.e. flags = ...).
+ // However, that causes PolicyConfigTest.testConfigFlagsPassedInAtConstructionIsAvailable to fail.
+ // However, tht looks mad - do we really need to support it?!
+ // The policy is defined with one key using a name in SetFromFlag, and another key using the same name.
+ // It expects both of the config keys to have been set.
+ // @SetFromFlag("strKey")
+ // public static final ConfigKey<String> STR_KEY = new BasicConfigKey<String>(String.class, "akey", "a key");
+ // public static final ConfigKey<String> STR_KEY_WITH_DEFAULT = new BasicConfigKey<String>(String.class, "strKey", "str key", "str key default");
+ // I've preserved that behaviour (for now).
+ ConfigUtilsInternal.setAllConfigKeys(flags, getAdjunctType().getConfigKeys(), this);
--- End diff --
agree that behaviour is way overkill. if it's not too hard to log a warning wherever we rely on it, however, that would be ideal. if that's not easy then fine to abandon.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #734: Support ConfigKey deprecated names
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/734#discussion_r124001663
--- Diff: core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyDeprecationRebindTest.java ---
@@ -0,0 +1,373 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.core.config;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.List;
+
+import org.apache.brooklyn.api.entity.Application;
+import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.api.entity.EntitySpec;
+import org.apache.brooklyn.api.entity.ImplementedBy;
+import org.apache.brooklyn.api.objs.BrooklynObjectType;
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.enricher.AbstractEnricher;
+import org.apache.brooklyn.core.entity.AbstractApplication;
+import org.apache.brooklyn.core.entity.EntityInternal;
+import org.apache.brooklyn.core.feed.AbstractFeed;
+import org.apache.brooklyn.core.location.AbstractLocation;
+import org.apache.brooklyn.core.mgmt.rebind.AbstractRebindHistoricTest;
+import org.apache.brooklyn.core.policy.AbstractPolicy;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Joiner;
+import com.google.common.base.Predicates;
+import com.google.common.collect.Iterables;
+
+public class ConfigKeyDeprecationRebindTest extends AbstractRebindHistoricTest {
+
+ @Test
+ public void testUsingDeprecatedName() throws Exception {
+ MyApp entity = app().addChild(EntitySpec.create(MyApp.class)
+ .configure("oldKey1", "myval"));
+ assertEquals(entity.config().get(MyApp.KEY_1), "myval");
+
+ rebind();
+
+ Entity newEntity = mgmt().getEntityManager().getEntity(entity.getId());
+ assertEquals(newEntity.config().get(MyApp.KEY_1), "myval");
+
+ // Expect the persisted state of the entity to have used the non-deprecated name.
+ String allLines = getPersistanceFileContents(BrooklynObjectType.ENTITY, newEntity.getId());
+ assertFalse(allLines.contains("oldKey1"), "contains 'oldKey1', allLines="+allLines);
+ assertTrue(allLines.contains("key1"), "contains 'key1', allLines="+allLines);
+ }
+
+ /**
+ * Created with:
+ * <pre>
+ * {@code
+ * Entity app = mgmt().getEntityManager().createEntity(EntitySpec.create(MyApp.class)
+ * .configure("oldKey1", "myval"));
+ * }
+ * </pre>
+ */
+ @Test
+ public void testEntityPersistedWithDeprecatedKeyName() throws Exception {
+ String appId = "pps2ttgijb";
+ addMemento(BrooklynObjectType.ENTITY, "config-deprecated-key", appId);
+ rebind();
+
+ Entity newApp = mgmt().getEntityManager().getEntity(appId);
+ assertEquals(newApp.config().get(MyApp.KEY_1), "myval");
+
+ // Expect the persisted state to have been re-written with the new key value.
+ switchOriginalToNewManagementContext();
+ rebind();
+
+ String allLines = getPersistanceFileContents(BrooklynObjectType.ENTITY, appId);
+ assertFalse(allLines.contains("oldKey1"), "should not contain 'oldKey1', allLines="+allLines);
+ assertTrue(allLines.contains("<key1>"), "should contain '<key1>', allLines="+allLines);
+ }
+
+ /**
+ * Created with:
+ * <pre>
+ * {@code
+ * Entity app = mgmt().getEntityManager().createEntity(EntitySpec.create(MyApp.class)
+ * .configure("oldFlagKey2", "myval"));
+ * }
+ * </pre>
+ */
+ @Test
+ public void testEntityPersistedWithSetFromFlagNameOnKey() throws Exception {
+ String appId = "ug77ek2tkd";
+ addMemento(BrooklynObjectType.ENTITY, "config-deprecated-flagNameOnKey", appId);
+ rebind();
+
+ Entity newApp = mgmt().getEntityManager().getEntity(appId);
+ assertEquals(newApp.config().get(MyApp.KEY_2), "myval");
+
+ // Expect the persisted state to have been re-written with the new key value.
+ switchOriginalToNewManagementContext();
+ rebind();
+
+ String allLines = getPersistanceFileContents(BrooklynObjectType.ENTITY, appId);
+ assertFalse(allLines.contains("oldFlagKey2"), "should not contain 'oldFlagKey2', allLines="+allLines);
--- End diff --
not sure I understand this test; the original persisted state already does not contain `oldFlagKey2`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #734: Support ConfigKey deprecated names
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/734#discussion_r123965177
--- Diff: core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java ---
@@ -365,6 +384,7 @@ public boolean isValueValid(T value) {
}
/** @see ConfigKey#getNameParts() */
+ @Deprecated
--- End diff --
Add `Since...` comment
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #734: Support ConfigKey deprecated names
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/734#discussion_r123979478
--- Diff: core/src/main/java/org/apache/brooklyn/util/core/config/ConfigBag.java ---
@@ -421,14 +430,22 @@ public Object getStringKey(String key) {
return get(preferredKey);
}
- /** convenience for @see #getWithDeprecation(ConfigKey[], ConfigKey...) */
+ /**
+ * Convenience for @see #getWithDeprecation(ConfigKey[], ConfigKey...).
+ *
+ * @deprecated since 0.12.0; instead define deprecated names on key, see {@link ConfigKey#getDeprecatedNames()}
+ */
public Object getWithDeprecation(ConfigKey<?> key, ConfigKey<?> ...deprecatedKeys) {
return getWithDeprecation(new ConfigKey[] { key }, deprecatedKeys);
}
- /** returns the value for the first key in the list for which a value is set,
+ /**
+ * Returns the value for the first key in the list for which a value is set,
* warning if any of the deprecated keys have a value which is different to that set on the first set current key
- * (including warning if a deprecated key has a value but no current key does) */
+ * (including warning if a deprecated key has a value but no current key does).
+ *
+ * @deprecated since 0.12.0; instead define deprecated names on key, see {@link ConfigKey#getDeprecatedNames()}
--- End diff --
Need the annotation below too.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #734: Support ConfigKey deprecated names
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/734#discussion_r125617181
--- Diff: core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java ---
@@ -216,17 +216,14 @@ public AbstractLocation configure(Map<?,?> properties) {
config().removeKey(PARENT_LOCATION);
}
- // allow config keys to be set by name (or deprecated name)
+ // Allow config keys to be set by name (or deprecated name)
//
- // Aled thinks it would be sensible to remove the consumed flags below (i.e. properties = ...).
- // However, that caused ClockerDynamicLocationPatternTest to fail because there is a field of
- // StubContainerLocation annotated with `@SetFromFlag("owner")`, as well as a config key with
- // name "owner" (and with `@SetFromFlag("owner")`) in the super-type (DynamicLocation).
- // However, that looks mad - do we really need to support it?!
- // I've preserved that behaviour (for now).
- ConfigUtilsInternal.setAllConfigKeys(properties, getLocationTypeInternal().getConfigKeys().values(), this);
+ // The resulting `properties` will no longer contain the keys that we matched;
+ // we will not also use them to for `SetFromFlag` etc.
+ properties = ConfigUtilsInternal.setAllConfigKeys(properties, getLocationTypeInternal().getConfigKeys().values(), this);
// NB: flag-setting done here must also be done in BasicLocationRebindSupport
+ // Must call setFieldsFromFlagsWithBag, even if properites is empty, so defaults are extracted from annotations
--- End diff --
typo `properites`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #734: Support ConfigKey deprecated names
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/734#discussion_r123991510
--- Diff: core/src/main/java/org/apache/brooklyn/util/core/config/ConfigBag.java ---
@@ -507,6 +525,44 @@ protected Object getStringKey(String key, boolean markUsed) {
return getStringKeyMaybe(key, markUsed).orNull();
}
+ private synchronized Maybe<Object> getRawKeyMaybe(ConfigKey<?> key, boolean markUsed) {
+ Maybe<Object> val = getRawStringKeyMaybe(key.getName(), markUsed);
+
+ String firstDeprecatedName = null;
+ Maybe<Object> firstDeprecatedVal = null;
+ for (String deprecatedName : key.getDeprecatedNames()) {
+ Maybe<Object> deprecatedVal = getRawStringKeyMaybe(deprecatedName, markUsed);
+ if (deprecatedVal.isPresent()) {
+ if (val.isPresent()) {
+ if (!Objects.equal(val.get(), deprecatedVal.get())) {
+ log.warn("Conflicting value for key "+key+" from deprecated name '"+deprecatedName+"'; "
+ + "using value from preferred name "+key.getName());
+ } else {
+ log.warn("Duplicate value for key "+key+" from deprecated name '"+deprecatedName+"'; "
+ + "using same value from preferred name "+key.getName());
+ }
+ } else if (firstDeprecatedVal.isPresent()) {
--- End diff --
is there not a potential NPE here, as `firstDeprecatedVal` is set to `null` initially, and you'll arrive here without initializing it if `val` is not present?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #734: Support ConfigKey deprecated names
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/734#discussion_r125614883
--- Diff: core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java ---
@@ -396,6 +396,9 @@ public AbstractEntity configure(Map flags) {
}
// allow config keys to be set by name (or deprecated name)
+ //
+ // The resulting `flags` will no longer contain the keys that we matched;
+ // we will not also use them to for `SetFromFlag` etc.
--- End diff --
typo "to for"
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #734: Support ConfigKey deprecated names
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/734
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #734: Support ConfigKey deprecated names
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/734#discussion_r123979492
--- Diff: core/src/main/java/org/apache/brooklyn/util/core/config/ConfigBag.java ---
@@ -421,14 +430,22 @@ public Object getStringKey(String key) {
return get(preferredKey);
}
- /** convenience for @see #getWithDeprecation(ConfigKey[], ConfigKey...) */
+ /**
+ * Convenience for @see #getWithDeprecation(ConfigKey[], ConfigKey...).
+ *
+ * @deprecated since 0.12.0; instead define deprecated names on key, see {@link ConfigKey#getDeprecatedNames()}
--- End diff --
Need the `@Deprecated` annotation below too.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---