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/09/13 09:07:15 UTC
[GitHub] brooklyn-server pull request #819: Improve config key descriptions
GitHub user aledsage opened a pull request:
https://github.com/apache/brooklyn-server/pull/819
Improve config key descriptions
See individual commits for details.
Note the last two commits make significant changes to config:
* remove the default for `launch.command` of `VanillaSoftwareProcess`.
* Rename some config keys (deprecating the old names) for `latch.*` and `skip.*`.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/aledsage/brooklyn-server config-key-descriptions
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/819.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 #819
----
----
---
[GitHub] brooklyn-server pull request #819: Improve config key descriptions
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/819#discussion_r138818482
--- Diff: core/src/main/java/org/apache/brooklyn/core/entity/BrooklynConfigKeys.java ---
@@ -88,40 +93,48 @@
* <p>
* If this key is set on a {@link Location} then all entities in that location will be treated in this way, again as with {@link #ENTITY_STARTED}.
*
- * @see #ENTITY_STARTED
+ * @see #SKIP_ENTITY_START
*/
- public static final ConfigKey<Boolean> SKIP_ENTITY_START_IF_RUNNING = newBooleanConfigKey("entity.running", "Skip the startup process entirely, if service already running");
+ public static final ConfigKey<Boolean> SKIP_ENTITY_START_IF_RUNNING = ConfigKeys.builder(Boolean.class)
+ .name("skip.start.ifRunning")
+ .deprecatedNames("entity.running")
+ .description("Whether to skip the startup process entirely, but only if it already running")
--- End diff --
`Whether to skip the startup process if the entity is detected as already running` ?
---
[GitHub] brooklyn-server pull request #819: Improve config key descriptions
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/819
---
[GitHub] brooklyn-server issue #819: Improve config key descriptions
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/819
@aledsage I just realised that you miss one more bom file in `karaf/init/src/main/resources/catalog-classes.bom`. This one is the base for the karaf distribution.
(I think we should reuse the same base bom files for the classic and Karaf distributions, but not in the scope of this PR)
---
[GitHub] brooklyn-server pull request #819: Improve config key descriptions
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/819#discussion_r138585632
--- Diff: core/src/main/java/org/apache/brooklyn/entity/group/DynamicFabric.java ---
@@ -50,13 +51,20 @@
+ "for each given location).",
true);
+ @CatalogConfig(label = "Member spec")
@SetFromFlag("memberSpec")
ConfigKey<EntitySpec<?>> MEMBER_SPEC = ConfigKeys.newConfigKey(
- new TypeToken<EntitySpec<?>>() {}, "dynamiccfabric.memberspec", "entity spec for creating new cluster members", null);
+ new TypeToken<EntitySpec<?>>() {},
+ "dynamiccfabric.memberspec",
+ "entity spec for creating new members (one per location)",
--- End diff --
s/entity/Entity/
---
[GitHub] brooklyn-server issue #819: Improve config key descriptions
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/819
@aledsage Ok to me 👍 (build failure looks unrelated)
---
[GitHub] brooklyn-server pull request #819: Improve config key descriptions
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/819#discussion_r138818743
--- Diff: core/src/main/java/org/apache/brooklyn/enricher/stock/Joiner.java ---
@@ -47,31 +47,42 @@
private static final Logger LOG = LoggerFactory.getLogger(Joiner.class);
public static final ConfigKey<Entity> PRODUCER = ConfigKeys.newConfigKey(Entity.class,
- "enricher.producer");
+ "enricher.producer",
+ "The entity that has the source sensors (defaults to the entity that the enricher is attached to)");
+
public static final ConfigKey<Sensor<?>> SOURCE_SENSOR = ConfigKeys.newConfigKey(new TypeToken<Sensor<?>>() {},
- "enricher.sourceSensor");
+ "enricher.sourceSensor",
+ "The sensor (expected to be of type Map or Iterable) whose change triggers re-evaluation of the target value");
+
public static final ConfigKey<Sensor<?>> TARGET_SENSOR = ConfigKeys.newConfigKey(new TypeToken<Sensor<?>>() {},
- "enricher.targetSensor");
+ "enricher.targetSensor",
+ "The sensor that will be set on the associated entity, with the target value");
--- End diff --
`The sensor to be set on the associated entity with the value computed here` ?
---
[GitHub] brooklyn-server issue #819: Improve config key descriptions
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/819
@ahgittin @tbouron @drigodwin thanks for comments - incorporated in an additional commit. Is it ok to merge (with icons being separate from this PR)?
---
[GitHub] brooklyn-server pull request #819: Improve config key descriptions
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/819#discussion_r138584451
--- Diff: core/src/main/java/org/apache/brooklyn/core/entity/Attributes.java ---
@@ -52,11 +52,15 @@
// TODO these should switch to being TemplatedStringAttributeSensorAndConfigKey
BasicAttributeSensorAndConfigKey<String> DOWNLOAD_URL = new BasicAttributeSensorAndConfigKey<String>(
- String.class, "download.url", "URL pattern for downloading the installer (will substitute things like ${version} automatically)");
+ String.class,
+ "download.url",
+ "URL pattern for downloading the installer (will substitute things like ${version} automatically)");
--- End diff --
Shouldn't it just say `uses FreeMarker templating format` to be consistent with the other description below?
---
[GitHub] brooklyn-server pull request #819: Improve config key descriptions
Posted by drigodwin <gi...@git.apache.org>.
Github user drigodwin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/819#discussion_r138606016
--- Diff: core/src/main/java/org/apache/brooklyn/enricher/stock/Aggregator.java ---
@@ -65,18 +66,22 @@
"'list' (the default, putting any collection of items into a list), " +
"or 'first' (the first value, or null if empty)");
- public static final ConfigKey<Function<? super Collection<?>, ?>> TRANSFORMATION = ConfigKeys.newConfigKey(new TypeToken<Function<? super Collection<?>, ?>>() {},
- "enricher.transformation");
+ public static final ConfigKey<Function<? super Collection<?>, ?>> TRANSFORMATION = ConfigKeys.newConfigKey(
+ new TypeToken<Function<? super Collection<?>, ?>>() {},
+ "enricher.transformation",
+ "A function to be executed to evaluate the target sensor");
/**
* @see QuorumChecks
*/
public static final ConfigKey<String> QUORUM_CHECK_TYPE = ConfigKeys.newStringConfigKey(
- "quorum.check.type", "The requirement to be considered quorate -- possible values: " +
+ "quorum.check.type",
+ "The requirement to be considered quorate (used with transformation of type 'isQuorate') -- possible values: " +
"'all', 'allAndAtLeastOne', 'atLeastOne', 'atLeastOneUnlessEmpty', 'alwaysHealthy'", "allAndAtLeastOne");
public static final ConfigKey<Integer> QUORUM_TOTAL_SIZE = ConfigKeys.newIntegerConfigKey(
- "quorum.total.size", "The total size to consider when determining if quorate", 1);
+ "quorum.total.size",
+ "The total size to consider when determining if quorate (used iwth transformation of type 'isQuorate')", 1);
--- End diff --
typo iwth -> with
---
[GitHub] brooklyn-server pull request #819: Improve config key descriptions
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/819#discussion_r138584470
--- Diff: core/src/main/java/org/apache/brooklyn/core/entity/Attributes.java ---
@@ -52,11 +52,15 @@
// TODO these should switch to being TemplatedStringAttributeSensorAndConfigKey
BasicAttributeSensorAndConfigKey<String> DOWNLOAD_URL = new BasicAttributeSensorAndConfigKey<String>(
- String.class, "download.url", "URL pattern for downloading the installer (will substitute things like ${version} automatically)");
+ String.class,
+ "download.url",
+ "URL pattern for downloading the installer (will substitute things like ${version} automatically)");
@SuppressWarnings({ "unchecked", "rawtypes" })
BasicAttributeSensorAndConfigKey<Map<String,String>> DOWNLOAD_ADDON_URLS = new BasicAttributeSensorAndConfigKey(
- Map.class, "download.addon.urls", "URL patterns for downloading named add-ons (will substitute things like ${version} automatically)");
+ Map.class,
+ "download.addon.urls",
+ "URL patterns for downloading named add-ons (will substitute things like ${version} automatically)");
--- End diff --
Shouldn't it just say `uses FreeMarker templating format` to be consistent with the other description below?
---
[GitHub] brooklyn-server issue #819: Improve config key descriptions
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/819
retest this please
---
[GitHub] brooklyn-server issue #819: Improve config key descriptions
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/819
a couple minor suggestions of wording
LGTM
as per @tbouron - icons v important (more important than config description IMHO!)
---
[GitHub] brooklyn-server pull request #819: Improve config key descriptions
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/819#discussion_r138584725
--- Diff: core/src/main/java/org/apache/brooklyn/core/entity/BrooklynConfigKeys.java ---
@@ -180,23 +194,59 @@
* component is up, but this entity does not care about the dependent component's actual config values.
*/
- public static final ConfigKey<Boolean> PROVISION_LATCH = newBooleanConfigKey("provision.latch", "Latch for blocking location provision until ready");
- public static final ConfigKey<Boolean> START_LATCH = newBooleanConfigKey("start.latch", "Latch for blocking start until ready");
+ public static final ConfigKey<Boolean> PROVISION_LATCH = ConfigKeys.builder(Boolean.class)
+ .name("provision.latch")
+ .description("Latch for blocking machine provisioning; if non-null will wait for this to resolve (normal use is with '$brooklyn:attributeWhenReady')")
+ .build();
+ public static final ConfigKey<Boolean> START_LATCH = ConfigKeys.builder(Boolean.class)
+ .name("start.latch")
+ .description("Latch for blocking start (done post-provisioning for software processes); if non-null will wait for this to resolve (normal use is with '$brooklyn:attributeWhenReady')")
+ .build();
@Beta // on stop DSLs time out after a minute and unblock; may be easier to fix after https://github.com/apache/brooklyn-server/pull/390
- public static final ConfigKey<Boolean> STOP_LATCH = newBooleanConfigKey("stop.latch", "Latch for blocking stop until a condition is met; will block for at most 1 minute and then time out");
+ public static final ConfigKey<Boolean> STOP_LATCH = ConfigKeys.builder(Boolean.class)
+ .name("stop.latch")
+ .description("Latch for blocking stop; if non-null will wait for at most 1 minute for this to resolve (normal use is with '$brooklyn:attributeWhenReady')")
+ .build();
- public static final ConfigKey<Boolean> SETUP_LATCH = newBooleanConfigKey("setup.latch", "Latch for blocking setup until ready");
- public static final ConfigKey<Boolean> PRE_INSTALL_RESOURCES_LATCH = newBooleanConfigKey("resources.preInstall.latch", "Latch for blocking pre-install resources until ready");
- public static final ConfigKey<Boolean> INSTALL_RESOURCES_LATCH = newBooleanConfigKey("resources.install.latch", "Latch for blocking install resources until ready");
- public static final ConfigKey<Boolean> INSTALL_LATCH = newBooleanConfigKey("install.latch", "Latch for blocking install until ready");
- public static final ConfigKey<Boolean> RUNTIME_RESOURCES_LATCH = newBooleanConfigKey("resources.runtime.latch", "Latch for blocking runtime resources until ready");
- public static final ConfigKey<Boolean> CUSTOMIZE_LATCH = newBooleanConfigKey("customize.latch", "Latch for blocking customize until ready");
- public static final ConfigKey<Boolean> CUSTOMIZE_RESOURCES_LATCH = newBooleanConfigKey("resources.customize.latch", "Latch for blocking customize resources until ready");
- public static final ConfigKey<Boolean> LAUNCH_LATCH = newBooleanConfigKey("launch.latch", "Latch for blocking launch until ready");
+ public static final ConfigKey<Boolean> SETUP_LATCH = ConfigKeys.builder(Boolean.class)
+ .name("setup.latch")
+ .description("Latch for blocking setup; if non-null will wait for this to resolve (normal use is with '$brooklyn:attributeWhenReady')")
+ .build();
+
+ public static final ConfigKey<Boolean> PRE_INSTALL_RESOURCES_LATCH = ConfigKeys.builder(Boolean.class)
+ .name("resources.preInstall.latch")
+ .description("Latch for blocking files being copied before the pre-install; if non-null will wait for this to resolve (normal use is with '$brooklyn:attributeWhenReady')")
+ .build();
+ public static final ConfigKey<Boolean> INSTALL_RESOURCES_LATCH = ConfigKeys.builder(Boolean.class)
+ .name("resources.install.latch")
+ .description("Latch for blocking files being copied before the install; if non-null will wait for this to resolve (normal use is with '$brooklyn:attributeWhenReady')")
+ .build();
+ public static final ConfigKey<Boolean> INSTALL_LATCH = ConfigKeys.builder(Boolean.class)
+ .name("install.latch")
+ .description("Latch for blocking install; if non-null will wait for this to resolve (normal use is with '$brooklyn:attributeWhenReady')")
+ .build();
+ public static final ConfigKey<Boolean> CUSTOMIZE_RESOURCES_LATCH = ConfigKeys.builder(Boolean.class)
+ .name("resources.customize.latch")
+ .description("Latch for blocking files being copied before customize; if non-null will wait for this to resolve (normal use is with '$brooklyn:attributeWhenReady')")
+ .build();
+ public static final ConfigKey<Boolean> CUSTOMIZE_LATCH = ConfigKeys.builder(Boolean.class)
+ .name("customize.latch")
+ .description("Latch for blocking customize; if non-null will wait for this to resolve (normal use is with '$brooklyn:attributeWhenReady')")
+ .build();
+ public static final ConfigKey<Boolean> RUNTIME_RESOURCES_LATCH = ConfigKeys.builder(Boolean.class)
+ .name("resources.runtime.latch")
+ .description("Latch for blocking files being copied before the launch; if non-null will wait for this to resolve (normal use is with '$brooklyn:attributeWhenReady')")
+ .build();
+ public static final ConfigKey<Boolean> LAUNCH_LATCH = ConfigKeys.builder(Boolean.class)
+ .name("launch.latch")
+ .description("Latch for blocking luanch; if non-null will wait for this to resolve (normal use is with '$brooklyn:attributeWhenReady')")
+ .build();
public static final ConfigKey<Duration> START_TIMEOUT = newConfigKey(
- "start.timeout", "Time to wait for process and for SERVICE_UP before failing (in seconds, default 2m)", Duration.seconds(120));
+ "start.timeout",
+ "Time to wait, after launching, for SERVICE_UP before failing (default to '2m')",
--- End diff --
Not sure the default needs to be written in the description. Would be hard to maintain and the CLI / UI have access to this info anyway.
---
[GitHub] brooklyn-server issue #819: Improve config key descriptions
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/819
Thanks @tbouron - comments addressed.
@drigodwin I believe you were also looking at this? Ready to merge?
---
[GitHub] brooklyn-server pull request #819: Improve config key descriptions
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/819#discussion_r138585654
--- Diff: core/src/main/java/org/apache/brooklyn/entity/group/DynamicFabric.java ---
@@ -50,13 +51,20 @@
+ "for each given location).",
true);
+ @CatalogConfig(label = "Member spec")
@SetFromFlag("memberSpec")
ConfigKey<EntitySpec<?>> MEMBER_SPEC = ConfigKeys.newConfigKey(
- new TypeToken<EntitySpec<?>>() {}, "dynamiccfabric.memberspec", "entity spec for creating new cluster members", null);
+ new TypeToken<EntitySpec<?>>() {},
+ "dynamiccfabric.memberspec",
+ "entity spec for creating new members (one per location)",
+ null);
@SetFromFlag("firstMemberSpec")
ConfigKey<EntitySpec<?>> FIRST_MEMBER_SPEC = ConfigKeys.newConfigKey(
- new TypeToken<EntitySpec<?>>() {}, "dynamiccfabric.firstmemberspec", "entity spec for creating new cluster members", null);
+ new TypeToken<EntitySpec<?>>() {},
+ "dynamiccfabric.firstmemberspec",
+ "entity spec for the first member",
--- End diff --
s/entity/Entity/
---