[GitHub] brooklyn-ui pull request #112: DSL editor: support referencing `brooklyn.par...
GitHub user aledsage opened a pull request:
https://github.com/apache/brooklyn-ui/pull/112
DSL editor: support referencing `brooklyn.parameters`
Allows a blueprint to use `brooklyn.parameters`, and to reference that
parameter using the DSL editor.
For example, I wrote the blueprint below in YAML (except the values of foo
and bar), then switched to graphical view. I used the DSL editor to set the
values for foo and bar to reference the brooklyn.parameters at the top-level
app and on another entity.
```
brooklyn.config:
toplevelkey: toplevelval
brooklyn.parameters:
- name: toplevelParam
services:
- type: server
id: server1
brooklyn.parameters:
- name: MyParameter
type: string
- type: server
id: server2
brooklyn.config:
bar: '$brooklyn:component("server1").config("MyParameter")'
foo: '$brooklyn:parent().config("toplevelParam")'
```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/aledsage/brooklyn-ui
dsl-editor-reference-brooklyn-parameter
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-ui/pull/112.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 #112
commit dc8ee4e019ae581db0fe72a4d09890db566c3045
Author: Aled Sage
Date: 2018-11-21T16:55:24Z
DSL editor: support referencing `brooklyn.parameters`
---
[GitHub] brooklyn-ui issue #111: fix json editor state issues
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-ui/pull/111 LGTM; merging. ---
[GitHub] brooklyn-ui issue #109: catalog saver to support 'application', 'template', ...
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-ui/pull/109 Sure, happy to merge and we can make subsequent improvements in other PRs. Merging now. ---
[GitHub] brooklyn-server issue #1016: REST API includes icon url source
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/1016 LGTM - merging. ---
[GitHub] brooklyn-ui issue #109: catalog saver to support 'application', 'template', ...
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-ui/pull/109 @ahgittin for 'template', is that really what it means in the product from a user's perspective? Something declared as a 'template' will appear in the quick launch, whereas other things will not. When you click on it in the 'quick launch', then a dialog pops up with two buttons: `deploy` and `open in editor`. The latter opens a simple text editor in the quick-launch dialog, rather than re-directing to the blueprint composer. Your description of 'template' is accurate for when this was originally added to the code base, but I don't think that is what it does now, or how it is used now. That description won't make sense to a user, based on how the Brooklyn UI behaves. ---
[GitHub] brooklyn-server pull request #1014: [WIP] Subtype setting config val: use as...
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn-server/pull/1014 [WIP] Subtype setting config val: use as keyâs default val **For review/feedback only - failing unit tests.** Otherwise if another blueprint uses this subtype, it does not see the config val for that key. Worst case, it says the blueprint is invalid because the config key has no value. --- My new unit tests pass, but it breaks existing unit tests (e.g. in `ConfigParametersYamlTest.java`). The logic is applied to a catalog item and also to a blueprint being deployed. Our assertions in existing tests check that the config keys of the app being deployed have particular default vals, but these are now being overridden by the blueprint. What do folk think (cc @ahgittin) - should we use this approach, and fix the tests, or do something else? You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn-server subtype-config-used-as-key-defaultval Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn-server/pull/1014.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 #1014 commit 80d4806bc2828bb1437a42e48320e59282c5e86d Author: Aled Sage Date: 2018-11-15T09:57:21Z Subtype setting config val: use as keyâs default val Otherwise if another blueprint uses this subtype, it does not see the config val for that key. Worst case, it says the blueprint is invalid because the config key has no value. ---
[GitHub] brooklyn-ui issue #97: Generalise usage of $templateCache for blueprint comp...
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-ui/pull/97 Description sounds sensible to me, and scanning through the code I didn't see anything obviously wrong! I've tested it in `npm run start` mode, and all the relevant blueprint-composer features seem to still work, so I think the restructuring/wiring has been done correctly. Happy for this to be merged, but if anyone else is reviewing the code more thoroughly then even happier to wait for them! ---
[GitHub] brooklyn-server pull request #1011: Fix requiredUnless config key constraint...
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn-server/pull/1011 Fix requiredUnless config key constraint, with attributeWhenReady Previously this caused the entityâs startup to hang. You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn-server fix-requiredUnless-with-attributeWhenReady Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn-server/pull/1011.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 #1011 commit 2ff53c7a2d3551dfca69aaca296b7872da97f8cf Author: Aled Sage Date: 2018-10-24T15:05:52Z Fix requiredUnless config key constraint, with attributeWhenReady Previously this caused the entityâs startup to hang. ---
[GitHub] brooklyn-ui issue #90: DSL editor: allow ref to entity anywhere
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-ui/pull/90 Ah, you beat me to it @tbouron (and I should have refreshed my browser page to see your comment!). The build worked after you requested the retest. My retest was unnecessary. ---
[GitHub] brooklyn-ui issue #90: DSL editor: allow ref to entity anywhere
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-ui/pull/90 retest this please Failed with seemingly unrelated failure in a different module: ``` [ERROR] Failed to execute goal com.github.eirslett:frontend-maven-plugin:1.3:npm (npm install) on project brooklyn-ui-location-manager: Failed to run task: 'npm install' failed. java.lang.InterruptedException -> [Help 1] ``` ---
[GitHub] brooklyn-ui pull request #90: DSL editor: allow ref to entity anywhere
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn-ui/pull/90 DSL editor: allow ref to entity anywhere This change means that the DSL editor will let you choose a reference to an entity in any config key (rather than only if the config key is of type `Entity`). This makes it more consistent with the rest of the DSL editor: e.g. if generating `attributeWhenReady`, it doesn't type-check the attributes' types to filter the view to only those that match the config key's type. This is a good thing, because otherwise we'd have to repeat all the type coercion rules in the UI, for it to know what is legal! --- I tested this by running `npm run start` in brooklyn-ui/ui-modules/blueprint-composer. I chose a couple of arbitrary entities, and successfully wired up a config key's value to reference the other entity: https://user-images.githubusercontent.com/593113/47419941-30857c00-d775-11e8-9509-2bc87a8fb146.png;> You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn-ui dsl-editor-allow-entity-ref Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn-ui/pull/90.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 #90 commit e8762af21c1567493d491a2e4a5ff26384026941 Author: Aled Sage Date: 2018-10-24T09:09:18Z DSL editor: allow ref to entity anywhere ---
[GitHub] brooklyn-ui issue #74: Support for karaf 4.2.1 and running under JDK 11
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-ui/pull/74 retest this please ---
[GitHub] brooklyn-ui issue #79: Implement missing endpoint for palette API
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-ui/pull/79 LGTM; I grep'ed for uses of `getTypeVersion` to check if anything was using the old name - all looks good. Merging. ---
[GitHub] brooklyn-server issue #1002: Bump karaf to 4.2.1 to allow running under JDK ...
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/1002 Thanks @kemitix - looks good, merging now. ---
[GitHub] brooklyn-server issue #1003: Update IPTables save method
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/1003 LGTM; happy for this to be merged (once confusion with the identical-looking https://github.com/apache/brooklyn-server/pull/1005 is cleared up). ---
[GitHub] brooklyn-server issue #1006: Update IPTables save method
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/1006 This looks identical to https://github.com/apache/brooklyn-server/pull/1003 - is it? Can we close your PR @grkvlt and merge @frogfather 's original? ---
[GitHub] brooklyn-server issue #1005: [TEST] Update README.md
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/1005 @grkvlt did your test work? Are you happy to close this? Ping me if you want to discuss PR builds - I've been working on our build configuration in jenkins. ---
[GitHub] brooklyn-server issue #1008: Adds constraint (required|forbidden)unlessAnyOf
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/1008 @kemitix I completely agree about the tests being cryptic. Passing four booleans in a row in a method signature is an anti-pattern: it is hard to read and very error-prone. These tests were originally written by Alex - I did some minor improvements to their readability (see https://github.com/apache/brooklyn-server/commit/d19d2079b1a2b16c6de8c950b67a2daf0b8d3372). I decided it was hard to make it much more readable without significantly increasing the amount of code required. On balance, because it is a test class and because the methods are private, it seemed ok to leave the methods as cryptic - unpleasant for code reviewers looking at a diff, but if you open up the class it's liveable with. ---
[GitHub] brooklyn-server issue #1008: Adds constraint (required|forbidden)unlessAnyOf
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/1008 retest this please Changed test config, to enable `Delete workspace before build starts` ---
[GitHub] brooklyn-server pull request #1008: Adds constraint (required|forbidden)unle...
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn-server/pull/1008 Adds constraint (required|forbidden)unlessAnyOf You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn-server constraint-forbiddenUnlessAnyOf Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn-server/pull/1008.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 #1008 commit 554b9f7e560269b797220ba231ddf9f4f139f4f6 Author: Aled Sage Date: 2018-10-08T22:15:16Z Adds constraint (required|forbidden)unlessAnyOf ---
[GitHub] brooklyn-ui issue #76: Composer palette: search entity tags
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-ui/pull/76 retest this please ---
[GitHub] brooklyn-server issue #1004: BROOKLYN-602: fix config key order for yaml ove...
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/1004 Thanks @geomacy - variable renamed; merging. ---
[GitHub] brooklyn-server pull request #1004: BROOKLYN-602: fix config key order for y...
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn-server/pull/1004 BROOKLYN-602: fix config key order for yaml overrides You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn-server fix-BROOKYLN-602 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn-server/pull/1004.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 #1004 commit 1a410be58602d063161fdf746c7aa048b6da9be5 Author: Aled Sage Date: 2018-10-01T14:49:20Z BROOKLYN-602: fix config key order for yaml overrides ---
[GitHub] brooklyn pull request #23: jenkins: fix npm build, removing docker -u
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn/pull/23 jenkins: fix npm build, removing docker -u npm writes to ~/.npm, and reads ~/.npmrc. But user 910 does not exist in container, so has no home directory. Therefore npm fails. Reverting `-u` for now. e.g. https://builds.apache.org/view/B/view/Brooklyn/job/brooklyn-master-build-docker-pipeline/10/console failed with: ``` [INFO] --- frontend-maven-plugin:1.3:npm (npm install) @ brooklyn-ui-utils --- [INFO] Running 'npm install' in /home/jenkins/jenkins-slave/workspace/brooklyn-master-build-docker-pipeline/brooklyn-ui/ui-modules/utils [ERROR] Unhandled rejection Error: EACCES: permission denied, mkdir '/.npm' ``` This stackoverflow looks useful for fixing it longer term: https://stackoverflow.com/questions/46129443/sudo-permission-inside-docker-container-for-jenkins-pipeline. We could `npm config set cache /tmp` and also `export HOME=...`. Or we could create a user with id 910:910 in the container, as part of the Dockerfile. You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn jenkins-pipeline-3 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn/pull/23.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 #23 commit 18d07ee82b10c5fc9415accd200701e1e4ca8642 Author: Aled Sage Date: 2018-10-01T08:14:58Z jenkins: fix npm build, removing docker -u npm writes to ~/.npm, and reads ~/.npmrc. But user 910 does not exist in container, so has no home directory. Therefore npm fails. Reverting `-u` for now. ---
[GitHub] brooklyn issue #19: Update Docker image to use Maven 3.5.4
Github user aledsage commented on the issue: https://github.com/apache/brooklyn/pull/19 LGTM; thanks @slachiewicz - merging. ---
[GitHub] brooklyn-server issue #1002: Bump karaf to 4.2.1 to allow running under JDK ...
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/1002 The PR build's test failures look related to the change, e.g.: ``` 2018-09-28 10:56:56,059 INFO - TESTNG FAILED CONFIGURATION: "Surefire test" - @BeforeClass org.apache.brooklyn.rest.testing.BrooklynRestApiTest.setUpClass() finished in 0 ms java.lang.NoClassDefFoundError: org/eclipse/jetty/server/SessionManager at org.apache.cxf.transport.http_jetty.JettyHTTPServerEngineFactory.getOrCreate(JettyHTTPServerEngineFactory.java:117) at org.apache.cxf.transport.http_jetty.JettyHTTPServerEngineFactory.createJettyHTTPServerEngine(JettyHTTPServerEngineFactory.java:273) at org.apache.cxf.transport.http_jetty.JettyHTTPDestination.retrieveEngine(JettyHTTPDestination.java:122) at org.apache.cxf.transport.http_jetty.JettyHTTPDestination.finalizeConfig(JettyHTTPDestination.java:154) at org.apache.cxf.transport.http.HTTPTransportFactory.getDestination(HTTPTransportFactory.java:281) at org.apache.cxf.endpoint.ServerImpl.initDestination(ServerImpl.java:84) at org.apache.cxf.endpoint.ServerImpl.(ServerImpl.java:63) at org.apache.cxf.jaxrs.JAXRSServerFactoryBean.create(JAXRSServerFactoryBean.java:173) at org.apache.brooklyn.rest.testing.BrooklynRestResourceTest.startServer(BrooklynRestResourceTest.java:100) at org.apache.brooklyn.rest.testing.BrooklynRestResourceTest.initClass(BrooklynRestResourceTest.java:80) at org.apache.brooklyn.rest.testing.BrooklynRestApiTest.setUpClass(BrooklynRestApiTest.java:66) Caused by: java.lang.ClassNotFoundException: org.eclipse.jetty.server.SessionManager at java.net.URLClassLoader.findClass(URLClassLoader.java:381) at java.lang.ClassLoader.loadClass(ClassLoader.java:424) at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:349) at java.lang.ClassLoader.loadClass(ClassLoader.java:357) ... 40 more ``` ---
[GitHub] brooklyn-client issue #70: fix jenkins docker build
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-client/pull/70 Thanks @frogfather - LGTM, and jenkins PR build worked. Merging now. ---
[GitHub] brooklyn-ui pull request #76: Composer palette: search entity tags
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn-ui/pull/76 Composer palette: search entity tags The palette should also search the entity's tags, when filtering. People can use tags to categorise or provide additional metadata, so this is useful to include. You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn-ui palette-search-tags Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn-ui/pull/76.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 #76 commit 0b098749f6ce1d1f055b4667c1bae1400b5edd54 Author: Aled Sage Date: 2018-09-28T12:25:19Z Composer palette: search entity tags ---
[GitHub] brooklyn pull request #22: Jenkins pipeline: fix `user.name`
GitHub user aledsage opened a pull request:
https://github.com/apache/brooklyn/pull/22
Jenkins pipeline: fix `user.name`
User 910 is unknown in the docker container, so `${id -un 910}` does not
return a name. We donât care what `user.name` it has, as long as there is one.
Otherwise tests in
`org.apache.brooklyn.location.jclouds.DefaultConnectivityResolverTest` fail
because it didn't find a user name, so goes down a different code path to try
to figure it out and set it.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/aledsage/brooklyn jenkins-pipeline-2
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn/pull/22.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 #22
commit c2a3f37518278c8a03c738f8ef1d4378fe628d3b
Author: Aled Sage
Date: 2018-09-27T09:40:47Z
jenkins: fix user.name in docker
User 910 is unknown in the docker container, so
`${id -un 910}` does not return a name. We donât
care what user.name it has, as long as there is one.
commit 5122c6d558465a6254c81fa7c1bcc6b959f6710d
Author: Aled Sage
Date: 2018-09-27T09:41:07Z
README: fix path to built karaf artifact
---
[GitHub] brooklyn-ui pull request #74: Support for karaf 4.2.1 and running under JDK ...
Github user aledsage commented on a diff in the pull request: https://github.com/apache/brooklyn-ui/pull/74#discussion_r220589038 --- Diff: pom.xml --- @@ -99,7 +99,8 @@ 3.0.1 3.0.0 1.3 - 6.0.6 +6.0.6 --- End diff -- Do we need to redeclare this here? Or can we just inherit it from brooklyn-server's pom.xml (which it should get from its parent). ---
[GitHub] brooklyn-server issue #1002: Bump karaf to 4.2.1 to allow running under JDK ...
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/1002 Eye-balling the changes, they look good. A few things to raise: 1. Would be good to upgrade our jetty.version to 9.4.x (from 9.3.24) to match karaf's version. But @kemitix has tested with us including both bundle versions in karaf and it's working. So we can do that separately. 2. A little uncomfortable about having two `pax-web` versions - what needs 6.0.6 explicitly? What version is in karaf 4.2.1? 3. The karaf.plugin.version in 4.1.5 onwards does not respect maven's `-s /path/to/settings.xml` (as described in http://karaf.922171.n3.nabble.com/4-1-5-and-Karaf-Maven-Plugin-td4052587.html - I've been meaning to create a formal bug report for that!). This shouldn't affect Brooklyn because Apache Jenkins uses the standard location `~/.m2/settings.xml`, but might impact downstream projects. I'd like to have some time to build this locally and do some more testing before we merge it please. ---
[GitHub] brooklyn issue #21: Jenkins: use pipeline, and don’t bind-mount .m2
Github user aledsage commented on the issue: https://github.com/apache/brooklyn/pull/21 Merging now, to test build. ---
[GitHub] brooklyn pull request #21: Jenkins: use pipeline, and don’t bind-mount .m2
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn/pull/21 Jenkins: use pipeline, and donât bind-mount .m2 Copies the configuration approach in https://github.com/apache/brooklyn-library/blob/master/Jenkinsfile, to be used by https://builds.apache.org/view/B/view/Brooklyn/job/brooklyn-master-build-docker-pipeline/ You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn jenkins-pipeline Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn/pull/21.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 #21 commit d1280c84bb88ccb9b99ccaf3faa0ce0583b36a61 Author: Aled Sage Date: 2018-09-25T14:21:49Z Jenkins: use pipeline, and donât bind-mount .m2 ---
[GitHub] brooklyn-library pull request #162: DO NOT MERGE - for testing PR builds
Github user aledsage closed the pull request at: https://github.com/apache/brooklyn-library/pull/162 ---
[GitHub] brooklyn-library issue #162: DO NOT MERGE - for testing PR builds
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-library/pull/162 retest this please ---
[GitHub] brooklyn-library issue #162: DO NOT MERGE - for testing PR builds
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-library/pull/162 retest this please ---
[GitHub] brooklyn-library pull request #162: DO NOT MERGE - for testing PR builds
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn-library/pull/162 DO NOT MERGE - for testing PR builds You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn-library test-pr Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn-library/pull/162.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 #162 commit 0012d2324d46ebcaeb5f48d0d4fe3de696bd Author: Aled Sage Date: 2018-09-25T11:50:35Z DO NOT MERGE - for testing PR builds ---
[GitHub] brooklyn-library issue #154: Use the settings.xml from Jenkins to deploy art...
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-library/pull/154 retest this please (just using this to test https://builds.apache.org/view/B/view/Brooklyn/job/brooklyn-library-pull-requests-pipeline/) ---
[GitHub] brooklyn-library pull request #161: jenkins: add settings.xml for 'mvn deplo...
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn-library/pull/161 jenkins: add settings.xml for 'mvn deploy' You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn-library fix-jenkins-build-pipeline Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn-library/pull/161.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 #161 commit 35612262cf8c782d2152049d8b865cdd094c03dd Author: Aled Sage Date: 2018-09-25T11:26:29Z jenkins: add settings.xml for 'mvn deploy' ---
[GitHub] brooklyn-library issue #160: jenkins: don't bind mount .m2
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-library/pull/160 I've created https://builds.apache.org/view/B/view/Brooklyn/job/brooklyn-library-master-docker-pipeline/. The first run failed due to the rat check, which this PR fixes. Merging this now, and will re-run the pipeline build. ---
[GitHub] brooklyn-library issue #160: jenkins: don't bind mount .m2
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-library/pull/160 @tbouron I suspect that the `Jenkinsfile` has no effect, and that it just uses the configuration in https://builds.apache.org/view/B/view/Brooklyn/job/brooklyn-library-master-docker/configure. e.g. the most recent build (https://builds.apache.org/view/B/view/Brooklyn/job/brooklyn-library-master-docker/336/consoleFull) has a docker command that matches the `/configure` page rather than the Jenkinsfile. Am I missing something? I'm going to create a new job (`brooklyn-library-master-docker-pipeline`) to try to set it up to use the Jenkinsfile (i.e. use pipelines). ---
[GitHub] brooklyn-server issue #1000: Config constraints: add more tests
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/1000 retest this please Jenkins failure was an environment problem I believe: ``` Caused by: hudson.plugins.git.GitException: Command "git fetch --tags --progress git://github.com/apache/brooklyn-server.git +refs/pull/*:refs/remotes/origin/pr/*" returned status code 128: ``` ---
[GitHub] brooklyn-ui pull request #72: More constraints
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-ui/pull/72#discussion_r219920469
--- Diff:
ui-modules/blueprint-composer/app/components/providers/blueprint-service.provider.js
---
@@ -275,20 +275,54 @@ function BlueprintService($log, $q, $sce, paletteApi,
iconGenerator, dslService)
return $q((resolve) => {
if (entity.miscData.has('config')) {
entity.miscData.get('config')
-.filter(config => config.constraints &&
Object.keys(config.constraints).length > 0)
+.filter(config => config.constraints &&
config.constraints.length > 0)
.forEach(config => {
-for (let [key, constraint] of
Object.entries(config.constraints) ) {
+for (let constraintO of config.constraints) {
let message = null;
+let key = null, args = null;
+if (constraintO instanceof String) {
+key = constraintO;
+} else if (Object.keys(constraintO).length==1)
{
+key = Object.keys(constraintO)[0];
+args = constraintO[key];
+} else {
+$log.warn("Unknown constraint object",
constraintO);
+key = constraintO;
+}
+let val = (k) => entity.config.get(k ||
config.name);
+let isSet = (k) => entity.config.has(k ||
config.name) && angular.isDefined(val(k));
+let hasDefault = () =>
angular.isDefined(config.defaultValue);
switch (key) {
case 'Predicates.notNull()':
case 'required':
-if
(!angular.isDefined(config.defaultValue) && (!entity.config.has(config.name) ||
!angular.isDefined(entity.config.get(config.name {
+if (!isSet() && !hasDefault() &&
val()!='') {
+// "required" also means that it
must not be the empty string
message =
`${config.name} is required`;
}
break;
case 'regex':
-if (!entity.config.has(config.name) ||
!angular.isDefined(entity.config.get(config.name)) || !(new
RegExp(constraint).test(entity.config.get(config.name {
-message =
`${config.name} does not match the required format:
${config.constraints.regex}`;
+if (isSet() && !(new
RegExp(args).test(val))) {
--- End diff --
Should the regex constraint respect default values? e.g. if a `username`
config key has a default of `brooklyn` and a constraint of `regex: [a-z]+`,
then I think we should not force the user to override the default. I can
imagine other defaults, like a default cidr with a regex enforcing that a
user-supplied cidr is in the right format.
---
[GitHub] brooklyn-server pull request #1000: Config constraints: add more tests
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn-server/pull/1000 Config constraints: add more tests You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn-server config-constraints-tests Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn-server/pull/1000.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 #1000 commit d19d2079b1a2b16c6de8c950b67a2daf0b8d3372 Author: Aled Sage Date: 2018-09-24T17:14:45Z Config constraints: add more tests ---
[GitHub] brooklyn-library issue #154: Use the settings.xml from Jenkins to deploy art...
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-library/pull/154 @tbouron I think you're right that we don't need this PR - for example, https://builds.apache.org/job/brooklyn-library-master-docker/335 passed and its console output included: ``` Uploading to apache.snapshots.https: https://repository.apache.org/content/repositories/snapshots/org/apache/brooklyn/brooklyn-library-catalog/1.0.0-SNAPSHOT/brooklyn-library-catalog-1.0.0-20180924.134212-335.jar Progress (1): 2.0/8.0 kB Progress (1): 4.1/8.0 kB Progress (1): 6.1/8.0 kB Progress (1): 8.0 kB Uploaded to apache.snapshots.https: https://repository.apache.org/content/repositories/snapshots/org/apache/brooklyn/brooklyn-library-catalog/1.0.0-SNAPSHOT/brooklyn-library-catalog-1.0.0-20180924.134212-335.jar (8.0 kB at 9.4 kB/s) ``` However, I'm not sure what will happen after we merge https://github.com/apache/brooklyn-library/pull/160 (so no longer mount the host's .m2 directory). We might need to mount the `settings.xml` file read-only. ---
[GitHub] brooklyn-library pull request #160: jenkins: don't bind mount .m2
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn-library/pull/160 jenkins: don't bind mount .m2 As per the recommendations from Apache Infra in INFRA-16417 (comments [1] and [2]). [1] https://issues.apache.org/jira/browse/INFRA-16417?focusedCommentId=16452458=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16452458 [2] https://issues.apache.org/jira/browse/INFRA-16417?focusedCommentId=16466275=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16466275 --- This will make then jenkins build a bit slower (downloading all of the .m2 cache each time), but should fix the permissions errors we've been seeing. You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn-library fix-jenkins-build-m2 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn-library/pull/160.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 #160 commit 83417d46821a1f5dcfc25b5da9dba05b29eafb79 Author: Aled Sage Date: 2018-09-24T15:11:43Z jenkins: don't bind mount .m2 ---
[GitHub] brooklyn-library issue #158: Update Docker image to use Maven 3.5.4
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-library/pull/158 Thanks @slachiewicz - merging now. ---
[GitHub] brooklyn-library issue #159: Jenkins docker: use non-root user
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-library/pull/159 @tbouron will do, but will first confirm that brooklyn-library master build works, and will also look to see if/how we can avoid the bind mount of the writable directories. ---
[GitHub] brooklyn-docs issue #268: document add'l constraints
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-docs/pull/268 LGTM - merging. ---
[GitHub] brooklyn-library pull request #159: Jenkins docker: use non-root user
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn-library/pull/159 Jenkins docker: use non-root user Implementing part of the advice from infra in https://issues.apache.org/jira/browse/INFRA-16417 --- They advised we use `-u 910:910`, rather than the mvn command running as root in the container. Note that running the docker build on my local (mac) laptop, the user ids are interesting! In the container, (which use bind mounts for `.m2` and and the workspace) are owned by root:root. However, on my laptop, the files created are still owned by my own user. Magic?! I'm therefore not sure whether this change will make much difference. --- The other big change we need (not done here - I'll look at that next), recommended in INFRA-16417, is to STOP bind mounting .m2. They said: "Please don't use a bind mount for filesystems you intend to write to". You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn-library fix-jenkins-build Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn-library/pull/159.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 #159 commit 5a9fc95a99cd7b21fe2de9707e4a3286a358635f Author: Aled Sage Date: 2018-09-24T12:32:07Z Jenkins docker: use non-root user ---
[GitHub] brooklyn-server issue #999: Constraints - serialization and {forbidden,requi...
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/999 Looks good - happy to merge, after I've build + run tests locally. The jenkins PR build is still failing with the error below (I suspect it's a network connection problem while it's trying to download lots from github; I wonder if we can use `--depth=1` for PR builds, or if that would mess up because it wants to check if the PR is mergeable etc): ``` Caused by: hudson.plugins.git.GitException: Command "git fetch --tags --progress git://github.com/apache/brooklyn-server.git +refs/pull/*:refs/remotes/origin/pr/*" returned status code 128: ``` It would be good to also expand out the tests in `ConfigParametersYamlTest`, so that we regression-test it is always calling `ConstraintSerialization` (currently it just does `required`). I'll look at adding that, along with some more unit tests I was playing around with in `ConstraintSerializationTest`. ---
[GitHub] brooklyn-dist pull request #126: update order in pom so we get the preferred...
Github user aledsage commented on a diff in the pull request: https://github.com/apache/brooklyn-dist/pull/126#discussion_r219619890 --- Diff: all/pom.xml --- @@ -36,11 +36,35 @@ +
[GitHub] brooklyn-dist pull request #126: update order in pom so we get the preferred...
Github user aledsage commented on a diff in the pull request: https://github.com/apache/brooklyn-dist/pull/126#discussion_r219619308 --- Diff: all/pom.xml --- @@ -36,11 +36,35 @@ +east we've attempted to do this! --> --- End diff -- Extra `-->` on on this line. ---
[GitHub] brooklyn-server pull request #999: Constraints - serialization and {forbidde...
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/999#discussion_r219558955
--- Diff:
core/src/main/java/org/apache/brooklyn/core/objs/ConstraintSerialization.java
---
@@ -0,0 +1,369 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.core.objs;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.function.Supplier;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.ConfigConstraints;
+import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.collections.MutableSet;
+import org.apache.brooklyn.util.core.ResourcePredicates;
+import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.text.StringEscapes.JavaStringEscapes;
+import org.apache.brooklyn.util.text.StringPredicates;
+import org.apache.brooklyn.util.text.Strings;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+
+public class ConstraintSerialization {
+
+private final Map predicateToStringToPreferredName =
MutableMap.of();
+private final Map,Predicate>>
predicatePreferredNameToConstructor = MutableMap.of();
+
+public static class PredicateSerializationRuleAdder {
+private String preferredName;
+private Function, T> constructorArgsFromList;
+private Function> constructor;
+private Predicate predicateSample;
+private T constructorSampleInput;
+private Set equivalentNames = MutableSet.of();
+private Set> equivalentPredicateSamples =
MutableSet.of();
+
+ConstraintSerialization serialization;
+
+public PredicateSerializationRuleAdder(Function>
constructor, Function, T> constructorArgsFromList, T
constructorSampleInput) {
+this.constructorArgsFromList = constructorArgsFromList;
+this.constructor = constructor;
+this.constructorSampleInput = constructorSampleInput;
+}
+
+public static PredicateSerializationRuleAdder>>
predicateListConstructor(Function>,Predicate> constructor)
{
+PredicateSerializationRuleAdder>> result =
new PredicateSerializationRuleAdder>>(constructor,
+null, MutableList.of());
+result.constructorArgsFromList = o ->
result.serialization.toPredicateListFromJsonList(o);
+return result;
+}
+
+public static PredicateSerializationRuleAdder
stringConstructor(Function> constructor) {
+return new
PredicateSerializationRuleAdder(constructor,
+o -> Strings.toString(Iterables.getOnlyElement(o)), "");
+}
+
+public static PredicateSerializationRuleAdder
noArgConstructor(Supplier> constructor) {
+return new PredicateSerializationRuleAdder(
+(o) -> constructor.get(), o -> null, null);
+}
+
+/** Preferred name for predicate when serializing. Defaults to the
predicate name in the output of the {@link #sample(Predicate)}. */
+public PredicateSerializationRuleAdder preferredName(String
preferredName) {
+this.preferredName = preferredName;
+return this;
+}
+
+/** Other predi
[GitHub] brooklyn-server pull request #999: Constraints - serialization and {forbidde...
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/999#discussion_r219557804
--- Diff:
core/src/main/java/org/apache/brooklyn/core/objs/ConstraintSerialization.java
---
@@ -0,0 +1,369 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.core.objs;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.function.Supplier;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.ConfigConstraints;
+import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.collections.MutableSet;
+import org.apache.brooklyn.util.core.ResourcePredicates;
+import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.text.StringEscapes.JavaStringEscapes;
+import org.apache.brooklyn.util.text.StringPredicates;
+import org.apache.brooklyn.util.text.Strings;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+
+public class ConstraintSerialization {
+
+private final Map predicateToStringToPreferredName =
MutableMap.of();
+private final Map,Predicate>>
predicatePreferredNameToConstructor = MutableMap.of();
+
+public static class PredicateSerializationRuleAdder {
+private String preferredName;
+private Function, T> constructorArgsFromList;
+private Function> constructor;
+private Predicate predicateSample;
+private T constructorSampleInput;
+private Set equivalentNames = MutableSet.of();
+private Set> equivalentPredicateSamples =
MutableSet.of();
+
+ConstraintSerialization serialization;
+
+public PredicateSerializationRuleAdder(Function>
constructor, Function, T> constructorArgsFromList, T
constructorSampleInput) {
+this.constructorArgsFromList = constructorArgsFromList;
+this.constructor = constructor;
+this.constructorSampleInput = constructorSampleInput;
+}
+
+public static PredicateSerializationRuleAdder>>
predicateListConstructor(Function>,Predicate> constructor)
{
+PredicateSerializationRuleAdder>> result =
new PredicateSerializationRuleAdder>>(constructor,
+null, MutableList.of());
+result.constructorArgsFromList = o ->
result.serialization.toPredicateListFromJsonList(o);
+return result;
+}
+
+public static PredicateSerializationRuleAdder
stringConstructor(Function> constructor) {
+return new
PredicateSerializationRuleAdder(constructor,
+o -> Strings.toString(Iterables.getOnlyElement(o)), "");
+}
+
+public static PredicateSerializationRuleAdder
noArgConstructor(Supplier> constructor) {
+return new PredicateSerializationRuleAdder(
+(o) -> constructor.get(), o -> null, null);
+}
+
+/** Preferred name for predicate when serializing. Defaults to the
predicate name in the output of the {@link #sample(Predicate)}. */
+public PredicateSerializationRuleAdder preferredName(String
preferredName) {
+this.preferredName = preferredName;
+return this;
+}
+
+/** Other predi
[GitHub] brooklyn-server pull request #999: Constraints - serialization and {forbidde...
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/999#discussion_r219567394
--- Diff:
core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java ---
@@ -218,4 +228,88 @@ public LocationConfigConstraints(Location
brooklynObject) {
}
}
+public static Predicate required() {
+return new RequiredPredicate();
+}
+
+/** Predicate indicating a field is required: it must not be null and
if a string it must not be empty */
+public static class RequiredPredicate implements Predicate {
+@Override
+public boolean apply(T input) {
+if (input==null) return false;
+if (input instanceof CharSequence &&
((CharSequence)input).length()==0) return false;
+return true;
+}
+@Override
+public String toString() {
+return "required()";
+}
+}
+
+private static abstract class OtherKeyPredicate implements
BrooklynObjectPredicate {
+private String otherKeyName;
--- End diff --
Declare this final (pretty much wherever we can declare fields final, it's
a good idea).
---
[GitHub] brooklyn-server pull request #999: Constraints - serialization and {forbidde...
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/999#discussion_r219567021
--- Diff:
core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java ---
@@ -218,4 +228,88 @@ public LocationConfigConstraints(Location
brooklynObject) {
}
}
+public static Predicate required() {
+return new RequiredPredicate();
+}
+
+/** Predicate indicating a field is required: it must not be null and
if a string it must not be empty */
+public static class RequiredPredicate implements Predicate {
+@Override
+public boolean apply(T input) {
+if (input==null) return false;
+if (input instanceof CharSequence &&
((CharSequence)input).length()==0) return false;
+return true;
+}
+@Override
+public String toString() {
+return "required()";
+}
+}
+
+private static abstract class OtherKeyPredicate implements
BrooklynObjectPredicate {
+private String otherKeyName;
+
+public OtherKeyPredicate(String otherKeyName) {
+this.otherKeyName = otherKeyName;
+}
+
+public abstract String predicateName();
+
+@Override
+public String toString() {
+return
predicateName()+"("+JavaStringEscapes.wrapJavaString(otherKeyName)+")";
+}
+
+@Override
+public boolean apply(Object input) {
+return apply(input,
BrooklynTaskTags.getContextEntity(Tasks.current()));
+}
+
+@Override
+public boolean apply(Object input, BrooklynObject context) {
+if (context==null) return true;
+// would be nice to offer an explanation, but that will need a
richer API or a thread local
+return test(input,
context.config().get(ConfigKeys.newConfigKey(Object.class, otherKeyName)));
+}
+
+public abstract boolean test(Object thisValue, Object otherValue);
+
+}
+
+public static Predicate forbiddenIf(String otherKeyName) {
return new ForbiddenIfPredicate(otherKeyName); }
+public static class ForbiddenIfPredicate extends OtherKeyPredicate {
+public ForbiddenIfPredicate(String otherKeyName) {
super(otherKeyName); }
+@Override public String predicateName() { return "forbiddenIf"; }
+@Override public boolean test(Object thisValue, Object otherValue)
{
+return (thisValue==null) || (otherValue==null);
+}
+}
+
+public static Predicate forbiddenUnless(String otherKeyName) {
return new ForbiddenUnlessPredicate(otherKeyName); }
+public static class ForbiddenUnlessPredicate extends OtherKeyPredicate
{
--- End diff --
Make the classes protected at the very most. Otherwise someone will call
the class' constructor directly, and we'll be stuck with it in our public API
for ages to come.
---
[GitHub] brooklyn-server pull request #999: Constraints - serialization and {forbidde...
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/999#discussion_r219560576
--- Diff:
core/src/main/java/org/apache/brooklyn/core/objs/ConstraintSerialization.java
---
@@ -0,0 +1,369 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.core.objs;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.function.Supplier;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.ConfigConstraints;
+import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.collections.MutableSet;
+import org.apache.brooklyn.util.core.ResourcePredicates;
+import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.text.StringEscapes.JavaStringEscapes;
+import org.apache.brooklyn.util.text.StringPredicates;
+import org.apache.brooklyn.util.text.Strings;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+
+public class ConstraintSerialization {
+
+private final Map predicateToStringToPreferredName =
MutableMap.of();
+private final Map,Predicate>>
predicatePreferredNameToConstructor = MutableMap.of();
+
+public static class PredicateSerializationRuleAdder {
+private String preferredName;
+private Function, T> constructorArgsFromList;
+private Function> constructor;
+private Predicate predicateSample;
+private T constructorSampleInput;
+private Set equivalentNames = MutableSet.of();
+private Set> equivalentPredicateSamples =
MutableSet.of();
+
+ConstraintSerialization serialization;
+
+public PredicateSerializationRuleAdder(Function>
constructor, Function, T> constructorArgsFromList, T
constructorSampleInput) {
+this.constructorArgsFromList = constructorArgsFromList;
+this.constructor = constructor;
+this.constructorSampleInput = constructorSampleInput;
+}
+
+public static PredicateSerializationRuleAdder>>
predicateListConstructor(Function>,Predicate> constructor)
{
+PredicateSerializationRuleAdder>> result =
new PredicateSerializationRuleAdder>>(constructor,
+null, MutableList.of());
+result.constructorArgsFromList = o ->
result.serialization.toPredicateListFromJsonList(o);
+return result;
+}
+
+public static PredicateSerializationRuleAdder
stringConstructor(Function> constructor) {
+return new
PredicateSerializationRuleAdder(constructor,
+o -> Strings.toString(Iterables.getOnlyElement(o)), "");
+}
+
+public static PredicateSerializationRuleAdder
noArgConstructor(Supplier> constructor) {
+return new PredicateSerializationRuleAdder(
+(o) -> constructor.get(), o -> null, null);
+}
+
+/** Preferred name for predicate when serializing. Defaults to the
predicate name in the output of the {@link #sample(Predicate)}. */
+public PredicateSerializationRuleAdder preferredName(String
preferredName) {
+this.preferredName = preferredName;
+return this;
+}
+
+/** Other predi
[GitHub] brooklyn-server pull request #999: Constraints - serialization and {forbidde...
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/999#discussion_r219559231
--- Diff:
core/src/main/java/org/apache/brooklyn/core/objs/ConstraintSerialization.java
---
@@ -0,0 +1,369 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.core.objs;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.function.Supplier;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.ConfigConstraints;
+import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.collections.MutableSet;
+import org.apache.brooklyn.util.core.ResourcePredicates;
+import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.text.StringEscapes.JavaStringEscapes;
+import org.apache.brooklyn.util.text.StringPredicates;
+import org.apache.brooklyn.util.text.Strings;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+
+public class ConstraintSerialization {
+
+private final Map predicateToStringToPreferredName =
MutableMap.of();
+private final Map,Predicate>>
predicatePreferredNameToConstructor = MutableMap.of();
+
+public static class PredicateSerializationRuleAdder {
+private String preferredName;
+private Function, T> constructorArgsFromList;
+private Function> constructor;
+private Predicate predicateSample;
+private T constructorSampleInput;
+private Set equivalentNames = MutableSet.of();
+private Set> equivalentPredicateSamples =
MutableSet.of();
+
+ConstraintSerialization serialization;
+
+public PredicateSerializationRuleAdder(Function>
constructor, Function, T> constructorArgsFromList, T
constructorSampleInput) {
+this.constructorArgsFromList = constructorArgsFromList;
+this.constructor = constructor;
+this.constructorSampleInput = constructorSampleInput;
+}
+
+public static PredicateSerializationRuleAdder>>
predicateListConstructor(Function>,Predicate> constructor)
{
+PredicateSerializationRuleAdder>> result =
new PredicateSerializationRuleAdder>>(constructor,
+null, MutableList.of());
+result.constructorArgsFromList = o ->
result.serialization.toPredicateListFromJsonList(o);
+return result;
+}
+
+public static PredicateSerializationRuleAdder
stringConstructor(Function> constructor) {
+return new
PredicateSerializationRuleAdder(constructor,
+o -> Strings.toString(Iterables.getOnlyElement(o)), "");
+}
+
+public static PredicateSerializationRuleAdder
noArgConstructor(Supplier> constructor) {
+return new PredicateSerializationRuleAdder(
+(o) -> constructor.get(), o -> null, null);
+}
+
+/** Preferred name for predicate when serializing. Defaults to the
predicate name in the output of the {@link #sample(Predicate)}. */
+public PredicateSerializationRuleAdder preferredName(String
preferredName) {
+this.preferredName = preferredName;
+return this;
+}
+
+/** Other predi
[GitHub] brooklyn-server pull request #999: Constraints - serialization and {forbidde...
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/999#discussion_r219561614
--- Diff:
core/src/main/java/org/apache/brooklyn/core/objs/ConstraintSerialization.java
---
@@ -0,0 +1,369 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.core.objs;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.function.Supplier;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.ConfigConstraints;
+import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.collections.MutableSet;
+import org.apache.brooklyn.util.core.ResourcePredicates;
+import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.text.StringEscapes.JavaStringEscapes;
+import org.apache.brooklyn.util.text.StringPredicates;
+import org.apache.brooklyn.util.text.Strings;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+
+public class ConstraintSerialization {
+
+private final Map predicateToStringToPreferredName =
MutableMap.of();
+private final Map,Predicate>>
predicatePreferredNameToConstructor = MutableMap.of();
+
+public static class PredicateSerializationRuleAdder {
+private String preferredName;
+private Function, T> constructorArgsFromList;
+private Function> constructor;
+private Predicate predicateSample;
+private T constructorSampleInput;
+private Set equivalentNames = MutableSet.of();
+private Set> equivalentPredicateSamples =
MutableSet.of();
+
+ConstraintSerialization serialization;
+
+public PredicateSerializationRuleAdder(Function>
constructor, Function, T> constructorArgsFromList, T
constructorSampleInput) {
+this.constructorArgsFromList = constructorArgsFromList;
+this.constructor = constructor;
+this.constructorSampleInput = constructorSampleInput;
+}
+
+public static PredicateSerializationRuleAdder>>
predicateListConstructor(Function>,Predicate> constructor)
{
+PredicateSerializationRuleAdder>> result =
new PredicateSerializationRuleAdder>>(constructor,
+null, MutableList.of());
+result.constructorArgsFromList = o ->
result.serialization.toPredicateListFromJsonList(o);
+return result;
+}
+
+public static PredicateSerializationRuleAdder
stringConstructor(Function> constructor) {
+return new
PredicateSerializationRuleAdder(constructor,
+o -> Strings.toString(Iterables.getOnlyElement(o)), "");
+}
+
+public static PredicateSerializationRuleAdder
noArgConstructor(Supplier> constructor) {
+return new PredicateSerializationRuleAdder(
+(o) -> constructor.get(), o -> null, null);
+}
+
+/** Preferred name for predicate when serializing. Defaults to the
predicate name in the output of the {@link #sample(Predicate)}. */
+public PredicateSerializationRuleAdder preferredName(String
preferredName) {
+this.preferredName = preferredName;
+return this;
+}
+
+/** Other predi
[GitHub] brooklyn-server issue #999: Constraints - serialization and {forbidden,requi...
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/999 retest this please Build failure was environmental: ``` ERROR: Error fetching remote repo 'origin' hudson.plugins.git.GitException: Failed to fetch from git://github.com/apache/brooklyn-server.git ``` ---
[GitHub] brooklyn-server pull request #998: Pom tidy, and version upgrades
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn-server/pull/998 Pom tidy, and version upgrades See individual commits (the first one just moves stuff around). You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn-server pom-tidy Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn-server/pull/998.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 #998 commit bb86fe38f5bc9bf62cc1c65cac19d17d6fbf88cc Author: Aled Sage Date: 2018-09-15T21:23:18Z tidy pom.xml Groups and comments dependencies more sensibly, but does not change/add/remove any of those dependencies. commit bab742a15ba8ed1ba619cd0698cc0cafe735dacf Author: Aled Sage Date: 2018-09-18T07:49:20Z pom: remove commons-beanutils version conflict camp-brooklyn depended on 1.9.1. parent pom declared 1.9.3. commit fdc7d82f6d69d3db956b805e45ec57707709dd5d Author: Aled Sage Date: 2018-09-18T07:49:48Z pom: bump felix-framework version to match Kara 4.1.6 commit 57f5da6bb02c0f5acc0dc366d82e65e1dda17711 Author: Aled Sage Date: 2018-09-18T07:51:09Z pom: bump gson to 2.5, to match jclouds ---
[GitHub] brooklyn-server pull request #997: Add testMergeMapsPreferringSecondOnConfli...
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn-server/pull/997 Add testMergeMapsPreferringSecondOnConflict Following on from https://github.com/apache/brooklyn-server/pull/996, this adds a test for that functionality. You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn-server test-CollectionMerger-preferSecond Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn-server/pull/997.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 #997 commit 71bf99b08a67483e5eb5ff81db801d09c4ab62d3 Author: Aled Sage Date: 2018-09-18T06:39:11Z Add testMergeMapsPreferringSecondOnConflict ---
[GitHub] brooklyn-server pull request #989: LogWatcher - don't change log level if no...
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/989#discussion_r218311895
--- Diff:
test-support/src/main/java/org/apache/brooklyn/test/LogWatcher.java ---
@@ -268,17 +248,14 @@ public void assertHasEvent() {
@Override
public void run() {
assertFalse(events.isEmpty());
+System.out.println("EVENTS: "+events);
--- End diff --
Don't include `System.out`
---
[GitHub] brooklyn-server issue #992: minor simple test fixes/improvements
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/992 LGTM; merging. ---
[GitHub] brooklyn-ui pull request #71: allow palette footer to be customised
Github user aledsage commented on a diff in the pull request: https://github.com/apache/brooklyn-ui/pull/71#discussion_r218071640 --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.directive.js --- @@ -235,6 +235,9 @@ function controller($scope, $element, $q, $uibModal, $log, $templateCache, palet // downstream can override this to insert lines below the header $scope.customSubHeadTemplateName = 'composer-palette-empty-sub-head'; $templateCache.put($scope.customSubHeadTemplateName, ''); + +$scope.customFooterTemplateName = 'composer-palette-empty-foort'; --- End diff -- Typo? Should this be `empty-footer`? ---
[GitHub] brooklyn-ui pull request #71: allow palette footer to be customised
Github user aledsage commented on a diff in the pull request: https://github.com/apache/brooklyn-ui/pull/71#discussion_r218076598 --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.template.html --- @@ -61,7 +61,7 @@
[GitHub] brooklyn-ui issue #70: Bump karaf to 4.1.6 (from 4.1.2)
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-ui/pull/70 retest this please ---
[GitHub] brooklyn-server issue #995: Bump karaf to 4.1.6 (from 4.1.2)
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/995 Main change was reviewed in #994; therefore merging this now. ---
[GitHub] brooklyn-ui issue #70: Bump karaf to 4.1.6 (from 4.1.2)
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-ui/pull/70 Jenkins failed because https://github.com/apache/brooklyn-server/pull/995 is not yet merged. Error was: ``` Message: Unable to resolve root: missing requirement [root] osgi.identity; osgi.identity=brooklyn-ui-proxy; type=karaf.feature; version=1.0.0.SNAPSHOT; filter:="(&(osgi.identity=brooklyn-ui-proxy)(type=karaf.feature)(version>=1.0.0.SNAPSHOT))" [caused by: Unable to resolve brooklyn-ui-proxy/1.0.0.SNAPSHOT: missing requirement [brooklyn-ui-proxy/1.0.0.SNAPSHOT] osgi.identity; osgi.identity=brooklyn-ui-proxy; type=osgi.bundle; version="[1.0.0.SNAPSHOT,1.0.0.SNAPSHOT]"; resolution:=mandatory [caused by: Unable to resolve brooklyn-ui-proxy/1.0.0.SNAPSHOT: missing requirement [brooklyn-ui-proxy/1.0.0.SNAPSHOT] osgi.wiring.package; filter:="(&(osgi.wiring.package=org.eclipse.jetty.proxy)(version>=9.3.0)(!(version>=10.0.0)))" [caused by: Unable to resolve org.eclipse.jetty.proxy/9.3.24.v20180605: missing requirement [org.eclipse.jetty.proxy/9.3.24.v20180605] osgi.wiring.package; filter:="(&(osgi.wiring.package=org.eclipse.jetty.client)(version>=9.3.24)(!(version>=9.3.25)))"]]] ``` ---
[GitHub] brooklyn-server issue #994: bump karaf version to get consistent jetty
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/994 Suggest we close this, and instead merge: https://github.com/apache/brooklyn-server/pull/995 https://github.com/apache/brooklyn-ui/pull/70 If jenkins builds work for those, then I'll merge those PRs. ---
[GitHub] brooklyn-ui pull request #70: Bump karaf to 4.1.6 (from 4.1.2)
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn-ui/pull/70 Bump karaf to 4.1.6 (from 4.1.2) See https://github.com/apache/brooklyn-server/pull/995 You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn-ui karaf-bump-version Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn-ui/pull/70.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 #70 commit 04765962498a04ca77a9847858564f4dc574b5ea Author: Aled Sage Date: 2018-09-15T20:45:36Z Bump karaf to 4.1.6 (from 4.1.2) ---
[GitHub] brooklyn-server pull request #995: Bump karaf to 4.1.6 (from 4.1.2)
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn-server/pull/995 Bump karaf to 4.1.6 (from 4.1.2) Bumping jetty version to 9.3.24 (from 9.3.14) done in #991 clashes with jetty that ships in karaf 4.1.2, breaking the build. Therefore bumping karaf to 4.1.6. Replaces https://github.com/apache/brooklyn-server/pull/994 - difference is this PR also bumps the `karaf.plugin.version` to match. You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn-server karaf-bump-version Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn-server/pull/995.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 #995 commit 211cfb40e403d2df5288f51b6ae7178b0140f8b2 Author: Aled Sage Date: 2018-09-15T20:44:48Z Bump karaf to 4.1.6 (from 4.1.2) ---
[GitHub] brooklyn-server issue #994: bump karaf version to get consistent jetty
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/994 Confirmed that with *my* build (with the additional edits I listed above), the `brooklyn-dist/karaf/apache-brooklyn/target/assembly` works. I suggest we also change the `karaf.plugin.version` and the two version ranges to be 4.1.6. Then everything is more consistent (even if it's not essential on your laptop @ahgittin). The other version differences we can tackle in separate PRs - they were already out-of-sync. ---
[GitHub] brooklyn-server issue #994: bump karaf version to get consistent jetty
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/994 Eye-balling what karaf 4.1.6 ships with, what we have in the brooklyn based on 4.1.2, and what dependencies we declare in the poms, there are a few discrepancies: ``` Karaf 4.1.6 (vanilla) Broolyn dependency Previous Brooklyn Karaf distro (4.1.2)New Brooklyn Karaf distro (4.1.6) slf4j-api-1.7.12.jar1.7.25 slf4j-api-1.7.12.jar and jul-to-slf4j-1.7.25 slf4j-api-1.7.12.jar and jul-to-slf4j-1.7.25 asm-all-5.2.jar 5.0.4asm-all-5.2.jar and asm-all-5.0.2.jar asm-all-5.2.jar and asm-all-5.0.2.jar jansi-1.17.1.jar1.2.1jansi-1.16.jar jansi-1.17.1.jar org/jline:jline-3.6.2.jar jline:jline-2.12 jline-3.4.0.jar jline-3.6.2.jar felix.framework-5.6.10.jar 5.6.1felix.framework-5.6.6 felix.framework-5.6.10.jar ``` ---
[GitHub] brooklyn-server issue #994: bump karaf version to get consistent jetty
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/994 I changed a few more things as well when looking at this: In `pom.xml`: ``` 4.1.6 ``` In `karaf/features/pom.xml`: ``` [4.1.6,) ``` https://github.com/apache/brooklyn-ui/blob/master/modularity-server/features/pom.xml#L179 to version range `[4.1.6,)`. Without this last change, it failed with: ``` [ERROR] Failed to execute goal org.apache.karaf.tooling:karaf-maven-plugin:4.1.2:verify (verify-brooklyn-ui-modularity-feature) on project brooklyn-ui-modularity-features: Verification failures: Verification failures: [ERROR] Unable to resolve framework features [ERROR] Unable to resolve framework features [ERROR] Unable to resolve framework features ``` --- On my old laptop, when trying to build with `mvnf ./ -DscmBranch=mater -DbuildNumber=1.0.0-SNAPSHOT`, I also had a weird (unrelated?) error, which went away when I bumped the `swagger-maven-plugin` to 3.1.7 (from 3.1.4). However I didn't hit this error when I built it on my new laptop! ``` [INFO] Brooklyn REST API .. FAILURE [ 2.858 s] [ERROR] Failed to execute goal com.github.kongchen:swagger-maven-plugin:3.1.4:generate (default) on project brooklyn-rest-api: Execution default of goal com.github.kongchen:swagger-maven-plugin:3.1.4:generate failed: Plugin com.github.kongchen:swagger-maven-plugin:3.1.4 or one of its dependencies could not be resolved: Failed to collect dependencies at com.github.kongchen:swagger-maven-plugin:jar:3.1.4 -> org.apache.commons:commons-lang3:jar:[3.4,4.0): No versions available for org.apache.commons:commons-lang3:jar:[3.4,4.0) within specified range -> ``` ---
[GitHub] brooklyn-server issue #991: bump jetty version in response to recent CVE's
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/991
LGTM. Merging now.
The failing test looks unrelated
(`VanillaSoftwareProcessYamlTest.testAlternativeServiceUpPolling`). Confirmed
it passed locally for me. Looking at the test, I could well believe it is
non-deterministic. We'll look at that separately.
For the record, the way this gets into karaf seems to be in
`brooklyn-ui/modularity-server/features/src/main/feature/feature.xml`:
```
mvn:org.eclipse.jetty/jetty-proxy/${jetty.version}
```
---
[GitHub] brooklyn-server pull request #990: Revert "This closes #988"
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn-server/pull/990 Revert "This closes #988" This reverts commit f6ad11846de2d9ba2e9004648179270e3ca3c25f, reversing changes made to c782aae54f424e317c0999f5cde3fab19bc45cfb. This PR broke `DynamicClusterWithAvailabilityZonesRebindTest.testRebindWithCustomZoneFailureDetector`, because it caused the feed to be persisted (which should not have been). You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn-server revert-988 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn-server/pull/990.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 #990 commit fc5abc7164b11b11107efa276cfa2b90928d3228 Author: Aled Sage Date: 2018-09-12T13:43:54Z Revert "This closes #988" This reverts commit f6ad11846de2d9ba2e9004648179270e3ca3c25f, reversing changes made to c782aae54f424e317c0999f5cde3fab19bc45cfb. ---
[GitHub] brooklyn-ui issue #68: composer tidies
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-ui/pull/68 LGTM, merging. @tbouron is you get a chance to take a look post-merge, that would be appreciated. ---
[GitHub] brooklyn-server pull request #988: Fix AutoScalerPolicyRebindTest: when high...
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn-server/pull/988 Fix AutoScalerPolicyRebindTest: when highlights change, request re-pe⦠â¦rsist You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn-server fix-AutoScalerPolicyRebindTest Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn-server/pull/988.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 #988 commit cf1e72b989f9e62af6f1951631f2ea5a5fde5c29 Author: Aled Sage Date: 2018-09-10T23:28:12Z Fix AutoScalerPolicyRebindTest: when highlights change, request re-persist ---
[GitHub] brooklyn-server pull request #987: Fix DSL recursive-reference detection
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn-server/pull/987 Fix DSL recursive-reference detection As per the code comment, I broke this in https://github.com/apache/brooklyn-server/pull/971. This PR fixes it. You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn-server fix-dsl-recursion-detection Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn-server/pull/987.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 #987 commit 5bc7546b9e2f5af743fad41533322c7804dd8e1b Author: Aled Sage Date: 2018-09-07T18:50:39Z Fix DSL recursive-reference detection ---
[GitHub] brooklyn-server pull request #986: BROOKLYN-600: cleanup entities on deploy-...
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn-server/pull/986 BROOKLYN-600: cleanup entities on deploy-failure Fix for https://issues.apache.org/jira/browse/BROOKLYN-600 Note the `LocalEntityManager. discardPremanaged(Entity)` is a bit more complex/long that it might need to be because I want to make sure it does no harm - fail if the entity or any child is already managed (because then should call `unmanage()` instead. You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn-server BROOKLYN-600 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn-server/pull/986.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 #986 commit a87e6692a54a10d389165fd82c93b3fd2f4ccc03 Author: Aled Sage Date: 2018-09-06T11:38:05Z BROOKLYN-600: cleanup entities on deploy-failure ---
[GitHub] brooklyn-server pull request #985: BROOKLYN-599: fix getApplication()
GitHub user aledsage opened a pull request: https://github.com/apache/brooklyn-server/pull/985 BROOKLYN-599: fix getApplication() Fixes https://issues.apache.org/jira/browse/BROOKLYN-599 You can merge this pull request into a Git repository by running: $ git pull https://github.com/aledsage/brooklyn-server BROOKLYN-599 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/brooklyn-server/pull/985.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 #985 commit 25de68863a98b8c66450e134ff758240b110e13b Author: Aled Sage Date: 2018-09-06T09:02:47Z BROOKLYN-599: fix getApplication() ---
[GitHub] brooklyn-server issue #984: restore the field `subType` for sub-element conf...
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/984 +1, LGTM; waiting for jenkins to finish running tests. ---
[GitHub] brooklyn-server issue #971: DSL for $brooklyn:location()
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/971 Thanks @geomacy @nakomis - merging now. Brooklyn issues are probably the best way to record and track further improvements, I think. ---
[GitHub] brooklyn-server issue #971: DSL for $brooklyn:location()
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/971
Thanks @geomacy @nakomis
I've added `testDslLocationIndexOutOfBounds`, which asserts that we get a
nice exception when one tries to retrieve in anger the config value that has an
invalid index.
I think this is tricky to improve, without going into bigger more general
changes.
As you point out, for the blueprint:
```
location: localhost
services:
- type: org.apache.brooklyn.entity.stock.BasicApplication
brooklyn.config:
config1: $brooklyn:location()
config2: $brooklyn:location("0")
config3: $brooklyn:location("1")
config4: $brooklyn:attributeWhenReady("sensorDoesNotExist")
```
`br app obrjk09hec config` returns:
```
Key | Value
config1 | map[type:org.apache.brooklyn.api.location.Location
id:whoq9008c0]
defaultDisplayName |
camp.template.id | ClysL4Gs
config2 | map[type:org.apache.brooklyn.api.location.Location
id:whoq9008c0]
config3 | map[component:map[componentId:
componentIdSupplier: scopeComponent: scope:THIS] index:1]
config4 | map[component:map[componentId:
componentIdSupplier: scopeComponent: scope:THIS]
sensorName:sensorDoesNotExist]
```
The `config3` evaluation has only been logged at debug, because the value
isn't yet being used - it's just retrieved for display purposes, so hasn't
blocked. It failed to resolve immediately, so we go the DSL object back. You
can see we get the same behaviour for an `attributeWhenReady` that cannot yet
be evaluated.
If we wrote a blueprint that actually tried to use it (i.e. that caused
`entity.config().get("config3")` to be called), then it would throw the
exception, which would be logged at warn.
The two things you point out would be good more general improvements:
1.
`/v1/applications//entities//config/current-state?raw=false`
should return nicer representation of DSL objects (e.g. calling `toString()` on
it, rather than showing the values of the fields `component` and `index`).
2. Location representation in the above call doesn't give much info - just
the id, and that it's a location. It would be good to give more info as well
(e.g. the display name).
---
Are you ok with merging this PR as-is, and us making those more general
improvements separately?
---
[GitHub] brooklyn-server pull request #982: Improve coercions esp with generics
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/982#discussion_r214612451
--- Diff:
utils/common/src/test/java/org/apache/brooklyn/util/text/StringEscapesTest.java
---
@@ -115,4 +117,80 @@ public void testJavaLists() {
JavaStringEscapes.unwrapJsonishListIfPossible("\"\",,x,\"\""));
}
+@Test
+public void testJavaListString() {
+Assert.assertEquals(MutableList.of("hello", "world"),
+JavaStringEscapes.unwrapQuotedJavaStringList("\"hello\",
\"world\"", ","));
+try {
+JavaStringEscapes.unwrapQuotedJavaStringList("\"hello\",
world", ",");
+Assert.fail("Should have thrown");
+} catch (Exception e) { /* expected */ }
+
+Assert.assertEquals(MutableList.of("hello", "world"),
+
JavaStringEscapes.unwrapJsonishListStringIfPossible("\"hello\", \"world\""));
+Assert.assertEquals(MutableList.of("hello"),
+JavaStringEscapes.unwrapJsonishListStringIfPossible("hello"));
+Assert.assertEquals(MutableList.of("hello", "world"),
+JavaStringEscapes.unwrapJsonishListStringIfPossible("hello,
world"));
+Assert.assertEquals(MutableList.of("hello", "world"),
+
JavaStringEscapes.unwrapJsonishListStringIfPossible("\"hello\", world"));
+Assert.assertEquals(MutableList.of("hello", "world"),
+JavaStringEscapes.unwrapJsonishListStringIfPossible("[
\"hello\", world ]"));
+// if can't parse e.g. because contains double quote, then returns
original string as single element list
+Assert.assertEquals(MutableList.of("hello\", \"world\""),
+JavaStringEscapes.unwrapJsonishListStringIfPossible("hello\",
\"world\""));
+Assert.assertEquals(MutableList.of(),
+JavaStringEscapes.unwrapJsonishListStringIfPossible(" "));
+Assert.assertEquals(MutableList.of(""),
+JavaStringEscapes.unwrapJsonishListStringIfPossible("\"\""));
+Assert.assertEquals(MutableList.of("x"),
+JavaStringEscapes.unwrapJsonishListStringIfPossible(",,x,"));
+Assert.assertEquals(MutableList.of("","x",""),
+
JavaStringEscapes.unwrapJsonishListStringIfPossible("\"\",,x,\"\""));
+}
+
+@Test
+public void testJavaListObject() {
+Assert.assertEquals(MutableList.of("hello", "world"),
+JavaStringEscapes.tryUnwrapJsonishList("\"hello\",
\"world\"").get());
+Assert.assertEquals(MutableList.of("hello"),
+JavaStringEscapes.tryUnwrapJsonishList("hello").get());
+Assert.assertEquals(MutableList.of("hello", "world"),
+JavaStringEscapes.tryUnwrapJsonishList("hello, world").get());
+Assert.assertEquals(MutableList.of("hello", "world"),
+JavaStringEscapes.tryUnwrapJsonishList("\"hello\",
world").get());
+Assert.assertEquals(MutableList.of("hello", "world"),
+JavaStringEscapes.tryUnwrapJsonishList("[ \"hello\", world
]").get());
+Assert.assertEquals(MutableList.of(),
+JavaStringEscapes.tryUnwrapJsonishList(" ").get());
+Assert.assertEquals(MutableList.of(""),
+JavaStringEscapes.tryUnwrapJsonishList("\"\"").get());
+Assert.assertEquals(MutableList.of("","","x",""),
+JavaStringEscapes.tryUnwrapJsonishList(",,x,").get());
+Assert.assertEquals(MutableList.of("","","x",""),
+JavaStringEscapes.tryUnwrapJsonishList("\"\",,x,\"\"").get());
+Assert.assertEquals(MutableList.of(MutableMap.of("a",
1),MutableMap.of("b", 2)),
+JavaStringEscapes.tryUnwrapJsonishList("[ a : 1, b : 2
]").get());
+
+Assert.assertEquals(MutableList.of(1, 2, "buckle my shoe",
true, "true", null, "null", ","),
--- End diff --
Can't see any tests for doubles.
---
[GitHub] brooklyn-server pull request #982: Improve coercions esp with generics
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/982#discussion_r214640708
--- Diff:
utils/common/src/test/java/org/apache/brooklyn/util/text/StringEscapesTest.java
---
@@ -115,4 +117,80 @@ public void testJavaLists() {
JavaStringEscapes.unwrapJsonishListIfPossible("\"\",,x,\"\""));
}
+@Test
+public void testJavaListString() {
+Assert.assertEquals(MutableList.of("hello", "world"),
+JavaStringEscapes.unwrapQuotedJavaStringList("\"hello\",
\"world\"", ","));
+try {
+JavaStringEscapes.unwrapQuotedJavaStringList("\"hello\",
world", ",");
+Assert.fail("Should have thrown");
+} catch (Exception e) { /* expected */ }
+
+Assert.assertEquals(MutableList.of("hello", "world"),
--- End diff --
These assert calls are the wrong way round - first argument is `actual`,
second argument is `expected` for testng.
---
[GitHub] brooklyn-server pull request #982: Improve coercions esp with generics
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/982#discussion_r214615588
--- Diff:
utils/common/src/main/java/org/apache/brooklyn/util/guava/Maybe.java ---
@@ -365,7 +365,7 @@ public RuntimeException getException() {
});
}
public static Maybe changeExceptionSupplier(Maybe
original, Function,Supplier> transform) {
-if (original.isPresent()) return original;
+if (original==null || original.isPresent()) return original;
--- End diff --
I think we should fail-fast if you pass in null. The caller has asked to
change the exception supplier, and we've ignored their request without telling
them of the problem.
This kind of null check just leads to more `NullPointerException`s later in
the code paths, or more null checks later (which coders/reviewers would not
expect to have to do when using `Maybe` or `Optional`).
---
[GitHub] brooklyn-server pull request #982: Improve coercions esp with generics
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/982#discussion_r214611925
--- Diff:
utils/common/src/main/java/org/apache/brooklyn/util/text/StringEscapes.java ---
@@ -315,12 +320,121 @@ else if (c=='\"') {
throw new IllegalArgumentException("String '"+s+"' is not a
valid Java string (unterminated string)");
}
+/** @deprecated since 1.0.0, use {@link
#unwrapJsonishListStringIfPossible(String)} (old semantics)
+ * or {@link #unwrapJsonishListStringIfPossible(String)}
(improved) */
+public static List unwrapJsonishListIfPossible(String
input) {
+return unwrapJsonishListStringIfPossible(input);
+}
+
+/** converts a comma separated list in a single string to a list
of json primitives or maps,
+ * falling back to returning the input.
+ *
+ * specifically:
+ * 1) if of form [ X ] (in brackets after trim),
parse as YAML;
+ * if parse succeeds return the result, or if parse fails,
return {@link Maybe#absent()}.
+ * 2) if not of form [ X ], wrap in brackets and
parse as YAML,
+ * and if that succeeds and is a list, return the result.
+ * 3) otherwise, expect comma-separated tokens which after
trimming are of the form "A" or B,
+ * where "A" is a valid Java string or C is any string not
starting with '
+ * and not containing " or ,. return the list of those
tokens, where A and B
+ * are their string value, and C as a primitive if it is a
number or boolean or null,
+ * or else a string, including the empty string if empty.
+ * 4) if such tokens are not found, return {@link
Maybe#absent()}.
+ *
+ * @see #unwrapOptionallyQuotedJavaStringList(String)
+ **/
+public static Maybe> tryUnwrapJsonishList(String
input) {
+if (input==null) return Maybe.absent("null input cannot unwrap
to a list");
+String inputT = input.trim();
+
+String inputYaml = null;
+if (!inputT.startsWith("[") && !inputT.endsWith("]")) {
+inputYaml = "[" + inputT + "]";
+}
+if (inputT.startsWith("[") && inputT.endsWith("]")) {
+inputYaml = inputT;
+}
+if (inputYaml!=null) {
+try {
+Object r = Iterables.getOnlyElement(
Yamls.parseAll(inputYaml) );
+if (r instanceof List) {
+@SuppressWarnings("unchecked")
+List result = (List)r;
+return Maybe.of(result);
+}
+} catch (Exception e) {}
+if (inputT.startsWith("[")) {
+// if supplied as yaml, don't allow failures
+return Maybe.absent("Supplied format looked like YAML
but could not parse as YAML");
+}
+}
+
+List result = MutableList.of();
+
+// double quote: ^ \s* " ([not quote or backslash] or
[backslash any-char])* " \s* (, or $)
+Pattern dq =
Pattern.compile("^\\s*(\"([^\"]|[.])*\")\\s*(,|$)");
+// could also support this, but we need new unescape routines
+//// single quote: ^ \s* ' ([not quote or backslash] or
[backslash any-char])* ' \s* (, or $)
+//Pattern sq =
Pattern.compile("^\\s*'([^\']|[.])'*\\s*(,|$)");
+// no quote: ^ \s* (empty, or [not ' or " or space] ([not ,
or "]* [not , or " or space])?) \s* (, or $)
+Pattern nq =
Pattern.compile("^\\s*(|[^,\"\\s]([^,\"]*[^,\"\\s])?)\\s*(,|$)");
+
+int removedChars = 0;
+while (true) {
+Object ri;
+Matcher m = dq.matcher(input);
+if (m.find()) {
+try {
+ri = unwrapJavaString(m.group(1));
+} catch (Exception e) {
+Exceptions.propagateIfFatal(e);
+return Maybe.absent("Could not match valid quote
pattern" +
+(removedChars>0 ? " at position "+removedC
[GitHub] brooklyn-server pull request #982: Improve coercions esp with generics
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/982#discussion_r214640867
--- Diff:
utils/common/src/test/java/org/apache/brooklyn/util/text/StringEscapesTest.java
---
@@ -115,4 +117,80 @@ public void testJavaLists() {
JavaStringEscapes.unwrapJsonishListIfPossible("\"\",,x,\"\""));
}
+@Test
+public void testJavaListString() {
+Assert.assertEquals(MutableList.of("hello", "world"),
+JavaStringEscapes.unwrapQuotedJavaStringList("\"hello\",
\"world\"", ","));
+try {
+JavaStringEscapes.unwrapQuotedJavaStringList("\"hello\",
world", ",");
+Assert.fail("Should have thrown");
+} catch (Exception e) { /* expected */ }
+
+Assert.assertEquals(MutableList.of("hello", "world"),
+
JavaStringEscapes.unwrapJsonishListStringIfPossible("\"hello\", \"world\""));
--- End diff --
Minor: I'd follow the java convention (and the Brooklyn convention) for
indents: 8 spaces. The 4 spaces looks too much like a new code block, rather
than a continuation of the previous line.
---
[GitHub] brooklyn-server pull request #982: Improve coercions esp with generics
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/982#discussion_r214612997
--- Diff:
core/src/main/java/org/apache/brooklyn/util/core/flags/MethodCoercions.java ---
@@ -87,13 +89,23 @@ public boolean apply(@Nullable Method input) {
Method accessibleMethod =
Reflections.findAccessibleMethod(method).get();
try {
Type paramType = method.getGenericParameterTypes()[0];
-Object coercedArgument = TypeCoercions.coerce(argument,
TypeToken.of(paramType));
-return Maybe.of(accessibleMethod.invoke(instance,
coercedArgument));
+Maybe coercedArgumentM =
TypeCoercions.tryCoerce(argument, TypeToken.of(paramType));
+RuntimeException exception =
Maybe.getException(coercedArgumentM);
--- End diff --
Looks wrong - this will cast `coercedArgumentM` to `Maybe.absent`, but on
the line below you do `coercedArgumentM.isPresent`. So presumably the call to
`getException` will sometimes throw a `ClassCastException`?
Ah, I see you've changed the semantics of `getException` to return null if
it wasn't an exception.
Why change it? I liked the way that asking for the exception when `present`
was a programming error - there will never be an exception when present. It
looks like you can easily avoid that by removing this line, and changing the if
statement below to just `if (coercedArgumentM.isAbsent()) {`.
---
[GitHub] brooklyn-server pull request #982: Improve coercions esp with generics
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/982#discussion_r214610344
--- Diff:
utils/common/src/main/java/org/apache/brooklyn/util/text/StringEscapes.java ---
@@ -315,12 +320,121 @@ else if (c=='\"') {
throw new IllegalArgumentException("String '"+s+"' is not a
valid Java string (unterminated string)");
}
+/** @deprecated since 1.0.0, use {@link
#unwrapJsonishListStringIfPossible(String)} (old semantics)
+ * or {@link #unwrapJsonishListStringIfPossible(String)}
(improved) */
+public static List unwrapJsonishListIfPossible(String
input) {
+return unwrapJsonishListStringIfPossible(input);
+}
+
+/** converts a comma separated list in a single string to a list
of json primitives or maps,
+ * falling back to returning the input.
+ *
+ * specifically:
+ * 1) if of form [ X ] (in brackets after trim),
parse as YAML;
+ * if parse succeeds return the result, or if parse fails,
return {@link Maybe#absent()}.
+ * 2) if not of form [ X ], wrap in brackets and
parse as YAML,
+ * and if that succeeds and is a list, return the result.
+ * 3) otherwise, expect comma-separated tokens which after
trimming are of the form "A" or B,
+ * where "A" is a valid Java string or C is any string not
starting with '
+ * and not containing " or ,. return the list of those
tokens, where A and B
+ * are their string value, and C as a primitive if it is a
number or boolean or null,
+ * or else a string, including the empty string if empty.
+ * 4) if such tokens are not found, return {@link
Maybe#absent()}.
+ *
+ * @see #unwrapOptionallyQuotedJavaStringList(String)
+ **/
+public static Maybe> tryUnwrapJsonishList(String
input) {
+if (input==null) return Maybe.absent("null input cannot unwrap
to a list");
+String inputT = input.trim();
+
+String inputYaml = null;
+if (!inputT.startsWith("[") && !inputT.endsWith("]")) {
+inputYaml = "[" + inputT + "]";
+}
+if (inputT.startsWith("[") && inputT.endsWith("]")) {
+inputYaml = inputT;
+}
+if (inputYaml!=null) {
+try {
+Object r = Iterables.getOnlyElement(
Yamls.parseAll(inputYaml) );
+if (r instanceof List) {
+@SuppressWarnings("unchecked")
+List result = (List)r;
+return Maybe.of(result);
+}
+} catch (Exception e) {}
--- End diff --
Strong preference to never have empty catch blocks. I'd do:
```
catch(Exception e) {
Exceptions.propagateIfFatal(e);
// Logic below decides whether to return absent or to keep trying
}
```
---
[GitHub] brooklyn-server pull request #982: Improve coercions esp with generics
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/982#discussion_r214619292
--- Diff:
utils/common/src/main/java/org/apache/brooklyn/util/guava/Maybe.java ---
@@ -501,6 +501,6 @@ public boolean equals(Object obj) {
/** Finds the {@link Absent#getException()} if {@link #isAbsent()}, or
null */
public static RuntimeException getException(Maybe t) {
-return ((Maybe.Absent)t).getException();
+return t instanceof Maybe.Absent ?
((Maybe.Absent)t).getException() : null;
--- End diff --
As per earlier comment, I'd leave it as throwing an exception if you try to
get the exception of a `present`.
---
[GitHub] brooklyn-server pull request #982: Improve coercions esp with generics
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/982#discussion_r214642777
--- Diff:
utils/common/src/main/java/org/apache/brooklyn/util/text/StringEscapes.java ---
@@ -315,12 +320,121 @@ else if (c=='\"') {
throw new IllegalArgumentException("String '"+s+"' is not a
valid Java string (unterminated string)");
}
+/** @deprecated since 1.0.0, use {@link
#unwrapJsonishListStringIfPossible(String)} (old semantics)
+ * or {@link #unwrapJsonishListStringIfPossible(String)}
(improved) */
+public static List unwrapJsonishListIfPossible(String
input) {
+return unwrapJsonishListStringIfPossible(input);
+}
+
+/** converts a comma separated list in a single string to a list
of json primitives or maps,
+ * falling back to returning the input.
+ *
+ * specifically:
+ * 1) if of form [ X ] (in brackets after trim),
parse as YAML;
+ * if parse succeeds return the result, or if parse fails,
return {@link Maybe#absent()}.
+ * 2) if not of form [ X ], wrap in brackets and
parse as YAML,
+ * and if that succeeds and is a list, return the result.
+ * 3) otherwise, expect comma-separated tokens which after
trimming are of the form "A" or B,
+ * where "A" is a valid Java string or C is any string not
starting with '
+ * and not containing " or ,. return the list of those
tokens, where A and B
+ * are their string value, and C as a primitive if it is a
number or boolean or null,
+ * or else a string, including the empty string if empty.
+ * 4) if such tokens are not found, return {@link
Maybe#absent()}.
+ *
+ * @see #unwrapOptionallyQuotedJavaStringList(String)
+ **/
+public static Maybe> tryUnwrapJsonishList(String
input) {
+if (input==null) return Maybe.absent("null input cannot unwrap
to a list");
+String inputT = input.trim();
+
+String inputYaml = null;
+if (!inputT.startsWith("[") && !inputT.endsWith("]")) {
+inputYaml = "[" + inputT + "]";
+}
+if (inputT.startsWith("[") && inputT.endsWith("]")) {
+inputYaml = inputT;
+}
+if (inputYaml!=null) {
+try {
+Object r = Iterables.getOnlyElement(
Yamls.parseAll(inputYaml) );
+if (r instanceof List) {
+@SuppressWarnings("unchecked")
+List result = (List)r;
+return Maybe.of(result);
+}
+} catch (Exception e) {}
+if (inputT.startsWith("[")) {
+// if supplied as yaml, don't allow failures
+return Maybe.absent("Supplied format looked like YAML
but could not parse as YAML");
+}
+}
+
+List result = MutableList.of();
+
+// double quote: ^ \s* " ([not quote or backslash] or
[backslash any-char])* " \s* (, or $)
+Pattern dq =
Pattern.compile("^\\s*(\"([^\"]|[.])*\")\\s*(,|$)");
+// could also support this, but we need new unescape routines
+//// single quote: ^ \s* ' ([not quote or backslash] or
[backslash any-char])* ' \s* (, or $)
+//Pattern sq =
Pattern.compile("^\\s*'([^\']|[.])'*\\s*(,|$)");
+// no quote: ^ \s* (empty, or [not ' or " or space] ([not ,
or "]* [not , or " or space])?) \s* (, or $)
+Pattern nq =
Pattern.compile("^\\s*(|[^,\"\\s]([^,\"]*[^,\"\\s])?)\\s*(,|$)");
+
+int removedChars = 0;
+while (true) {
+Object ri;
+Matcher m = dq.matcher(input);
+if (m.find()) {
+try {
+ri = unwrapJavaString(m.group(1));
+} catch (Exception e) {
+Exceptions.propagateIfFatal(e);
+return Maybe.absent("Could not match valid quote
pattern" +
+(removedChars>0 ? " at position "+removedC
[GitHub] brooklyn-server pull request #982: Improve coercions esp with generics
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/982#discussion_r214607884
--- Diff:
utils/common/src/main/java/org/apache/brooklyn/util/text/StringEscapes.java ---
@@ -315,12 +320,121 @@ else if (c=='\"') {
throw new IllegalArgumentException("String '"+s+"' is not a
valid Java string (unterminated string)");
}
+/** @deprecated since 1.0.0, use {@link
#unwrapJsonishListStringIfPossible(String)} (old semantics)
+ * or {@link #unwrapJsonishListStringIfPossible(String)}
(improved) */
--- End diff --
It looks like the two methods being suggested (for old semantics and
improved) are identical - what should they be? Should it be pointing folk at
`tryUnwrapJsonishList(String)` as well?
---
[GitHub] brooklyn-server pull request #982: Improve coercions esp with generics
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/982#discussion_r214554995
--- Diff:
core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java ---
@@ -149,19 +153,28 @@ public ConfigConstraints(T brooklynObject) {
return violating;
}
-@SuppressWarnings("unchecked")
boolean isValueValid(ConfigKey configKey, V value) {
+return !checkValueValid(configKey, value).hasError();
+}
+
+/** returns reference to null without error if valid; otherwise
returns reference to predicate and a good error message */
+@SuppressWarnings("unchecked")
+ ReferenceWithError> checkValueValid(ConfigKey
configKey, V value) {
--- End diff --
Naming convention in guava is that `checkXyz` methods will throw an
exception if the check fails (e.g.
https://google.github.io/guava/releases/19.0/api/docs/com/google/common/base/Preconditions.html).
I'd prefer the name `validateValue(...)`.
---
[GitHub] brooklyn-server pull request #982: Improve coercions esp with generics
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/982#discussion_r214608331
--- Diff:
utils/common/src/main/java/org/apache/brooklyn/util/text/StringEscapes.java ---
@@ -315,12 +320,121 @@ else if (c=='\"') {
throw new IllegalArgumentException("String '"+s+"' is not a
valid Java string (unterminated string)");
}
+/** @deprecated since 1.0.0, use {@link
#unwrapJsonishListStringIfPossible(String)} (old semantics)
+ * or {@link #unwrapJsonishListStringIfPossible(String)}
(improved) */
+public static List unwrapJsonishListIfPossible(String
input) {
+return unwrapJsonishListStringIfPossible(input);
+}
+
+/** converts a comma separated list in a single string to a list
of json primitives or maps,
+ * falling back to returning the input.
+ *
+ * specifically:
+ * 1) if of form [ X ] (in brackets after trim),
parse as YAML;
+ * if parse succeeds return the result, or if parse fails,
return {@link Maybe#absent()}.
+ * 2) if not of form [ X ], wrap in brackets and
parse as YAML,
+ * and if that succeeds and is a list, return the result.
+ * 3) otherwise, expect comma-separated tokens which after
trimming are of the form "A" or B,
+ * where "A" is a valid Java string or C is any string not
starting with '
--- End diff --
Could do we rewording. This line introduces `C` - should it be `B`? There
was no mention of C previously.
---
[GitHub] brooklyn-server pull request #982: Improve coercions esp with generics
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/982#discussion_r214606998
--- Diff:
core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java ---
@@ -251,9 +255,23 @@ public ResolverBuilderPretype(Object v) {
else return Maybe.absent("No default value set");
}
-/** causes nested structures (maps, lists) to be descended and nested
unresolved values resolved */
+/** causes nested structures (maps, lists) to be descended and nested
unresolved values resolved.
+ * for legacy reasons this sets deepTraversalUsesRootType.
+ * @deprecated use {@link #deep(boolean, boolean)} */
public ValueResolver deep(boolean forceDeep) {
+return deep(true, true);
+}
+/** causes nested structures (maps, lists) to be descended and nested
unresolved values resolved.
+ * if the second argument is true, the type specified here is used
against non-map/iterable items
+ * inside maps and iterables encountered. if false, any generic
signature on the supplied type
+ * is traversed to match contained items. if null (default), it is
inferred from the type,
+ * those with generics mean true, and those without mean false.
--- End diff --
Is it not the other way round: "those with generics mean false, and those
without mean true"?
---
[GitHub] brooklyn-docs issue #262: some blueprinting tweaks
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-docs/pull/262 LGTM; merging. ---
