You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by tbouron <gi...@git.apache.org> on 2016/11/08 09:23:30 UTC
[GitHub] brooklyn-server pull request #414: Improvement of TestHttpCall entity
GitHub user tbouron opened a pull request:
https://github.com/apache/brooklyn-server/pull/414
Improvement of TestHttpCall entity
This adds the possibility to:
- do GET, POST, PUT, DELETE and OPTIONS requests
- specify headers
- specify request body for POST and PUT requests
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/tbouron/brooklyn-server feature/test-http-call-methods
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/414.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 #414
----
commit bb683b7cd809b5f3a6a94af13d38b11325670ea5
Author: Thomas Bouron <th...@cloudsoftcorp.com>
Date: 2016-11-08T09:20:48Z
TestHttpCall entity can now perform requests other than GET, setup headers and body
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #414: Improvement of TestHttpCall entity
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/414#discussion_r87090436
--- Diff: test-framework/src/main/java/org/apache/brooklyn/test/framework/TestHttpCallImpl.java ---
@@ -109,6 +130,49 @@ public Integer get() {
}
}
+ private HttpRequestBase createHttpMethod(HttpMethod method, String url, Map<String, String> headers, String body) throws Exception {
+ switch (method) {
+ case GET:
+ HttpTool.HttpGetBuilder httpGetBuilder = new HttpTool.HttpGetBuilder(new URI(url));
--- End diff --
I wondered about moving the `HttpMethod` into `HttpTool`. No strong feelings. If we did try to add a generic builder, then it would allow setting a body for all methods, and then would have to validate on `build()`. That's not ideal, but maybe worth it to avoid duplication.
Also, see https://en.wikipedia.org/wiki/Hypertext_Transfer_Protocol#Request_methods. There are other methods we're not supporting (e.g. `TRACE`, `OPTIONS`, etc). Are we right to use an enum with a subset of the methods? I think we are: the enum lists all the methods that are supported. And there is no need to try to support the less common methods until there's a real use-case.
Let's leave it as-is, and think about it some more :-)
But if you feel it would be a lot cleaner to have a generic builder, and/or to move `HttpMethod` into `HttpTool` then go for it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #414: Improvement of TestHttpCall entity
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/414#discussion_r86971541
--- Diff: test-framework/src/main/java/org/apache/brooklyn/test/framework/TestHttpCall.java ---
@@ -32,9 +36,34 @@
@SetFromFlag(nullable = false)
ConfigKey<String> TARGET_URL = ConfigKeys.newStringConfigKey("url", "URL to test");
+ @SetFromFlag(nullable = false)
+ ConfigKey<HttpMethod> TARGET_METHOD = ConfigKeys.builder(HttpMethod.class)
+ .name("method")
+ .description("Method to request the URL: GET, POST, PUT, DELETE, etc")
+ .defaultValue(HttpMethod.GET)
+ .build();
+
+ @SetFromFlag
+ ConfigKey<Map<String, String>> TARGET_HEADERS = ConfigKeys.builder(new TypeToken<Map<String, String>>() {})
--- End diff --
I wonder if we should use type `org.apache.commons.collections.MultiMap`, and add to `CommonAdaptorTypeCoercions.registerStandardAdapters` a coercion from `java.util.Map` to `MultiMap`.
But probably not worth it, given that yaml won't support a map with duplicate keys anyway!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #414: Improvement of TestHttpCall entity
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/414#discussion_r86971858
--- Diff: test-framework/src/main/java/org/apache/brooklyn/test/framework/TestHttpCallImpl.java ---
@@ -50,19 +55,25 @@
*/
public void start(Collection<? extends Location> locations) {
String url = null;
+ HttpMethod method = null;
--- End diff --
Minor; feel free to ignore :-)
We only declare `url` above because the variable is needed in the catch block for a nicer error message. I'd declare these variables in-line, rather than outside of the try block, unless we also want to log any of them in the catch error message.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #414: Improvement of TestHttpCall entity
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/414
@tbouron can you add tests to `org.apache.brooklyn.test.framework.TestHttpCallTest`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #414: Improvement of TestHttpCall entity
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/414
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #414: Improvement of TestHttpCall entity
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/414#discussion_r87090913
--- Diff: test-framework/src/test/java/org/apache/brooklyn/test/framework/TestHttpCallTest.java ---
@@ -87,6 +96,11 @@ public void testHttpBodyAssertions() {
.configure(TestHttpCall.TARGET_URL, server.getUrl() + "/body.json")
.configure(TestHttpCall.TIMEOUT, new Duration(10L, TimeUnit.SECONDS))
.configure(TestSensor.ASSERTIONS, newAssertion("matches", ".*123.*")));
+ app.createAndManageChild(EntitySpec.create(TestHttpCall.class)
+ .configure(TestHttpCall.TARGET_URL, server.getUrl() + "/foo/bar")
+ .configure(TestHttpCall.TARGET_METHOD, TestHttpCall.HttpMethod.POST)
+ .configure(TestHttpCall.TIMEOUT, new Duration(10L, TimeUnit.SECONDS))
--- End diff --
Personal preference: we should use `Asserts.DEFAULT_LONG_TIMEOUT` rather than our code being littered with arbitrary timeouts. In my opinion, the only time one should set different timeouts for something we expect to not timeout is if it's a performance test.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #414: Improvement of TestHttpCall entity
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/414#discussion_r87155408
--- Diff: test-framework/src/main/java/org/apache/brooklyn/test/framework/TestHttpCall.java ---
@@ -32,9 +36,34 @@
@SetFromFlag(nullable = false)
ConfigKey<String> TARGET_URL = ConfigKeys.newStringConfigKey("url", "URL to test");
+ @SetFromFlag(nullable = false)
+ ConfigKey<HttpMethod> TARGET_METHOD = ConfigKeys.builder(HttpMethod.class)
+ .name("method")
+ .description("Method to request the URL: GET, POST, PUT, DELETE, etc")
+ .defaultValue(HttpMethod.GET)
+ .build();
+
+ @SetFromFlag
--- End diff --
Fair enough, I'll remove the unnecessary ones
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #414: Improvement of TestHttpCall entity
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/414#discussion_r86976014
--- Diff: test-framework/src/main/java/org/apache/brooklyn/test/framework/TestHttpCallImpl.java ---
@@ -94,7 +110,12 @@ public String get() {
@Override
public Integer get() {
try {
- return HttpTool.getHttpStatusCode(url);
+ final Maybe<HttpResponse> response = HttpTool.execAndConsume(HttpTool.httpClientBuilder().build(), createHttpMethod(method, url, headers, body)).getResponse();
+ if (response.isPresentAndNonNull()) {
+ return response.get().getStatusLine().getStatusCode();
+ } else {
+ throw new Exception("HTTP call did not return any reponse");
--- End diff --
Addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #414: Improvement of TestHttpCall entity
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/414#discussion_r86974177
--- Diff: test-framework/src/main/java/org/apache/brooklyn/test/framework/TestHttpCallImpl.java ---
@@ -109,6 +130,49 @@ public Integer get() {
}
}
+ private HttpRequestBase createHttpMethod(HttpMethod method, String url, Map<String, String> headers, String body) throws Exception {
+ switch (method) {
+ case GET:
+ HttpTool.HttpGetBuilder httpGetBuilder = new HttpTool.HttpGetBuilder(new URI(url));
--- End diff --
Agree but the the `HttpTool.HttpRequestBuilder` is based on the request class which is why there is this conversion enum -> request. Do you think the `HttpMethod` and this private method `createHttpMethod` should move into `HttpTool`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #414: Improvement of TestHttpCall entity
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/414#discussion_r86972321
--- Diff: test-framework/src/main/java/org/apache/brooklyn/test/framework/TestHttpCall.java ---
@@ -32,9 +36,34 @@
@SetFromFlag(nullable = false)
ConfigKey<String> TARGET_URL = ConfigKeys.newStringConfigKey("url", "URL to test");
+ @SetFromFlag(nullable = false)
+ ConfigKey<HttpMethod> TARGET_METHOD = ConfigKeys.builder(HttpMethod.class)
+ .name("method")
+ .description("Method to request the URL: GET, POST, PUT, DELETE, etc")
+ .defaultValue(HttpMethod.GET)
+ .build();
+
+ @SetFromFlag
+ ConfigKey<Map<String, String>> TARGET_HEADERS = ConfigKeys.builder(new TypeToken<Map<String, String>>() {})
--- End diff --
The `TestHttpCall` is meant to be used in YAML therefore I don't think it's worth to change this to a `MultiMap` (as not supported)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #414: Improvement of TestHttpCall entity
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/414#discussion_r86972723
--- Diff: test-framework/src/main/java/org/apache/brooklyn/test/framework/TestHttpCallImpl.java ---
@@ -50,19 +55,25 @@
*/
public void start(Collection<? extends Location> locations) {
String url = null;
+ HttpMethod method = null;
--- End diff --
Good catch, I'll update
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #414: Improvement of TestHttpCall entity
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/414#discussion_r86972174
--- Diff: test-framework/src/main/java/org/apache/brooklyn/test/framework/TestHttpCallImpl.java ---
@@ -94,7 +110,12 @@ public String get() {
@Override
public Integer get() {
try {
- return HttpTool.getHttpStatusCode(url);
+ final Maybe<HttpResponse> response = HttpTool.execAndConsume(HttpTool.httpClientBuilder().build(), createHttpMethod(method, url, headers, body)).getResponse();
+ if (response.isPresentAndNonNull()) {
+ return response.get().getStatusLine().getStatusCode();
+ } else {
+ throw new Exception("HTTP call did not return any reponse");
--- End diff --
Type: `reponse`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #414: Improvement of TestHttpCall entity
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/414#discussion_r87088045
--- Diff: test-framework/src/main/java/org/apache/brooklyn/test/framework/TestHttpCall.java ---
@@ -32,9 +36,34 @@
@SetFromFlag(nullable = false)
ConfigKey<String> TARGET_URL = ConfigKeys.newStringConfigKey("url", "URL to test");
+ @SetFromFlag(nullable = false)
+ ConfigKey<HttpMethod> TARGET_METHOD = ConfigKeys.builder(HttpMethod.class)
+ .name("method")
+ .description("Method to request the URL: GET, POST, PUT, DELETE, etc")
+ .defaultValue(HttpMethod.GET)
+ .build();
+
+ @SetFromFlag
--- End diff --
I believe you would still be able to set it outside of the `brooklyn.config:` block in yaml. See https://github.com/apache/brooklyn-server/pull/418 for a new test that demonstrates this.
We should get rid of all the `@SetFromFlag`s, where it's not adding additional info (e.g. to say non-null or such like).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #414: Improvement of TestHttpCall entity
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/414#discussion_r86972775
--- Diff: test-framework/src/main/java/org/apache/brooklyn/test/framework/TestHttpCallImpl.java ---
@@ -109,6 +130,49 @@ public Integer get() {
}
}
+ private HttpRequestBase createHttpMethod(HttpMethod method, String url, Map<String, String> headers, String body) throws Exception {
+ switch (method) {
+ case GET:
+ HttpTool.HttpGetBuilder httpGetBuilder = new HttpTool.HttpGetBuilder(new URI(url));
--- End diff --
This is a code-smell that we need to do lots of identical code for each case, rather than being able to use the `HttpTool` in a simpler way (e.g. passing in the method to a generic request builder, perhaps).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #414: Improvement of TestHttpCall entity
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/414
@aledsage Comment addressed. I created a generic builder for the `HttpRequest`. Hope this is fine now and can be merged
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #414: Improvement of TestHttpCall entity
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/414#discussion_r86971996
--- Diff: test-framework/src/main/java/org/apache/brooklyn/test/framework/TestHttpCallImpl.java ---
@@ -77,13 +88,18 @@ public void start(Collection<? extends Location> locations) {
}
private void doRequestAndCheckAssertions(Map<String, Duration> flags, List<Map<String, Object>> assertions,
- HttpAssertionTarget target, final String url) {
+ HttpAssertionTarget target, final HttpMethod method, final String url, final Map<String, String> headers, final String body) {
switch (target) {
case body:
Supplier<String> getBody = new Supplier<String>() {
@Override
public String get() {
- return HttpTool.getContent(url);
+ try {
+ return HttpTool.execAndConsume(HttpTool.httpClientBuilder().build(), createHttpMethod(method, url, headers, body)).getContentAsString();
--- End diff --
Minor: a couple of very long lines here. Probably worth splitting them up for readability (particularly as there is another method call at the end of this one, which can easily be missed if someone doesn't scroll).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #414: Improvement of TestHttpCall entity
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/414
LGTM; merging.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #414: Improvement of TestHttpCall entity
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/414#discussion_r87178356
--- Diff: test-framework/src/test/java/org/apache/brooklyn/test/framework/TestHttpCallTest.java ---
@@ -107,6 +121,47 @@ public void testHttpStatusAssertions() {
.configure(TestHttpCall.TIMEOUT, new Duration(10L, TimeUnit.SECONDS))
.configure(TestHttpCall.ASSERTION_TARGET, TestHttpCall.HttpAssertionTarget.status)
.configure(TestSensor.ASSERTIONS, newAssertion("matches", "2[0-9][0-9]")));
+ app.createAndManageChild(EntitySpec.create(TestHttpCall.class)
+ .configure(TestHttpCall.TARGET_URL, server.getUrl() + "/foo/bar")
+ .configure(TestHttpCall.TARGET_METHOD, TestHttpCall.HttpMethod.POST)
+ .configure(TestHttpCall.TIMEOUT, new Duration(10L, TimeUnit.SECONDS))
+ .configure(TestHttpCall.ASSERTION_TARGET, TestHttpCall.HttpAssertionTarget.status)
+ .configure(TestSensor.ASSERTIONS, newAssertion("isEqualTo", HttpStatus.SC_CREATED)));
+ app.start(ImmutableList.of(loc));
+ }
+
+ @Test(groups = "Integration")
+ public void testHttpMethodAssertions() {
+ app.createAndManageChild(EntitySpec.create(TestHttpCall.class)
+ .configure(TestHttpCall.TARGET_URL, server.getUrl() + "/foo/bar")
+ .configure(TestHttpCall.TARGET_METHOD, TestHttpCall.HttpMethod.GET)
+ .configure(TestHttpCall.TIMEOUT, new Duration(10L, TimeUnit.SECONDS))
+ .configure(TestHttpCall.ASSERTION_TARGET, TestHttpCall.HttpAssertionTarget.status)
+ .configure(TestSensor.ASSERTIONS, newAssertion("isEqualTo", HttpStatus.SC_METHOD_NOT_ALLOWED)));
+ app.createAndManageChild(EntitySpec.create(TestHttpCall.class)
+ .configure(TestHttpCall.TARGET_URL, server.getUrl() + "/foo/bar")
+ .configure(TestHttpCall.TARGET_METHOD, TestHttpCall.HttpMethod.POST)
+ .configure(TestHttpCall.TIMEOUT, new Duration(10L, TimeUnit.SECONDS))
+ .configure(TestHttpCall.ASSERTION_TARGET, TestHttpCall.HttpAssertionTarget.status)
+ .configure(TestSensor.ASSERTIONS, newAssertion("isEqualTo", HttpStatus.SC_CREATED)));
+ app.createAndManageChild(EntitySpec.create(TestHttpCall.class)
+ .configure(TestHttpCall.TARGET_URL, server.getUrl() + "/foo/bar")
+ .configure(TestHttpCall.TARGET_METHOD, TestHttpCall.HttpMethod.PUT)
+ .configure(TestHttpCall.TIMEOUT, new Duration(10L, TimeUnit.SECONDS))
+ .configure(TestHttpCall.ASSERTION_TARGET, TestHttpCall.HttpAssertionTarget.status)
+ .configure(TestSensor.ASSERTIONS, newAssertion("isEqualTo", HttpStatus.SC_METHOD_NOT_ALLOWED)));
--- End diff --
It does implicitly. The server will return a 405 if the method is different than `PUT`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #414: Improvement of TestHttpCall entity
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/414#discussion_r86971624
--- Diff: test-framework/src/main/java/org/apache/brooklyn/test/framework/TestHttpCall.java ---
@@ -32,9 +36,34 @@
@SetFromFlag(nullable = false)
ConfigKey<String> TARGET_URL = ConfigKeys.newStringConfigKey("url", "URL to test");
+ @SetFromFlag(nullable = false)
+ ConfigKey<HttpMethod> TARGET_METHOD = ConfigKeys.builder(HttpMethod.class)
+ .name("method")
+ .description("Method to request the URL: GET, POST, PUT, DELETE, etc")
+ .defaultValue(HttpMethod.GET)
+ .build();
+
+ @SetFromFlag
--- End diff --
Remove the `@SetFromFlag` - we're just relying on the config key name, which is the right thing to do!
Same below for TARGET_BODY.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #414: Improvement of TestHttpCall entity
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/414#discussion_r86972493
--- Diff: test-framework/src/main/java/org/apache/brooklyn/test/framework/TestHttpCall.java ---
@@ -32,9 +36,34 @@
@SetFromFlag(nullable = false)
ConfigKey<String> TARGET_URL = ConfigKeys.newStringConfigKey("url", "URL to test");
+ @SetFromFlag(nullable = false)
+ ConfigKey<HttpMethod> TARGET_METHOD = ConfigKeys.builder(HttpMethod.class)
+ .name("method")
+ .description("Method to request the URL: GET, POST, PUT, DELETE, etc")
+ .defaultValue(HttpMethod.GET)
+ .build();
+
+ @SetFromFlag
--- End diff --
But if I remove the `@SetFromFlag`, we won't be able to set this config key outside the `brooklyn.config` block, isn't it @aledsage ?
I also keep them as it was the "standard" across the test entities
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #414: Improvement of TestHttpCall entity
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/414
@tbouron this now has merge conflicts. Can you resolve those, take a look at the additional minor comments to see what you think should be done, and then we can merge this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---