You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by GitBox <gi...@apache.org> on 2021/07/14 12:44:15 UTC
[GitHub] [brooklyn-server] tbouron opened a new pull request #1200: Add support setup default initializers for all deployment
tbouron opened a new pull request #1200:
URL: https://github.com/apache/brooklyn-server/pull/1200
This looks up a new configuration options called `brooklyn.deployment.initializers` (comma separated list). If specified on a Brooklyn instance, all deployments will load and execute these initializers. Theses classes are expected to be `EntityInitializer`, if an error occur (either cast or anything else) then the deployment will fail.
The code will try to:
1. load the class from the default class loader.
2. if (1) fails, it will try to load the class from the `TypeRegistry`. This is to allow execution of custom initializers that might be installed in the catalog later on.
3. if (1) and (2) fails, then the deployment is aborted.
This PR contains a new initializer that give more information about the deployment (who and when currently) but it is not set by default.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] ahgittin commented on a change in pull request #1200: Add support setup default initializers for all deployment
Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #1200:
URL: https://github.com/apache/brooklyn-server/pull/1200#discussion_r669660476
##########
File path: core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java
##########
@@ -465,12 +487,30 @@ public void run() {
}
return super.constructOldStyle(clazz, flags);
}
-
+
private <T extends Entity> Class<? extends T> getImplementedBy(EntitySpec<T> spec) {
if (spec.getImplementation() != null) {
return spec.getImplementation();
} else {
return entityTypeRegistry.getImplementedBy(spec.getType());
}
}
+
+ private List<EntityInitializer> getDefaultInitializers() {
+ return Arrays.stream(managementContext.getConfig().getConfig(DEFAULT_INITIALIZERS_CLASSNAMES).split(","))
+ .filter(Strings::isNonEmpty)
+ .map(className -> {
+ try {
Review comment:
to load a java class, use `JavaBrooklynClassLoadingContext.create(mgmt).tryLoadClass(className)` -- has slightly better classpath semantics
also i wonder whether we should try the type registry first?
(longer term i'd like to clean up and standardize better how we instantiate things!)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] ahgittin commented on pull request #1200: Add support setup default initializers for all deployment
Posted by GitBox <gi...@apache.org>.
ahgittin commented on pull request #1200:
URL: https://github.com/apache/brooklyn-server/pull/1200#issuecomment-880515489
looks great - merged
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] ahgittin commented on a change in pull request #1200: Add support setup default initializers for all deployment
Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #1200:
URL: https://github.com/apache/brooklyn-server/pull/1200#discussion_r669647198
##########
File path: core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java
##########
@@ -369,6 +385,12 @@ public void run() {
}
((AbstractEntity)entity).addLocations(spec.getLocations());
+ List<EntityInitializer> initializers = Stream.concat(spec.getInitializers().stream(), getDefaultInitializers().stream())
Review comment:
this will run `spec.getInitializers()` _before_ default / global initializers, whereas i think default / globals should run first
also we have the lines below 394-396 which run `spec.getInitializers()` _again_ after
suggest this should just be `getDefaultInitializers().forEach(i -> i.apply(entity))` -- no need to concat streams
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] tbouron commented on pull request #1200: Add support setup default initializers for all deployment
Posted by GitBox <gi...@apache.org>.
tbouron commented on pull request #1200:
URL: https://github.com/apache/brooklyn-server/pull/1200#issuecomment-879955713
@ahgittin That should be all your comments addressed now
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] ahgittin commented on a change in pull request #1200: Add support setup default initializers for all deployment
Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #1200:
URL: https://github.com/apache/brooklyn-server/pull/1200#discussion_r669662325
##########
File path: core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java
##########
@@ -369,6 +385,12 @@ public void run() {
}
((AbstractEntity)entity).addLocations(spec.getLocations());
+ List<EntityInitializer> initializers = Stream.concat(spec.getInitializers().stream(), getDefaultInitializers().stream())
Review comment:
no strong feelings about one loop or two, as long as globals done first, in case they want to fail fast to prevent even local initializers from running
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] asfgit closed pull request #1200: Add support setup default initializers for all deployment
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1200:
URL: https://github.com/apache/brooklyn-server/pull/1200
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] tbouron commented on a change in pull request #1200: Add support setup default initializers for all deployment
Posted by GitBox <gi...@apache.org>.
tbouron commented on a change in pull request #1200:
URL: https://github.com/apache/brooklyn-server/pull/1200#discussion_r669684607
##########
File path: core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java
##########
@@ -465,12 +487,30 @@ public void run() {
}
return super.constructOldStyle(clazz, flags);
}
-
+
private <T extends Entity> Class<? extends T> getImplementedBy(EntitySpec<T> spec) {
if (spec.getImplementation() != null) {
return spec.getImplementation();
} else {
return entityTypeRegistry.getImplementedBy(spec.getType());
}
}
+
+ private List<EntityInitializer> getDefaultInitializers() {
+ return Arrays.stream(managementContext.getConfig().getConfig(DEFAULT_INITIALIZERS_CLASSNAMES).split(","))
+ .filter(Strings::isNonEmpty)
+ .map(className -> {
+ try {
Review comment:
Done!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] ahgittin commented on a change in pull request #1200: Add support setup default initializers for all deployment
Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #1200:
URL: https://github.com/apache/brooklyn-server/pull/1200#discussion_r669661064
##########
File path: core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java
##########
@@ -46,46 +48,60 @@
import org.apache.brooklyn.util.core.flags.FlagUtils;
import org.apache.brooklyn.util.core.task.Tasks;
import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.guava.Maybe;
import org.apache.brooklyn.util.javalang.AggregateClassLoader;
import org.apache.brooklyn.util.javalang.Reflections;
import org.apache.brooklyn.util.text.Strings;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import javax.annotation.Nonnull;
import java.lang.reflect.InvocationTargetException;
+import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.Objects;
import java.util.Queue;
import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
/**
- * Creates entities (and proxies) of required types, given the
- *
+ * Creates entities (and proxies) of required types, given the
+ *
* This is an internal class for use by core-brooklyn. End-users are strongly discouraged from
* using this class directly.
- *
+ *
* Used in three situations:
* <ul>
* <li>Normal entity creation (through entityManager.createEntity)
* <li>rebind (i.e. Brooklyn restart, or promotion of HA standby manager node)
* <li>yaml parsing
* </ul>
- *
+ *
* @author aled
*/
public class InternalEntityFactory extends InternalFactory {
private static final Logger log = LoggerFactory.getLogger(InternalEntityFactory.class);
-
+
private final EntityTypeRegistry entityTypeRegistry;
private final InternalPolicyFactory policyFactory;
private final ClassLoaderCache classLoaderCache;
-
+
+ /**
+ * The initializers to be add to any application deployed by Brooklyn.
+ * e.g. <code>brooklyn.deployment.initializers=org.apache.brooklyn.core.effector.AddDeploySensorsInitializer</code>
+ * will automatically add tags to the root node of any deployed application.
+ */
+ public final static ConfigKey<String> DEFAULT_INITIALIZERS_CLASSNAMES = ConfigKeys.newStringConfigKey(
Review comment:
`brooklyn.deployment.initializers` is a great name - i should have said!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] ahgittin commented on a change in pull request #1200: Add support setup default initializers for all deployment
Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #1200:
URL: https://github.com/apache/brooklyn-server/pull/1200#discussion_r669649982
##########
File path: core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java
##########
@@ -46,46 +48,60 @@
import org.apache.brooklyn.util.core.flags.FlagUtils;
import org.apache.brooklyn.util.core.task.Tasks;
import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.guava.Maybe;
import org.apache.brooklyn.util.javalang.AggregateClassLoader;
import org.apache.brooklyn.util.javalang.Reflections;
import org.apache.brooklyn.util.text.Strings;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import javax.annotation.Nonnull;
import java.lang.reflect.InvocationTargetException;
+import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.Objects;
import java.util.Queue;
import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
/**
- * Creates entities (and proxies) of required types, given the
- *
+ * Creates entities (and proxies) of required types, given the
+ *
* This is an internal class for use by core-brooklyn. End-users are strongly discouraged from
* using this class directly.
- *
+ *
* Used in three situations:
* <ul>
* <li>Normal entity creation (through entityManager.createEntity)
* <li>rebind (i.e. Brooklyn restart, or promotion of HA standby manager node)
* <li>yaml parsing
* </ul>
- *
+ *
* @author aled
*/
public class InternalEntityFactory extends InternalFactory {
private static final Logger log = LoggerFactory.getLogger(InternalEntityFactory.class);
-
+
private final EntityTypeRegistry entityTypeRegistry;
private final InternalPolicyFactory policyFactory;
private final ClassLoaderCache classLoaderCache;
-
+
+ /**
+ * The initializers to be add to any application deployed by Brooklyn.
+ * e.g. <code>brooklyn.deployment.initializers=org.apache.brooklyn.core.effector.AddDeploySensorsInitializer</code>
+ * will automatically add tags to the root node of any deployed application.
+ */
+ public final static ConfigKey<String> DEFAULT_INITIALIZERS_CLASSNAMES = ConfigKeys.newStringConfigKey(
Review comment:
`DEFAULT` feels wrong -- should we call this `GLOBAL_DEPLOYMENT_INITIALIZERS` ?
(and change the method `getDefaultInitializers()`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] ahgittin commented on pull request #1200: Add support setup default initializers for all deployment
Posted by GitBox <gi...@apache.org>.
ahgittin commented on pull request #1200:
URL: https://github.com/apache/brooklyn-server/pull/1200#issuecomment-879936074
looks good - a few minor comments, then can merge
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] ahgittin commented on pull request #1200: Add support setup default initializers for all deployment
Posted by GitBox <gi...@apache.org>.
ahgittin commented on pull request #1200:
URL: https://github.com/apache/brooklyn-server/pull/1200#issuecomment-879941899
pleased to see our test coverage caught the problem w initializers being run twice!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] tbouron commented on a change in pull request #1200: Add support setup default initializers for all deployment
Posted by GitBox <gi...@apache.org>.
tbouron commented on a change in pull request #1200:
URL: https://github.com/apache/brooklyn-server/pull/1200#discussion_r669655451
##########
File path: core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java
##########
@@ -46,46 +48,60 @@
import org.apache.brooklyn.util.core.flags.FlagUtils;
import org.apache.brooklyn.util.core.task.Tasks;
import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.guava.Maybe;
import org.apache.brooklyn.util.javalang.AggregateClassLoader;
import org.apache.brooklyn.util.javalang.Reflections;
import org.apache.brooklyn.util.text.Strings;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import javax.annotation.Nonnull;
import java.lang.reflect.InvocationTargetException;
+import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.Objects;
import java.util.Queue;
import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
/**
- * Creates entities (and proxies) of required types, given the
- *
+ * Creates entities (and proxies) of required types, given the
+ *
* This is an internal class for use by core-brooklyn. End-users are strongly discouraged from
* using this class directly.
- *
+ *
* Used in three situations:
* <ul>
* <li>Normal entity creation (through entityManager.createEntity)
* <li>rebind (i.e. Brooklyn restart, or promotion of HA standby manager node)
* <li>yaml parsing
* </ul>
- *
+ *
* @author aled
*/
public class InternalEntityFactory extends InternalFactory {
private static final Logger log = LoggerFactory.getLogger(InternalEntityFactory.class);
-
+
private final EntityTypeRegistry entityTypeRegistry;
private final InternalPolicyFactory policyFactory;
private final ClassLoaderCache classLoaderCache;
-
+
+ /**
+ * The initializers to be add to any application deployed by Brooklyn.
+ * e.g. <code>brooklyn.deployment.initializers=org.apache.brooklyn.core.effector.AddDeploySensorsInitializer</code>
+ * will automatically add tags to the root node of any deployed application.
+ */
+ public final static ConfigKey<String> DEFAULT_INITIALIZERS_CLASSNAMES = ConfigKeys.newStringConfigKey(
Review comment:
@ahgittin Should I also change the property name or are you happy with what is there already?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] tbouron commented on a change in pull request #1200: Add support setup default initializers for all deployment
Posted by GitBox <gi...@apache.org>.
tbouron commented on a change in pull request #1200:
URL: https://github.com/apache/brooklyn-server/pull/1200#discussion_r670223945
##########
File path: core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java
##########
@@ -465,12 +487,30 @@ public void run() {
}
return super.constructOldStyle(clazz, flags);
}
-
+
private <T extends Entity> Class<? extends T> getImplementedBy(EntitySpec<T> spec) {
if (spec.getImplementation() != null) {
return spec.getImplementation();
} else {
return entityTypeRegistry.getImplementedBy(spec.getType());
}
}
+
+ private List<EntityInitializer> getDefaultInitializers() {
+ return Arrays.stream(managementContext.getConfig().getConfig(DEFAULT_INITIALIZERS_CLASSNAMES).split(","))
+ .filter(Strings::isNonEmpty)
+ .map(className -> {
+ try {
Review comment:
What I meant is that I used the canonical way to load the class and changed this order: the code now checks the `TypeRegistry` first, then tries to load from the class loader. Thank you BTW for the bit of code you shared above 👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] ahgittin commented on a change in pull request #1200: Add support setup default initializers for all deployment
Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #1200:
URL: https://github.com/apache/brooklyn-server/pull/1200#discussion_r669651471
##########
File path: core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java
##########
@@ -465,12 +487,30 @@ public void run() {
}
return super.constructOldStyle(clazz, flags);
}
-
+
private <T extends Entity> Class<? extends T> getImplementedBy(EntitySpec<T> spec) {
if (spec.getImplementation() != null) {
return spec.getImplementation();
} else {
return entityTypeRegistry.getImplementedBy(spec.getType());
}
}
+
+ private List<EntityInitializer> getDefaultInitializers() {
+ return Arrays.stream(managementContext.getConfig().getConfig(DEFAULT_INITIALIZERS_CLASSNAMES).split(","))
+ .filter(Strings::isNonEmpty)
+ .map(className -> {
+ try {
Review comment:
there is a canonical way we do this -- let me dig this up
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] tbouron commented on a change in pull request #1200: Add support setup default initializers for all deployment
Posted by GitBox <gi...@apache.org>.
tbouron commented on a change in pull request #1200:
URL: https://github.com/apache/brooklyn-server/pull/1200#discussion_r669652985
##########
File path: core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java
##########
@@ -369,6 +385,12 @@ public void run() {
}
((AbstractEntity)entity).addLocations(spec.getLocations());
+ List<EntityInitializer> initializers = Stream.concat(spec.getInitializers().stream(), getDefaultInitializers().stream())
Review comment:
Aaah I meant to remove line 394-396 and have only one loop to be DRY, hence the concatenation. I'll update @ahgittin
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org