You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by andreaturli <gi...@git.apache.org> on 2017/01/20 11:18:58 UTC

[GitHub] brooklyn-server pull request #531: initial work to support HttpEntity

GitHub user andreaturli opened a pull request:

    https://github.com/apache/brooklyn-server/pull/531

    initial work to support HttpEntity

    - add HttpCommnadEffector
    - add CompositeEffector
    - add EntityInitializers util class to resolve DSL injected as params
      into the HttpCommandEffector
    
    ---
    
    It is based on https://github.com/apache/brooklyn-server/pull/152 which was not merged as it didn't handle rebind properly, happy to sort it out in this PR but I've not touched rebinding yet.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/andreaturli/brooklyn-server feature/http-command-effector

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/brooklyn-server/pull/531.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 #531
    
----
commit 025b222c0d914a6e9c111f1c7f000749c524cfbd
Author: Andrea Turli <an...@gmail.com>
Date:   2016-12-21T11:03:27Z

    initial work to support HttpEntity
    
    - add HttpCommnadEffector
    - add CompositeEffector
    - add EntityInitializers util class to resolve DSL injected as params
      into the HttpCommandEffector

----


---
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 #531: initial work to support HttpEntity

Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/531#discussion_r98395700
  
    --- Diff: core/src/test/java/org/apache/brooklyn/core/effector/http/HttpCommandEffectorIntegrationTest.java ---
    @@ -0,0 +1,125 @@
    +/*
    + * 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.effector.http;
    +
    +import org.apache.brooklyn.api.effector.Effector;
    +import org.apache.brooklyn.api.entity.EntityLocal;
    +import org.apache.brooklyn.api.entity.EntitySpec;
    +import org.apache.brooklyn.api.location.Location;
    +import org.apache.brooklyn.core.effector.Effectors;
    +import org.apache.brooklyn.core.entity.Entities;
    +import org.apache.brooklyn.core.sensor.Sensors;
    +import org.apache.brooklyn.core.test.entity.TestApplication;
    +import org.apache.brooklyn.core.test.entity.TestEntity;
    +import org.apache.brooklyn.util.collections.MutableMap;
    +import org.apache.brooklyn.util.core.config.ConfigBag;
    +import org.testng.Assert;
    +import org.testng.annotations.AfterMethod;
    +import org.testng.annotations.BeforeMethod;
    +import org.testng.annotations.Test;
    +
    +import com.google.common.collect.ImmutableList;
    +import com.google.common.collect.ImmutableMap;
    +import com.jayway.jsonpath.JsonPath;
    +
    +public class HttpCommandEffectorIntegrationTest {
    +
    +    final static Effector<String> EFFECTOR_GITHUB_APACHE_ACCOUNT = Effectors.effector(String.class, "GithubApacheAccount").buildAbstract();
    +
    +    private TestApplication app;
    +    private EntityLocal entity;
    +    
    +    @BeforeMethod(alwaysRun=true)
    +    public void setUp() throws Exception {
    +        app = TestApplication.Factory.newManagedInstanceForTests();
    +        entity = app.createAndManageChild(EntitySpec.create(TestEntity.class).location(TestApplication.LOCALHOST_MACHINE_SPEC));
    +        app.start(ImmutableList.<Location>of());
    +    }
    +
    +    @AfterMethod(alwaysRun=true)
    +    public void tearDown() throws Exception {
    +        if (app != null) Entities.destroyAll(app.getManagementContext());
    +    }
    +
    +    @Test(groups="Integration")
    +    public void testHttpEffector() throws Exception {
    +        new HttpCommandEffector(ConfigBag.newInstance()
    +                .configure(HttpCommandEffector.EFFECTOR_NAME, "GithubApacheAccount")
    +                .configure(HttpCommandEffector.EFFECTOR_URI, "https://api.github.com/users/apache")
    --- End diff --
    
    thanks @aledsage, much better, I'll use 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 #531: initial work to support HttpEntity

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/531#discussion_r98362129
  
    --- Diff: core/src/test/java/org/apache/brooklyn/core/effector/http/HttpCommandEffectorIntegrationTest.java ---
    @@ -0,0 +1,125 @@
    +/*
    + * 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.effector.http;
    +
    +import org.apache.brooklyn.api.effector.Effector;
    +import org.apache.brooklyn.api.entity.EntityLocal;
    +import org.apache.brooklyn.api.entity.EntitySpec;
    +import org.apache.brooklyn.api.location.Location;
    +import org.apache.brooklyn.core.effector.Effectors;
    +import org.apache.brooklyn.core.entity.Entities;
    +import org.apache.brooklyn.core.sensor.Sensors;
    +import org.apache.brooklyn.core.test.entity.TestApplication;
    +import org.apache.brooklyn.core.test.entity.TestEntity;
    +import org.apache.brooklyn.util.collections.MutableMap;
    +import org.apache.brooklyn.util.core.config.ConfigBag;
    +import org.testng.Assert;
    +import org.testng.annotations.AfterMethod;
    +import org.testng.annotations.BeforeMethod;
    +import org.testng.annotations.Test;
    +
    +import com.google.common.collect.ImmutableList;
    +import com.google.common.collect.ImmutableMap;
    +import com.jayway.jsonpath.JsonPath;
    +
    +public class HttpCommandEffectorIntegrationTest {
    +
    +    final static Effector<String> EFFECTOR_GITHUB_APACHE_ACCOUNT = Effectors.effector(String.class, "GithubApacheAccount").buildAbstract();
    +
    +    private TestApplication app;
    +    private EntityLocal entity;
    +    
    +    @BeforeMethod(alwaysRun=true)
    +    public void setUp() throws Exception {
    +        app = TestApplication.Factory.newManagedInstanceForTests();
    +        entity = app.createAndManageChild(EntitySpec.create(TestEntity.class).location(TestApplication.LOCALHOST_MACHINE_SPEC));
    +        app.start(ImmutableList.<Location>of());
    +    }
    +
    +    @AfterMethod(alwaysRun=true)
    +    public void tearDown() throws Exception {
    +        if (app != null) Entities.destroyAll(app.getManagementContext());
    +    }
    +
    +    @Test(groups="Integration")
    +    public void testHttpEffector() throws Exception {
    +        new HttpCommandEffector(ConfigBag.newInstance()
    +                .configure(HttpCommandEffector.EFFECTOR_NAME, "GithubApacheAccount")
    +                .configure(HttpCommandEffector.EFFECTOR_URI, "https://api.github.com/users/apache")
    --- End diff --
    
    I'd recommend http://httpbin.org/ for integration testing - it gives very predictable responses based on the inputs, allowing you to test a wide variety of things such as request/response headers, get/put/post/delete etc.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server pull request #531: initial work to support HttpEntity

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/brooklyn-server/pull/531


---
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 #531: initial work to support HttpEntity

Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on the issue:

    https://github.com/apache/brooklyn-server/pull/531
  
    rebuild please


---
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 #531: initial work to support HttpEntity

Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli closed the pull request at:

    https://github.com/apache/brooklyn-server/pull/531


---
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 #531: initial work to support HttpEntity

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:

    https://github.com/apache/brooklyn-server/pull/531
  
    It would be good to also have some yaml-based tests (e.g. see https://github.com/apache/brooklyn-server/blob/master/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/SshCommandEffectorYamlTest.java)


---
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 #531: initial work to support HttpEntity

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:

    https://github.com/apache/brooklyn-server/pull/531
  
    And rebind tests would be very good (e.g. see https://github.com/apache/brooklyn-server/blob/master/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlRebindTest.java and its sub-classes - the important thing is to call `rebind()` in the test method after creating your entities, and then doing some basic testing that the entity is still functional).


---
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 #531: initial work to support HttpEntity

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:

    https://github.com/apache/brooklyn-server/pull/531
  
    very nice @andreaturli -- and excellent tests.
    
    is there a corresponding PR against the docs to explain how to use this?
    
    looks good so 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 #531: initial work to support HttpEntity

Posted by andreaturli <gi...@git.apache.org>.
GitHub user andreaturli reopened a pull request:

    https://github.com/apache/brooklyn-server/pull/531

    initial work to support HttpEntity

    - add HttpCommnadEffector
    - add CompositeEffector
    - add EntityInitializers util class to resolve DSL injected as params
      into the HttpCommandEffector
    
    ---
    
    It is based on https://github.com/apache/brooklyn-server/pull/152 which was not merged as it didn't handle rebind properly, happy to sort it out in this PR but I've not touched rebinding yet.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/andreaturli/brooklyn-server feature/http-command-effector

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/brooklyn-server/pull/531.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 #531
    
----
commit eadec9ac9ca82ab1b5baea8c69b6ab35b55c452b
Author: Andrea Turli <an...@gmail.com>
Date:   2016-12-21T11:03:27Z

    initial work to support HttpEntity
    
    - add HttpCommnadEffector
    - add CompositeEffector
    - add EntityInitializers util class to resolve DSL injected as params
      into the HttpCommandEffector

commit 119ba5a47b4a5198ae99927511e35107d2ebce89
Author: Andrea Turli <an...@gmail.com>
Date:   2017-01-23T16:12:14Z

    fix CompositeEffector logic

commit 3d318e4e78085519233bcc763e22320ca40403df
Author: Andrea Turli <an...@gmail.com>
Date:   2017-01-23T16:24:31Z

    remove default value for JSON_PATH
    
    - some http calls may not require to parse the output

commit b0b83da785944b3929866a89470b52cd8215684f
Author: Andrea Turli <an...@gmail.com>
Date:   2017-01-31T15:51:05Z

    add more tests for HttpCommandEffector
    
    - add unit test (HttpCommandEffectorTest)
    - add YamlTest (HttpCommandEffectorYamlTest)
    - add YamlRebindTest (HttpCommandEffectorYamlRebindTest)

commit b32581dd81a390d69ff63779465cd83520c7dcab
Author: Andrea Turli <an...@gmail.com>
Date:   2017-02-02T16:27:41Z

    add CompositeEffector tests and improve its implementation

commit 8809d5cdde60f65636fafdce2917ea37e550f1e1
Author: Andrea Turli <an...@gmail.com>
Date:   2017-02-02T16:34:09Z

    add CompositeEffectorYamlRebindTest

commit 98edec705ce08b7fbca8e90759edde995f71c95b
Author: Andrea Turli <an...@gmail.com>
Date:   2017-02-13T14:39:39Z

    fix merge errors

----


---
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 #531: initial work to support HttpEntity

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/531#discussion_r98362353
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/effector/CompositeEffector.java ---
    @@ -0,0 +1,133 @@
    +/*
    + * 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.effector;
    +
    +import java.util.List;
    +import java.util.Map;
    +
    +import javax.annotation.Nullable;
    +
    +import org.apache.brooklyn.api.effector.Effector;
    +import org.apache.brooklyn.api.entity.EntityLocal;
    +import org.apache.brooklyn.config.ConfigKey;
    +import org.apache.brooklyn.core.config.ConfigKeys;
    +import org.apache.brooklyn.core.effector.Effectors.EffectorBuilder;
    +import org.apache.brooklyn.core.entity.EntityInitializers;
    +import org.apache.brooklyn.core.entity.EntityInternal;
    +import org.apache.brooklyn.util.core.config.ConfigBag;
    +import org.apache.brooklyn.util.guava.Maybe;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import com.google.common.annotations.Beta;
    +import com.google.common.base.Optional;
    +import com.google.common.base.Preconditions;
    +import com.google.common.base.Predicate;
    +import com.google.common.collect.ImmutableList;
    +import com.google.common.collect.Iterables;
    +import com.google.common.collect.Lists;
    +import com.google.common.reflect.TypeToken;
    +
    +@Beta
    +public class CompositeEffector extends AddEffector {
    +
    +    private static final Logger LOG = LoggerFactory.getLogger(CompositeEffector.class);
    +
    +    public static final ConfigKey<List<String>> EFFECTORS = ConfigKeys.newConfigKey(new TypeToken<List<String>>() {}, "effectors",
    --- End diff --
    
    I see we pass in the same parameters to every effector. I wonder whether we should also accept a list of maps, so that we could define the parameters to the individual effectors. But that would make the code more complicated.
    
    It would be interesting to see what the yaml would look like for that.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server issue #531: initial work to support HttpEntity

Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on the issue:

    https://github.com/apache/brooklyn-server/pull/531
  
    @aledsage, @grkvlt can you have another look at this PR, please?


---
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 #531: initial work to support HttpEntity

Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on the issue:

    https://github.com/apache/brooklyn-server/pull/531
  
    I've added more tests for the effectors:
    
    - add unit tests: HttpCommandEffectorTest and CompositeEffectorTest
    - add YamlTests: HttpCommandEffectorYamlTest and CompositeEffectorYamlTest.java
    - add YamlRebindTests: HttpCommandEffectorYamlRebindTest and CompositeEffectorYamlRebindTest
    
    ---
    
    please @aledsage can you have another look?



---
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 #531: initial work to support HttpEntity

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/531#discussion_r98362160
  
    --- Diff: core/src/test/java/org/apache/brooklyn/core/effector/http/HttpCommandEffectorIntegrationTest.java ---
    @@ -0,0 +1,125 @@
    +/*
    + * 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.effector.http;
    +
    +import org.apache.brooklyn.api.effector.Effector;
    +import org.apache.brooklyn.api.entity.EntityLocal;
    +import org.apache.brooklyn.api.entity.EntitySpec;
    +import org.apache.brooklyn.api.location.Location;
    +import org.apache.brooklyn.core.effector.Effectors;
    +import org.apache.brooklyn.core.entity.Entities;
    +import org.apache.brooklyn.core.sensor.Sensors;
    +import org.apache.brooklyn.core.test.entity.TestApplication;
    +import org.apache.brooklyn.core.test.entity.TestEntity;
    +import org.apache.brooklyn.util.collections.MutableMap;
    +import org.apache.brooklyn.util.core.config.ConfigBag;
    +import org.testng.Assert;
    +import org.testng.annotations.AfterMethod;
    +import org.testng.annotations.BeforeMethod;
    +import org.testng.annotations.Test;
    +
    +import com.google.common.collect.ImmutableList;
    +import com.google.common.collect.ImmutableMap;
    +import com.jayway.jsonpath.JsonPath;
    +
    +public class HttpCommandEffectorIntegrationTest {
    +
    +    final static Effector<String> EFFECTOR_GITHUB_APACHE_ACCOUNT = Effectors.effector(String.class, "GithubApacheAccount").buildAbstract();
    +
    +    private TestApplication app;
    +    private EntityLocal entity;
    +    
    +    @BeforeMethod(alwaysRun=true)
    +    public void setUp() throws Exception {
    --- End diff --
    
    You can write a unit test (rather than just an integration test) if you use something similar to https://github.com/apache/brooklyn-server/blob/master/core/src/test/java/org/apache/brooklyn/feed/http/HttpFeedTest.java


---
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 #531: initial work to support HttpEntity

Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on the issue:

    https://github.com/apache/brooklyn-server/pull/531
  
    I will open a PR for docs ASAP, thanks @ahgittin for merging 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.
---