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 2014/10/07 16:42:23 UTC
[GitHub] incubator-brooklyn pull request: Various: 20141007
GitHub user aledsage opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/224
Various: 20141007
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/aledsage/incubator-brooklyn various/20141007
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/224.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 #224
----
commit ab0c72b58441b9d3ed4379b1f43978c0562132de
Author: Aled Sage <al...@gmail.com>
Date: 2014-10-07T13:53:47Z
Fix LoadTest (missing .startManagement call)
commit c538aa4c03841a677180dcc8ba8275ae74aa3ad6
Author: Aled Sage <al...@gmail.com>
Date: 2014-10-07T13:56:25Z
SimulatedXyzImpl: add SIMULATE_ENTITY config
- For SimulatedJBoss7ServerImpl etc, add a SIMULATE_ENTITY config
option.
- If set, then don’t start the real entity (instead just run `sleep`
or skip the step entirely)
- Should we useful for setting to false, while also setting
simulateExternalMonitoring (in situations where there is enough
underlying resource to run many app-servers!)
commit e5878266d84ec5d03c32fbe180d4c2309cbf31e0
Author: Aled Sage <al...@gmail.com>
Date: 2014-10-07T13:57:57Z
YAML support for specifying entity impl
commit 2f7b5c31acc64468b6c538ad16b92caf1ba027ba
Author: Aled Sage <al...@gmail.com>
Date: 2014-10-07T14:13:47Z
Support for supplying machine_details 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: Various: 20141007
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/224#issuecomment-58287941
i don't like `impl`. neither the name nor the idea. do we really need it?
apart from that seems good. at some point we'll want to document the QA items but it is very much early days there so not that concerned.
---
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: Various: 20141007
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/224#issuecomment-58346218
I've added javadoc for the `Simulated*Impl` classes.
For `impl` in yaml, I didn't realise that `type: <concrete-class>` already worked! I've added a test for that.
The `impl` configuration option was motivated by how the `EntitySpec` can be used. But in that case, the generics get in the way for the return type (if you give a concrete class then you return type in the generics will not be correct because it will be a dynamic proxy).
I've left the `impl` and `implementation` support in as well though, so that one familiar with the `EntitySpec` can use the same thing in the yaml. @ahgittin @richardcloudsoft @grkvlt any opinions on 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: Various: 20141007
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/224#issuecomment-58221071
Agree with @richardcloudsoft the documentation for the QA and load testing is sparse. Otherwise seems good. No real preference over `impl` versus `implementation` as a key, so I propose accepting both.
---
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: Various: 20141007
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/224
---
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: Various: 20141007
Posted by richardcloudsoft <gi...@git.apache.org>.
Github user richardcloudsoft commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/224#issuecomment-58201930
I'm confused over the "simulation" changes. `SimulatedJBoss7Server` et al are classes that I wasn't previously aware of and which don't seem to be documented as to what or how they do their simulation. This change modifies the simulated entities to add configuration to enable or disable simulation. Presumably this is a *different* kind of simulation to what the entity was previously simulating, because otherwise surely the "simulate" config key could only ever be true :persevere:
Could you drop some Javadoc into these files to explain in more detail what they do, and in particular what the effect of changing the "simulate" config key is?
---
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: Various: 20141007
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/224#issuecomment-58440091
I've removed support for `impl: <concrete-type>` from yaml. We can always re-add it if there is a desire later. For future reference, it was in `BrooklynEntityMatcher` for extracting "impl" and "implementation", and in `BrooklynComponentTemplateResolver` for calling `entitySpec.impl(...)`.
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] incubator-brooklyn pull request: Various: 20141007
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/224#discussion_r18559189
--- Diff: core/src/main/java/brooklyn/location/basic/SshMachineLocation.java ---
@@ -130,6 +130,10 @@
public static final ConfigKey<Duration> SSH_CACHE_EXPIRY_DURATION = ConfigKeys.newConfigKey(Duration.class,
"sshCacheExpiryDuration", "Expiry time for unused cached ssh connections", Duration.FIVE_MINUTES);
+ public static final ConfigKey<MachineDetails> MACHINE_DETAILS = ConfigKeys.newConfigKey(
--- End diff --
neat idea. should help with testing. curious if there is another use case for this however?
---
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: Various: 20141007
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/224#discussion_r18559317
--- Diff: usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java ---
@@ -274,7 +274,17 @@ public boolean canResolve() {
planId = (String)attrs.getStringKey("id");
if (planId==null)
planId = (String) attrs.getStringKey(BrooklynCampConstants.PLAN_ID_FLAG);
-
+
+ String impl = (String)attrs.getStringKey("impl");
--- End diff --
why do we need this? can't we support specifying an implementation as the `type` and then detect whether it is a concrete class and construct the instance and proxy appropriately (taking all interfaces as the proxy)?
feels like it's painful even suggesting that someone coming from YAML should supply both `type` and `impl`.
---
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: Various: 20141007
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/224#discussion_r18573689
--- Diff: core/src/main/java/brooklyn/location/basic/SshMachineLocation.java ---
@@ -130,6 +130,10 @@
public static final ConfigKey<Duration> SSH_CACHE_EXPIRY_DURATION = ConfigKeys.newConfigKey(Duration.class,
"sshCacheExpiryDuration", "Expiry time for unused cached ssh connections", Duration.FIVE_MINUTES);
+ public static final ConfigKey<MachineDetails> MACHINE_DETAILS = ConfigKeys.newConfigKey(
--- End diff --
It was just tests - playing around with Couchbase download URL resolver on different distros. That turned out to be too painful to automate as a test (to get the actual full URL it would use - need to get access to the SshDriver, which only gets created part way through start, etc).
---
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: Various: 20141007
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/224#issuecomment-58364013
The `impl` part of EntitySpec is the ugliest part of it. Personally I'd really rather not have it at all in the YAML.
---
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.
---