You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by sjcorbett <gi...@git.apache.org> on 2015/07/07 11:34:05 UTC
[GitHub] incubator-brooklyn pull request: SoftwareProcess exposes Lifecycle...
GitHub user sjcorbett opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/735
SoftwareProcess exposes LifecycleEffectorTasks implementation as config
Thus it is much easier for subclasses of `SoftwareProcess` to customise which effectors they expose and how those effectors behave.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sjcorbett/incubator-brooklyn feature/lifecycle-configuration
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/735.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 #735
----
commit d0e49877a5eeeb88d4f1d99144f6bb5776b87e2e
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-07-07T09:09:02Z
SoftwareProcess exposes LifecycleEffectorTasks implementation as config
----
---
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] incubator-brooklyn pull request: SoftwareProcess exposes Lifecycle...
Posted by alasdairhodge <gi...@git.apache.org>.
Github user alasdairhodge commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/735#discussion_r34248297
--- Diff: software/webapp/src/test/java/brooklyn/test/entity/TestJavaWebAppEntityImpl.java ---
@@ -46,28 +46,10 @@ public TestJavaWebAppEntityImpl() {}
// constructor required for use in DynamicCluster.factory
public TestJavaWebAppEntityImpl(@SuppressWarnings("rawtypes") Map flags, Entity parent) { super(flags, parent); }
- private static final SoftwareProcessDriverLifecycleEffectorTasks LIFECYCLE_TASKS =
- new SoftwareProcessDriverLifecycleEffectorTasks() {
- public void start(java.util.Collection<? extends Location> locations) {
- ServiceStateLogic.setExpectedState(entity(), Lifecycle.STARTING);
- LOG.trace("Starting {}", this);
- entity().setAttribute(SERVICE_PROCESS_IS_RUNNING, true);
- entity().setAttribute(Attributes.SERVICE_UP, true);
- ServiceStateLogic.setExpectedState(entity(), Lifecycle.RUNNING);
- }
- public void stop() {
- ServiceStateLogic.setExpectedState(entity(), Lifecycle.STOPPING);
- LOG.trace("Stopping {}", this);
- entity().setAttribute(Attributes.SERVICE_UP, false);
- entity().setAttribute(SERVICE_PROCESS_IS_RUNNING, false);
- ServiceStateLogic.setExpectedState(entity(), Lifecycle.STOPPED);
- }
- };
-
@Override
public void init() {
super.init();
- LIFECYCLE_TASKS.attachLifecycleEffectors(this);
+ getLifecycleEffectorTasks().attachLifecycleEffectors(this);
--- End diff --
This is redundant, given `super.init()`
---
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] incubator-brooklyn pull request: SoftwareProcess exposes Lifecycle...
Posted by alasdairhodge <gi...@git.apache.org>.
Github user alasdairhodge commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/735#discussion_r34248099
--- Diff: software/webapp/src/test/java/brooklyn/test/entity/TestJavaWebAppEntityImpl.java ---
@@ -91,4 +73,25 @@ public int getB() {
public int getC() {
return c;
}
+
+ @Override
+ protected SoftwareProcessDriverLifecycleEffectorTasks getLifecycleEffectorTasks() {
+ return new SoftwareProcessDriverLifecycleEffectorTasks() {
+ public void start(java.util.Collection<? extends Location> locations) {
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.STARTING);
+ LOG.trace("Starting {}", this);
+ entity().setAttribute(SERVICE_PROCESS_IS_RUNNING, true);
+ entity().setAttribute(Attributes.SERVICE_UP, true);
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.RUNNING);
+ }
+
+ public void stop() {
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.STOPPING);
+ LOG.trace("Stopping {}", this);
+ entity().setAttribute(Attributes.SERVICE_UP, false);
+ entity().setAttribute(SERVICE_PROCESS_IS_RUNNING, false);
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.STOPPED);
+ }
+ };
+ }
--- End diff --
Even though it's just a "minor" class, it nonetheless serves as an example of how we recommend navigating this maze of lifecycle methods/effectors/overrides/fields/annotations; doubly so in the absence of decent documentation.
---
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] incubator-brooklyn pull request: SoftwareProcess exposes Lifecycle...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/735#discussion_r34338725
--- Diff: software/webapp/src/test/java/brooklyn/test/entity/TestJavaWebAppEntity.java ---
@@ -28,8 +39,38 @@
@ImplementedBy(TestJavaWebAppEntityImpl.class)
public interface TestJavaWebAppEntity extends VanillaJavaApp, WebAppService {
+ /**
+ * Injects the test entity's customised version of the class.
+ */
--- End diff --
Got an improvement?
---
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] incubator-brooklyn pull request: SoftwareProcess exposes Lifecycle...
Posted by alasdairhodge <gi...@git.apache.org>.
Github user alasdairhodge commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/735#discussion_r34296243
--- Diff: software/webapp/src/test/java/brooklyn/test/entity/TestJavaWebAppEntity.java ---
@@ -28,8 +39,38 @@
@ImplementedBy(TestJavaWebAppEntityImpl.class)
public interface TestJavaWebAppEntity extends VanillaJavaApp, WebAppService {
+ /**
+ * Injects the test entity's customised version of the class.
+ */
--- End diff --
Nitpick over phrasing: "Injects the test entity's customised lifecycle tasks."
---
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] incubator-brooklyn pull request: SoftwareProcess exposes Lifecycle...
Posted by alasdairhodge <gi...@git.apache.org>.
Github user alasdairhodge commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/735#issuecomment-120127497
Trivial nitpick, otherwise looks grand.
---
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] incubator-brooklyn pull request: SoftwareProcess exposes Lifecycle...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/735#discussion_r34247974
--- Diff: software/webapp/src/test/java/brooklyn/test/entity/TestJavaWebAppEntityImpl.java ---
@@ -91,4 +73,25 @@ public int getB() {
public int getC() {
return c;
}
+
+ @Override
+ protected SoftwareProcessDriverLifecycleEffectorTasks getLifecycleEffectorTasks() {
+ return new SoftwareProcessDriverLifecycleEffectorTasks() {
+ public void start(java.util.Collection<? extends Location> locations) {
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.STARTING);
+ LOG.trace("Starting {}", this);
+ entity().setAttribute(SERVICE_PROCESS_IS_RUNNING, true);
+ entity().setAttribute(Attributes.SERVICE_UP, true);
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.RUNNING);
+ }
+
+ public void stop() {
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.STOPPING);
+ LOG.trace("Stopping {}", this);
+ entity().setAttribute(Attributes.SERVICE_UP, false);
+ entity().setAttribute(SERVICE_PROCESS_IS_RUNNING, false);
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.STOPPED);
+ }
+ };
+ }
--- End diff --
Or just alter `TestJavaWebAppEntity` to set the default to this subclass?
---
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] incubator-brooklyn pull request: SoftwareProcess exposes Lifecycle...
Posted by alasdairhodge <gi...@git.apache.org>.
Github user alasdairhodge commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/735#discussion_r34339220
--- Diff: software/webapp/src/test/java/brooklyn/test/entity/TestJavaWebAppEntity.java ---
@@ -28,8 +39,38 @@
@ImplementedBy(TestJavaWebAppEntityImpl.class)
public interface TestJavaWebAppEntity extends VanillaJavaApp, WebAppService {
+ /**
+ * Injects the test entity's customised version of the class.
+ */
--- End diff --
Ha! My suggested improvement is right there in quotes ;o)
---
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] incubator-brooklyn pull request: SoftwareProcess exposes Lifecycle...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/735#discussion_r34339325
--- Diff: software/webapp/src/test/java/brooklyn/test/entity/TestJavaWebAppEntity.java ---
@@ -28,8 +39,38 @@
@ImplementedBy(TestJavaWebAppEntityImpl.class)
public interface TestJavaWebAppEntity extends VanillaJavaApp, WebAppService {
+ /**
+ * Injects the test entity's customised version of the class.
+ */
--- End diff --
Err, no explanation for how I missed that.
---
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] incubator-brooklyn pull request: SoftwareProcess exposes Lifecycle...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/735#discussion_r34247862
--- Diff: software/webapp/src/test/java/brooklyn/test/entity/TestJavaWebAppEntityImpl.java ---
@@ -91,4 +73,25 @@ public int getB() {
public int getC() {
return c;
}
+
+ @Override
+ protected SoftwareProcessDriverLifecycleEffectorTasks getLifecycleEffectorTasks() {
+ return new SoftwareProcessDriverLifecycleEffectorTasks() {
+ public void start(java.util.Collection<? extends Location> locations) {
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.STARTING);
+ LOG.trace("Starting {}", this);
+ entity().setAttribute(SERVICE_PROCESS_IS_RUNNING, true);
+ entity().setAttribute(Attributes.SERVICE_UP, true);
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.RUNNING);
+ }
+
+ public void stop() {
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.STOPPING);
+ LOG.trace("Stopping {}", this);
+ entity().setAttribute(Attributes.SERVICE_UP, false);
+ entity().setAttribute(SERVICE_PROCESS_IS_RUNNING, false);
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.STOPPED);
+ }
+ };
+ }
--- End diff --
I thought about it but figured it was a not-too-widely used test class. Happy to update if you think fit.
---
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] incubator-brooklyn pull request: SoftwareProcess exposes Lifecycle...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/735#issuecomment-123349384
Closing since merged.
---
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] incubator-brooklyn pull request: SoftwareProcess exposes Lifecycle...
Posted by alasdairhodge <gi...@git.apache.org>.
Github user alasdairhodge commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/735#discussion_r34247895
--- Diff: software/webapp/src/test/java/brooklyn/test/entity/TestJavaWebAppEntityImpl.java ---
@@ -91,4 +73,25 @@ public int getB() {
public int getC() {
return c;
}
+
+ @Override
+ protected SoftwareProcessDriverLifecycleEffectorTasks getLifecycleEffectorTasks() {
+ return new SoftwareProcessDriverLifecycleEffectorTasks() {
+ public void start(java.util.Collection<? extends Location> locations) {
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.STARTING);
+ LOG.trace("Starting {}", this);
+ entity().setAttribute(SERVICE_PROCESS_IS_RUNNING, true);
+ entity().setAttribute(Attributes.SERVICE_UP, true);
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.RUNNING);
+ }
+
+ public void stop() {
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.STOPPING);
+ LOG.trace("Stopping {}", this);
+ entity().setAttribute(Attributes.SERVICE_UP, false);
+ entity().setAttribute(SERVICE_PROCESS_IS_RUNNING, false);
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.STOPPED);
+ }
+ };
+ }
--- End diff --
Workaround: check *raw* config in `init()`, manually call `setConfig(LIFECYCLE_EFFECTOR_TASKS, getLifecycleEffectorTasks())` if unset. Pretty ugly.
---
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] incubator-brooklyn pull request: SoftwareProcess exposes Lifecycle...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/735#issuecomment-119999500
@alasdairhodge Thanks for comments. Would you like to double check?
---
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] incubator-brooklyn pull request: SoftwareProcess exposes Lifecycle...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/735#discussion_r34247911
--- Diff: software/webapp/src/test/java/brooklyn/test/entity/TestJavaWebAppEntityImpl.java ---
@@ -91,4 +73,25 @@ public int getB() {
public int getC() {
return c;
}
+
+ @Override
+ protected SoftwareProcessDriverLifecycleEffectorTasks getLifecycleEffectorTasks() {
+ return new SoftwareProcessDriverLifecycleEffectorTasks() {
+ public void start(java.util.Collection<? extends Location> locations) {
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.STARTING);
+ LOG.trace("Starting {}", this);
+ entity().setAttribute(SERVICE_PROCESS_IS_RUNNING, true);
+ entity().setAttribute(Attributes.SERVICE_UP, true);
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.RUNNING);
+ }
+
+ public void stop() {
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.STOPPING);
+ LOG.trace("Stopping {}", this);
+ entity().setAttribute(Attributes.SERVICE_UP, false);
+ entity().setAttribute(SERVICE_PROCESS_IS_RUNNING, false);
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.STOPPED);
+ }
+ };
+ }
--- End diff --
Actually in truth I did this in two steps - the first was to make getLifecycleEffectorTasks `protected` and the change above, then I added the config option without updating this case.
---
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] incubator-brooklyn pull request: SoftwareProcess exposes Lifecycle...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/735
---
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] incubator-brooklyn pull request: SoftwareProcess exposes Lifecycle...
Posted by alasdairhodge <gi...@git.apache.org>.
Github user alasdairhodge commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/735#discussion_r34247759
--- Diff: software/webapp/src/test/java/brooklyn/test/entity/TestJavaWebAppEntityImpl.java ---
@@ -91,4 +73,25 @@ public int getB() {
public int getC() {
return c;
}
+
+ @Override
+ protected SoftwareProcessDriverLifecycleEffectorTasks getLifecycleEffectorTasks() {
+ return new SoftwareProcessDriverLifecycleEffectorTasks() {
+ public void start(java.util.Collection<? extends Location> locations) {
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.STARTING);
+ LOG.trace("Starting {}", this);
+ entity().setAttribute(SERVICE_PROCESS_IS_RUNNING, true);
+ entity().setAttribute(Attributes.SERVICE_UP, true);
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.RUNNING);
+ }
+
+ public void stop() {
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.STOPPING);
+ LOG.trace("Stopping {}", this);
+ entity().setAttribute(Attributes.SERVICE_UP, false);
+ entity().setAttribute(SERVICE_PROCESS_IS_RUNNING, false);
+ ServiceStateLogic.setExpectedState(entity(), Lifecycle.STOPPED);
+ }
+ };
+ }
--- End diff --
This override ignores any confgured lifecycle-tasks, which could be surprising-in-a-bad-way. I guess we'd really like to override the config key's *default* (which isn't possible).
---
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.
---