You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by tbouron <gi...@git.apache.org> on 2016/09/28 11:47:03 UTC
[GitHub] brooklyn-server pull request #358: Allow REST API to return the config key's...
GitHub user tbouron opened a pull request:
https://github.com/apache/brooklyn-server/pull/358
Allow REST API to return the config key's constraints for catalog apps
Based on https://github.com/apache/brooklyn-server/pull/354 (needs to be merged first)
This changes the underlying structure of how the constraints are stored for the config keys. Instead of a single field `Predicates.and(Predicates...)`, there is a `Map<String, Predicate>` to store the keys and predicates to use. This allows the REST API to returns what constraint (keys) a config key is using.
The validation still do a `AND` to validate the value.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/tbouron/brooklyn-server fix/parameter-contraints
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/358.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 #358
----
commit d02e73ea87c60d517ac146c3888a7796ee74f7a3
Author: Thomas Bouron <th...@cloudsoftcorp.com>
Date: 2016-09-27T11:06:32Z
Config keys and parameters can now be marked as "pinned" regardless of their priority
commit bdd1f726cf4af3bfc9e3f66191cbf4b77520f243
Author: Thomas Bouron <th...@cloudsoftcorp.com>
Date: 2016-09-28T11:38:12Z
Expose config keys constraints on the REST API
----
---
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 #358: Allow REST API to return the config key's...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81122326
--- Diff: api/src/main/java/org/apache/brooklyn/api/catalog/CatalogConfig.java ---
@@ -34,5 +34,7 @@
* a higher value appears higher in the list. the default is 1.
* (negative values may be used to indicate advanced config which might not be shown unless requested.) */
double priority() default 1;
-
+
+ /** a pinned configuration means that it should be displayed, regardless of its priority */
+ boolean pinned() default false;
--- End diff --
Yeah, make sense. I'll update the Javadoc
---
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 #358: Allow REST API to return the pinned config key a...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/358
Looks great @tbouron. Can you squash then good to merge.
---
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 #358: Allow REST API to return the config key's...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81124282
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/EntityTransformer.java ---
@@ -162,7 +163,7 @@ public static EntityConfigSummary entityConfigSummary(Entity entity, ConfigKey<?
SensorTransformer.addNamedAction(lb, na, entity.getConfig(config), config, entity);
}
- return entityConfigSummary(config, label, priority, lb.build());
+ return entityConfigSummary(config, label, priority, false, lb.build());
--- End diff --
This will mark all config as invisible in `v1/applications/xxxxxxxx/entities/yyyyyyyy/config`. That's fine as long as the UI know about this caveat. Might be better to make this field nullable and set `null` here?
---
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 #358: Allow REST API to return the pinned confi...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/358
---
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 #358: Allow REST API to return the pinned confi...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81188720
--- Diff: core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java ---
@@ -184,7 +192,7 @@ public String getDescription() {
protected boolean reconfigurable;
protected ConfigInheritance typeInheritance;
protected ConfigInheritance runtimeInheritance;
- protected Predicate<? super T> constraint;
+ protected List<Predicate<? super T>> constraints;
--- End diff --
But this is the whole point of this change, expose the constraints as a list of string on the REST API. I thought about doing the change you are suggesting @neykov (looking into the `Predicates.and()`) but there is no way to get back the underlying predicates.
Reverting would mean to support only one predicate at a time which is a contradiction with the current YAML implementation (can specify more than one)
---
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 #358: Allow REST API to return the pinned confi...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81334424
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java ---
@@ -375,12 +376,14 @@ public static WeightedParameter getFieldConfig(ConfigKey<?> config, Field config
CatalogConfig catalogConfig = configKeyField.getAnnotation(CatalogConfig.class);
String label = config.getName();
Double priority = null;
+ Boolean pinned = Boolean.FALSE;
--- End diff --
I take it back - non-anotated items should be unpinned.
---
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 #358: Allow REST API to return the pinned config key a...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/358
In summary - looks good. Only concerned about introducing `constraints` which breaks persistence compatibility. Suggest reverting all related changes and making the predicate -> string conversion smarter to look inside `and` predicates.
---
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 #358: Allow REST API to return the pinned config key a...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/358
Unfortunately it doesn't work for Apache projects. The github repo is a read-only copy of the Apache repo where this will get merged. Also can't use the "share branch with committers" feature.
---
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 #358: Allow REST API to return the config key's constr...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/358
@neykov Comment addressed
---
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 #358: Allow REST API to return the config key's...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81126432
--- Diff: core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java ---
@@ -331,12 +337,19 @@ public ConfigInheritance getParentInheritance() {
return parentInheritance;
}
+ /** @see ConfigKey#getConstraints() */
+ @Nonnull
+ @Override
+ public Map<String, Predicate<? super T>> getConstraints() {
+ return constraints;
+ }
+
/** @see ConfigKey#getConstraint() */
@Override @Nonnull
public Predicate<? super T> getConstraint() {
// Could be null after rebinding
- if (constraint != null) {
- return constraint;
+ if (constraints.size() > 0) {
+ return Predicates.and(constraints.values());
} else {
return Predicates.alwaysTrue();
}
--- End diff --
agree
---
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 #358: Allow REST API to return the pinned config key a...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/358
LGTM - very minor comments only.
---
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 #358: Allow REST API to return the config key's...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81122190
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java ---
@@ -247,17 +248,17 @@ public String toString() {
.name(name)
.description(description)
.defaultValue(defaultValue)
- .constraint(constraints)
+ .constraints(constraints)
.runtimeInheritance(parentInheritance)
.typeInheritance(typeInheritance);
-
+
if (PortRange.class.equals(typeToken.getRawType())) {
sensorType = new PortAttributeSensorAndConfigKey(builder);
configType = ((HasConfigKey)sensorType).getConfigKey();
} else {
configType = builder.build();
}
- return new BasicSpecParameter(Objects.firstNonNull(label, name), true, configType, sensorType);
+ return new BasicSpecParameter(Objects.firstNonNull(label, name), Objects.firstNonNull(pinned, Boolean.FALSE), configType, sensorType);
--- End diff --
The usual way to express this in the codebase is: `Boolean.TRUE.equals(pinned)`, but this works as well.
---
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 #358: Allow REST API to return the pinned config key a...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/358
Thanks, merging.
---
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 #358: Allow REST API to return the config key's...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81122524
--- Diff: api/src/main/java/org/apache/brooklyn/api/catalog/CatalogConfig.java ---
@@ -34,5 +34,7 @@
* a higher value appears higher in the list. the default is 1.
* (negative values may be used to indicate advanced config which might not be shown unless requested.) */
double priority() default 1;
-
+
+ /** a pinned configuration means that it should be displayed, regardless of its priority */
+ boolean pinned() default false;
--- End diff --
This will break backwards compatibility by hiding all already annotated fields.
---
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 #358: Allow REST API to return the pinned confi...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81181965
--- Diff: camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/SpecParameterParsingTest.java ---
@@ -66,19 +66,20 @@ public void testYamlInputsParsed() {
" - simple",
" - name: explicit_name",
" - name: third_input",
- " type: integer");
+ " type: integer",
+ " pinned: true");
EntitySpec<?> item = mgmt().getTypeRegistry().createSpec(mgmt().getTypeRegistry().get(itemId), null, EntitySpec.class);
List<SpecParameter<?>> inputs = item.getParameters();
assertEquals(inputs.size(), 6);
SpecParameter<?> firstInput = inputs.get(0);
assertEquals(firstInput.getLabel(), "simple");
- assertEquals(firstInput.isPinned(), true);
+ assertEquals(firstInput.isPinned(), false);
--- End diff --
@neykov addressed
---
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 #358: Allow REST API to return the pinned confi...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81173978
--- Diff: camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/SpecParameterParsingTest.java ---
@@ -66,19 +66,20 @@ public void testYamlInputsParsed() {
" - simple",
" - name: explicit_name",
" - name: third_input",
- " type: integer");
+ " type: integer",
+ " pinned: true");
EntitySpec<?> item = mgmt().getTypeRegistry().createSpec(mgmt().getTypeRegistry().get(itemId), null, EntitySpec.class);
List<SpecParameter<?>> inputs = item.getParameters();
assertEquals(inputs.size(), 6);
SpecParameter<?> firstInput = inputs.get(0);
assertEquals(firstInput.getLabel(), "simple");
- assertEquals(firstInput.isPinned(), true);
+ assertEquals(firstInput.isPinned(), false);
--- End diff --
I changed the annotation `@CatalogConfig`, forgot about the YAML parsing... Updating 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 #358: Allow REST API to return the config key's...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81124480
--- Diff: core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java ---
@@ -331,12 +337,19 @@ public ConfigInheritance getParentInheritance() {
return parentInheritance;
}
+ /** @see ConfigKey#getConstraints() */
+ @Nonnull
+ @Override
+ public Map<String, Predicate<? super T>> getConstraints() {
+ return constraints;
+ }
+
/** @see ConfigKey#getConstraint() */
@Override @Nonnull
public Predicate<? super T> getConstraint() {
// Could be null after rebinding
- if (constraint != null) {
- return constraint;
+ if (constraints.size() > 0) {
+ return Predicates.and(constraints.values());
} else {
return Predicates.alwaysTrue();
}
--- End diff --
Another solution would be to create a specific interface for constraints but it felt over-engineered. That's why I kept the basic `Map<String, Predicate>`
---
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 #358: Allow REST API to return the pinned confi...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81184480
--- Diff: rest/rest-api/src/main/java/org/apache/brooklyn/rest/domain/EntityConfigSummary.java ---
@@ -18,43 +18,82 @@
*/
package org.apache.brooklyn.rest.domain;
-import com.fasterxml.jackson.annotation.JsonProperty;
-import com.fasterxml.jackson.databind.annotation.JsonSerialize;
-import com.google.common.collect.ImmutableMap;
-
-import org.apache.brooklyn.config.ConfigKey;
-
import java.net.URI;
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import javax.annotation.Nullable;
+
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.util.text.StringPredicates;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import com.google.common.base.Function;
+import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
+
public class EntityConfigSummary extends ConfigSummary {
private static final long serialVersionUID = -1336134336883426030L;
--- End diff --
Regenerate after adding members.
---
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 #358: Allow REST API to return the config key's...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81126447
--- Diff: api/src/main/java/org/apache/brooklyn/api/catalog/CatalogConfig.java ---
@@ -34,5 +34,7 @@
* a higher value appears higher in the list. the default is 1.
* (negative values may be used to indicate advanced config which might not be shown unless requested.) */
double priority() default 1;
-
+
+ /** a pinned configuration means that it should be displayed, regardless of its priority */
+ boolean pinned() default false;
--- End diff --
Ok, I'll update
---
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 #358: Allow REST API to return the config key's...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81126206
--- Diff: core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java ---
@@ -331,12 +337,19 @@ public ConfigInheritance getParentInheritance() {
return parentInheritance;
}
+ /** @see ConfigKey#getConstraints() */
+ @Nonnull
+ @Override
+ public Map<String, Predicate<? super T>> getConstraints() {
+ return constraints;
+ }
+
/** @see ConfigKey#getConstraint() */
@Override @Nonnull
public Predicate<? super T> getConstraint() {
// Could be null after rebinding
- if (constraint != null) {
- return constraint;
+ if (constraints.size() > 0) {
+ return Predicates.and(constraints.values());
} else {
return Predicates.alwaysTrue();
}
--- End diff --
OK, that makes sense. So to recap:
- `StringPredicates.isNonBlank()` becomes `required` for the REST API
- Any other predicate becomes the FQDN of the predicate? Sounds sensible @neykov ?
---
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 #358: Allow REST API to return the pinned confi...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81185397
--- Diff: rest/rest-api/src/main/java/org/apache/brooklyn/rest/domain/EntityConfigSummary.java ---
@@ -18,43 +18,82 @@
*/
package org.apache.brooklyn.rest.domain;
-import com.fasterxml.jackson.annotation.JsonProperty;
-import com.fasterxml.jackson.databind.annotation.JsonSerialize;
-import com.google.common.collect.ImmutableMap;
-
-import org.apache.brooklyn.config.ConfigKey;
-
import java.net.URI;
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import javax.annotation.Nullable;
+
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.util.text.StringPredicates;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import com.google.common.base.Function;
+import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
+
public class EntityConfigSummary extends ConfigSummary {
private static final long serialVersionUID = -1336134336883426030L;
@JsonSerialize(include = JsonSerialize.Inclusion.NON_NULL)
+ private final Boolean pinned;
+
+ @JsonSerialize(include = JsonSerialize.Inclusion.NON_NULL)
+ private final List<String> constraints;
+
+ @JsonSerialize(include = JsonSerialize.Inclusion.NON_NULL)
private final Map<String, URI> links;
public EntityConfigSummary(
@JsonProperty("name") String name,
@JsonProperty("type") String type,
@JsonProperty("description") String description,
@JsonProperty("defaultValue") Object defaultValue,
- @JsonProperty("reconfigurable") boolean reconfigurable,
+ @JsonProperty("reconfigurable") Boolean reconfigurable,
--- End diff --
Why change this?
---
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 #358: Allow REST API to return the config key's...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81124997
--- Diff: core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java ---
@@ -331,12 +337,19 @@ public ConfigInheritance getParentInheritance() {
return parentInheritance;
}
+ /** @see ConfigKey#getConstraints() */
+ @Nonnull
+ @Override
+ public Map<String, Predicate<? super T>> getConstraints() {
+ return constraints;
+ }
+
/** @see ConfigKey#getConstraint() */
@Override @Nonnull
public Predicate<? super T> getConstraint() {
// Could be null after rebinding
- if (constraint != null) {
- return constraint;
+ if (constraints.size() > 0) {
+ return Predicates.and(constraints.values());
} else {
return Predicates.alwaysTrue();
}
--- End diff --
What we care about is the "required" as implemented in yaml (`isNonBlank`). It's enought to support that, but could easily add some more to a white-list. Also alias to it as a "required" predicate to be used in Java if anyone wants to integrate with UI better from Java land.
Also keep in mind backwards compatibility (always :) ). Instead of changing the API straight away we'd have to go through a deprecation cycle where the old signatures are still supported. That's one more reason why stick to the existing non-labeled predicates.
---
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 #358: Allow REST API to return the config key's...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81125194
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/EntityTransformer.java ---
@@ -162,7 +163,7 @@ public static EntityConfigSummary entityConfigSummary(Entity entity, ConfigKey<?
SensorTransformer.addNamedAction(lb, na, entity.getConfig(config), config, entity);
}
- return entityConfigSummary(config, label, priority, lb.build());
+ return entityConfigSummary(config, label, priority, false, lb.build());
--- End diff --
Well, yes and no. I mean, the UI does not check a `pinned` flag for managed entities but only for catalog templates. So technically, config will never be invisible.
However, I do agree that the REST API shouldn't return a `pinned: false` for this case for I'll update for a nullable field.
---
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 #358: Allow REST API to return the config key's...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81124231
--- Diff: core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java ---
@@ -331,12 +337,19 @@ public ConfigInheritance getParentInheritance() {
return parentInheritance;
}
+ /** @see ConfigKey#getConstraints() */
+ @Nonnull
+ @Override
+ public Map<String, Predicate<? super T>> getConstraints() {
+ return constraints;
+ }
+
/** @see ConfigKey#getConstraint() */
@Override @Nonnull
public Predicate<? super T> getConstraint() {
// Could be null after rebinding
- if (constraint != null) {
- return constraint;
+ if (constraints.size() > 0) {
+ return Predicates.and(constraints.values());
} else {
return Predicates.alwaysTrue();
}
--- End diff --
Thought about that but there is issue as you can have more than one implementation of `Predicate` to do the same thing. In the case of `required`, I have seen implemented as:
- `StringPredicates.isNonBlank()`
- `Predicates.notNull()`
And we could potential have others
---
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 #358: Allow REST API to return the pinned config key a...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/358
@neykov You can do it from the [Github UI directly](https://help.github.com/articles/merging-a-pull-request/#merging-a-pull-request-using-the-github-web-interface) ;)
---
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 #358: Allow REST API to return the config key's...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81121259
--- Diff: api/src/main/java/org/apache/brooklyn/api/catalog/CatalogConfig.java ---
@@ -34,5 +34,7 @@
* a higher value appears higher in the list. the default is 1.
* (negative values may be used to indicate advanced config which might not be shown unless requested.) */
double priority() default 1;
-
+
+ /** a pinned configuration means that it should be displayed, regardless of its priority */
+ boolean pinned() default false;
--- End diff --
The `@CatalogConfig` annotation is set on items that we want visible in the catalog. To prevent an item from being seen by default one can set a negative priority above. This allows for having an order of the keys and at the same time declares whether they should be visible or not. Could keep the existing behaviour and not expose `pinned` here (derive it instead from priority). But having `pinned` might be more user friendly so I'm fine keeping it as is.
Can you update the javadoc above - doesn't make sense to support negative priorities any more. They make documenting and understanding the behaviour more difficult when paired with `pinned`. Let's keep the behaviour for backwards compatibility, but not advertise it.
---
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 #358: Allow REST API to return the pinned config key a...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/358
@neykov Comments addressed
---
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 #358: Allow REST API to return the pinned confi...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81173329
--- Diff: camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/SpecParameterParsingTest.java ---
@@ -66,19 +66,20 @@ public void testYamlInputsParsed() {
" - simple",
" - name: explicit_name",
" - name: third_input",
- " type: integer");
+ " type: integer",
+ " pinned: true");
EntitySpec<?> item = mgmt().getTypeRegistry().createSpec(mgmt().getTypeRegistry().get(itemId), null, EntitySpec.class);
List<SpecParameter<?>> inputs = item.getParameters();
assertEquals(inputs.size(), 6);
SpecParameter<?> firstInput = inputs.get(0);
assertEquals(firstInput.getLabel(), "simple");
- assertEquals(firstInput.isPinned(), true);
+ assertEquals(firstInput.isPinned(), false);
--- End diff --
How come the tests still pass with this change? Am I missing something?
The default is flipped to `true` now, so the change shouldn't be needed?
---
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 #358: Allow REST API to return the pinned config key a...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/358
@neykov Ah ok, I didn't know. Squashed
---
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 #358: Allow REST API to return the pinned config key a...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/358
@neykov updated
---
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 #358: Allow REST API to return the config key's constr...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/358
FYI, I tested before pushing. I also just retested locally and all tests pass. Not sure what is going on here
---
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 #358: Allow REST API to return the pinned confi...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81334163
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java ---
@@ -375,12 +376,14 @@ public static WeightedParameter getFieldConfig(ConfigKey<?> config, Field config
CatalogConfig catalogConfig = configKeyField.getAnnotation(CatalogConfig.class);
String label = config.getName();
Double priority = null;
+ Boolean pinned = Boolean.FALSE;
--- End diff --
Shouldn't the default be `true`?
---
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 #358: Allow REST API to return the pinned confi...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81183657
--- Diff: core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java ---
@@ -184,7 +192,7 @@ public String getDescription() {
protected boolean reconfigurable;
protected ConfigInheritance typeInheritance;
protected ConfigInheritance runtimeInheritance;
- protected Predicate<? super T> constraint;
+ protected List<Predicate<? super T>> constraints;
--- End diff --
This is persisted so should be extra careful with renames. When passing constraints from java the caller can wrap them in `and` if multiple, so this change is not needed. Suggest reverting.
---
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 #358: Allow REST API to return the pinned confi...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81185833
--- Diff: rest/rest-api/src/main/java/org/apache/brooklyn/rest/domain/EntityConfigSummary.java ---
@@ -18,43 +18,82 @@
*/
package org.apache.brooklyn.rest.domain;
-import com.fasterxml.jackson.annotation.JsonProperty;
-import com.fasterxml.jackson.databind.annotation.JsonSerialize;
-import com.google.common.collect.ImmutableMap;
-
-import org.apache.brooklyn.config.ConfigKey;
-
import java.net.URI;
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import javax.annotation.Nullable;
+
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.util.text.StringPredicates;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import com.google.common.base.Function;
+import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
+
public class EntityConfigSummary extends ConfigSummary {
private static final long serialVersionUID = -1336134336883426030L;
@JsonSerialize(include = JsonSerialize.Inclusion.NON_NULL)
+ private final Boolean pinned;
+
+ @JsonSerialize(include = JsonSerialize.Inclusion.NON_NULL)
+ private final List<String> constraints;
+
+ @JsonSerialize(include = JsonSerialize.Inclusion.NON_NULL)
private final Map<String, URI> links;
public EntityConfigSummary(
@JsonProperty("name") String name,
@JsonProperty("type") String type,
@JsonProperty("description") String description,
@JsonProperty("defaultValue") Object defaultValue,
- @JsonProperty("reconfigurable") boolean reconfigurable,
+ @JsonProperty("reconfigurable") Boolean reconfigurable,
@JsonProperty("label") String label,
@JsonProperty("priority") Double priority,
@JsonProperty("possibleValues") List<Map<String, String>> possibleValues,
+ @JsonProperty("pinned") Boolean pinned,
+ @JsonProperty("constraints") List<String> constraints,
@JsonProperty("links") Map<String, URI> links) {
super(name, type, description, defaultValue, reconfigurable, label, priority, possibleValues);
+ this.pinned = pinned;
+ this.constraints = (constraints == null) ? ImmutableList.<String>of() : ImmutableList.copyOf(constraints);
this.links = (links == null) ? ImmutableMap.<String, URI>of() : ImmutableMap.copyOf(links);
}
- public EntityConfigSummary(ConfigKey<?> config, String label, Double priority, Map<String, URI> links) {
+ public EntityConfigSummary(ConfigKey<?> config, String label, Double priority, Boolean pinned, Map<String, URI> links) {
super(config, label, priority);
+ this.pinned = pinned;
+ this.constraints = ImmutableList.copyOf(Iterables.transform(config.getConstraints(), new Function<Predicate, String>() {
+ @Nullable
+ @Override
+ public String apply(@Nullable Predicate predicate) {
+ if (predicate == null) {
+ return null;
+ } else if (predicate.getClass().getName().equals(StringPredicates.isNonBlank().getClass().getName())) {
--- End diff --
Could even compare the classes without going through the name, but doesn't really matter in the end.
---
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 #358: Allow REST API to return the pinned confi...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81332816
--- Diff: core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java ---
@@ -184,7 +192,7 @@ public String getDescription() {
protected boolean reconfigurable;
protected ConfigInheritance typeInheritance;
protected ConfigInheritance runtimeInheritance;
- protected Predicate<? super T> constraint;
+ protected List<Predicate<? super T>> constraints;
--- End diff --
Reverted. I'll try to address the multiple predicate issue in another PR
---
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 #358: Allow REST API to return the config key's...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81121933
--- Diff: core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java ---
@@ -331,12 +337,19 @@ public ConfigInheritance getParentInheritance() {
return parentInheritance;
}
+ /** @see ConfigKey#getConstraints() */
+ @Nonnull
+ @Override
+ public Map<String, Predicate<? super T>> getConstraints() {
+ return constraints;
+ }
+
/** @see ConfigKey#getConstraint() */
@Override @Nonnull
public Predicate<? super T> getConstraint() {
// Could be null after rebinding
- if (constraint != null) {
- return constraint;
+ if (constraints.size() > 0) {
+ return Predicates.and(constraints.values());
} else {
return Predicates.alwaysTrue();
}
--- End diff --
Having a name to describe the predicate duplicates the information coming from the predicate itself. Suggest deciding on whether the predicate means "required" based on its type name in the REST API.
---
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 #358: Allow REST API to return the config key's...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81123322
--- Diff: api/src/main/java/org/apache/brooklyn/api/catalog/CatalogConfig.java ---
@@ -34,5 +34,7 @@
* a higher value appears higher in the list. the default is 1.
* (negative values may be used to indicate advanced config which might not be shown unless requested.) */
double priority() default 1;
-
+
+ /** a pinned configuration means that it should be displayed, regardless of its priority */
+ boolean pinned() default false;
--- End diff --
I would rather modify https://github.com/apache/brooklyn-library/pull/65 to add `pinned = true` to all annotated field, than changing this to `true`. Mark by default annotated fields as pinned feels wrong
---
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 #358: Allow REST API to return the pinned confi...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81185580
--- Diff: rest/rest-api/src/main/java/org/apache/brooklyn/rest/domain/EntityConfigSummary.java ---
@@ -18,43 +18,82 @@
*/
package org.apache.brooklyn.rest.domain;
-import com.fasterxml.jackson.annotation.JsonProperty;
-import com.fasterxml.jackson.databind.annotation.JsonSerialize;
-import com.google.common.collect.ImmutableMap;
-
-import org.apache.brooklyn.config.ConfigKey;
-
import java.net.URI;
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import javax.annotation.Nullable;
+
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.util.text.StringPredicates;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import com.google.common.base.Function;
+import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
+
public class EntityConfigSummary extends ConfigSummary {
private static final long serialVersionUID = -1336134336883426030L;
@JsonSerialize(include = JsonSerialize.Inclusion.NON_NULL)
+ private final Boolean pinned;
+
+ @JsonSerialize(include = JsonSerialize.Inclusion.NON_NULL)
+ private final List<String> constraints;
+
+ @JsonSerialize(include = JsonSerialize.Inclusion.NON_NULL)
private final Map<String, URI> links;
public EntityConfigSummary(
@JsonProperty("name") String name,
@JsonProperty("type") String type,
@JsonProperty("description") String description,
@JsonProperty("defaultValue") Object defaultValue,
- @JsonProperty("reconfigurable") boolean reconfigurable,
+ @JsonProperty("reconfigurable") Boolean reconfigurable,
@JsonProperty("label") String label,
@JsonProperty("priority") Double priority,
@JsonProperty("possibleValues") List<Map<String, String>> possibleValues,
+ @JsonProperty("pinned") Boolean pinned,
+ @JsonProperty("constraints") List<String> constraints,
@JsonProperty("links") Map<String, URI> links) {
super(name, type, description, defaultValue, reconfigurable, label, priority, possibleValues);
+ this.pinned = pinned;
+ this.constraints = (constraints == null) ? ImmutableList.<String>of() : ImmutableList.copyOf(constraints);
this.links = (links == null) ? ImmutableMap.<String, URI>of() : ImmutableMap.copyOf(links);
}
- public EntityConfigSummary(ConfigKey<?> config, String label, Double priority, Map<String, URI> links) {
+ public EntityConfigSummary(ConfigKey<?> config, String label, Double priority, Boolean pinned, Map<String, URI> links) {
super(config, label, priority);
+ this.pinned = pinned;
+ this.constraints = ImmutableList.copyOf(Iterables.transform(config.getConstraints(), new Function<Predicate, String>() {
+ @Nullable
+ @Override
+ public String apply(@Nullable Predicate predicate) {
+ if (predicate == null) {
+ return null;
+ } else if (predicate.getClass().getName().equals(StringPredicates.isNonBlank().getClass().getName())) {
--- End diff --
Minor, could assign `StringPredicates.isNonBlank().getClass().getName()` to a static field of the anonymous class.
---
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 #358: Allow REST API to return the pinned confi...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81188889
--- Diff: rest/rest-api/src/main/java/org/apache/brooklyn/rest/domain/EntityConfigSummary.java ---
@@ -18,43 +18,82 @@
*/
package org.apache.brooklyn.rest.domain;
-import com.fasterxml.jackson.annotation.JsonProperty;
-import com.fasterxml.jackson.databind.annotation.JsonSerialize;
-import com.google.common.collect.ImmutableMap;
-
-import org.apache.brooklyn.config.ConfigKey;
-
import java.net.URI;
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import javax.annotation.Nullable;
+
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.util.text.StringPredicates;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import com.google.common.base.Function;
+import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
+
public class EntityConfigSummary extends ConfigSummary {
private static final long serialVersionUID = -1336134336883426030L;
@JsonSerialize(include = JsonSerialize.Inclusion.NON_NULL)
+ private final Boolean pinned;
+
+ @JsonSerialize(include = JsonSerialize.Inclusion.NON_NULL)
+ private final List<String> constraints;
+
+ @JsonSerialize(include = JsonSerialize.Inclusion.NON_NULL)
private final Map<String, URI> links;
public EntityConfigSummary(
@JsonProperty("name") String name,
@JsonProperty("type") String type,
@JsonProperty("description") String description,
@JsonProperty("defaultValue") Object defaultValue,
- @JsonProperty("reconfigurable") boolean reconfigurable,
+ @JsonProperty("reconfigurable") Boolean reconfigurable,
--- End diff --
That's a mistake, I'll revert back
---
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 #358: Allow REST API to return the config key's...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81122413
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java ---
@@ -247,17 +248,17 @@ public String toString() {
.name(name)
.description(description)
.defaultValue(defaultValue)
- .constraint(constraints)
+ .constraints(constraints)
.runtimeInheritance(parentInheritance)
.typeInheritance(typeInheritance);
-
+
if (PortRange.class.equals(typeToken.getRawType())) {
sensorType = new PortAttributeSensorAndConfigKey(builder);
configType = ((HasConfigKey)sensorType).getConfigKey();
} else {
configType = builder.build();
}
- return new BasicSpecParameter(Objects.firstNonNull(label, name), true, configType, sensorType);
+ return new BasicSpecParameter(Objects.firstNonNull(label, name), Objects.firstNonNull(pinned, Boolean.FALSE), configType, sensorType);
--- End diff --
I prefer to comply with the "standard". I'll update it
---
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 #358: Allow REST API to return the pinned confi...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81333940
--- Diff: core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java ---
@@ -422,15 +422,15 @@ public BasicConfigKeyOverwriting(Builder<T,?> builder, ConfigKey<T> parent) {
parentKey = parent;
Preconditions.checkArgument(Objects.equal(builder.name, parent.getName()), "Builder must use key of the same name.");
}
-
+
public BasicConfigKeyOverwriting(ConfigKey<T> key, T defaultValue) {
this(builder(key).defaultValue(defaultValue), key);
}
-
+
public BasicConfigKeyOverwriting(ConfigKey<T> key, String newDescription, T defaultValue) {
this(builder(key).description(newDescription).defaultValue(defaultValue), key);
}
-
+
--- End diff --
No non-white space changes in the file, can you remove from the change set.
---
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 #358: Allow REST API to return the config key's...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81126063
--- Diff: api/src/main/java/org/apache/brooklyn/api/catalog/CatalogConfig.java ---
@@ -34,5 +34,7 @@
* a higher value appears higher in the list. the default is 1.
* (negative values may be used to indicate advanced config which might not be shown unless requested.) */
double priority() default 1;
-
+
+ /** a pinned configuration means that it should be displayed, regardless of its priority */
+ boolean pinned() default false;
--- End diff --
It's not only `brooklyn-library`. Users of Brooklyn writing their own entities will have the same problem. The code will compile cleanly without warnings (even will be binary compatible for existing jars). The fields will just be invisible in UI.
Same applies (even stronger) to yaml. Making all fields invisible by default will break all existing blueprints.
For Java applying a `@CatalogConfig` annotation in 99% of the cases is done to show parameters in UI. In YAML adding top-level parameters is done for the same reason. That's why I think existing behaviour makes sense and we should keep it.
---
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 #358: Allow REST API to return the pinned confi...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/358#discussion_r81174007
--- Diff: camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/SpecParameterParsingTest.java ---
@@ -66,19 +66,20 @@ public void testYamlInputsParsed() {
" - simple",
" - name: explicit_name",
" - name: third_input",
- " type: integer");
+ " type: integer",
+ " pinned: true");
EntitySpec<?> item = mgmt().getTypeRegistry().createSpec(mgmt().getTypeRegistry().get(itemId), null, EntitySpec.class);
List<SpecParameter<?>> inputs = item.getParameters();
assertEquals(inputs.size(), 6);
SpecParameter<?> firstInput = inputs.get(0);
assertEquals(firstInput.getLabel(), "simple");
- assertEquals(firstInput.isPinned(), true);
+ assertEquals(firstInput.isPinned(), false);
--- End diff --
Oh I see, the default when coming parsing yaml is still `false`, can you change it to `true` as discussed above.
---
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.
---