You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ahgittin <gi...@git.apache.org> on 2015/08/19 12:07:12 UTC

[GitHub] incubator-brooklyn pull request: YAML pluggable camp

GitHub user ahgittin opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/846

    YAML pluggable camp

    rebases #760 with new package structure. builds on #844 so merge that first!
    (it's just the last commit here)

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

    $ git pull https://github.com/ahgittin/incubator-brooklyn yaml-pluggable-camp

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

    https://github.com/apache/incubator-brooklyn/pull/846.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 #846
    
----
commit bc509036738704419814ce972b52958fb5ace797
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2015-08-18T19:20:05Z

    BROOKLYN-162 - refactoring core, applying org.apache package prefix and grouping more sensibly; also restructuring api package similarly

commit d326759c5508c6767b2780fcb29156150ec0a7e8
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2015-08-18T20:14:34Z

    BROOKLYN-162 - apply org.apache package prefix to policy project (a few outstanding items)

commit 64c2b2e5a31c86b44c08d77012b200595d46728c
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2015-08-18T21:12:56Z

    BROOKLYN-162 - apply org.apache package prefix to software-base, tidying package names, and moving a few sensory things to core

commit a1ad34d718e37bbab7108956a2f3f9959be68000
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2015-08-18T21:38:46Z

    BROOKLYN-162 - jclouds last few package prefixes needed, and tidy in core and elsewhere related (or observed in the process)

commit c45bab04f2efec84ce489e6f505f12cf0bdbcd53
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2015-08-19T00:00:43Z

    BROOKLYN-162 - cleanup and test fixes following refectoring, esp the software/base changes

commit 95277b3abb6f6aad0e8294eabfa93994fc6ddca7
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2015-08-19T09:48:02Z

    updated default catalog with correct package names

commit 1ad11b0fb3edce2c3098aa2076281b38edb40313
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2015-08-19T10:02:12Z

    This closes #760. Makes YAML pluggable.
    
    I've done effectively a rebase-resolve-squash on the commits from that PR. Doing a formal rebase was very hard due to all the conflicts.
    
    This uses a ServiceLoader to get CAMP stuff, with a list of transformers supported.
    
    For the record the commit log of #760, squashed here, is:
    
    > commit f7067c2c10942cfea15ecbe0cb4e8e31e8fb6cde
    > Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
    > Date:   Tue Jul 21 17:25:15 2015 +0300
    >
    >     Use plugins to parse yaml plans
    >
    >     Move all CAMP related code to a CAMP implementation of the plugin.
    >
    > commit c9db2547e1fd0e16b23cb09f04f4cffe6db01ad8
    > Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
    > Date:   Tue Jul 21 15:01:51 2015 +0300
    >
    >     Decrease API surface in CampCatalogUtils
    >
    > commit 4e24d5f93d250dbbbcf579f2a406f63e1c315798
    > Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
    > Date:   Tue Jul 21 13:17:42 2015 +0300
    >
    >     Deduplicate code
    >
    > commit a953e059066aa53323db0ed027366a4b5994ecfc
    > Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
    > Date:   Mon Jul 20 18:55:18 2015 +0300
    >
    >     Move all CAMP-related code to a single place - brooklyn-launcher
    >
    >     Use existing utils methods wrapping CAMP calls - in EntityManagementUtils.
    >
    > commit dc4b37f6c07337171b61ab25beb9bcea617d9452
    > Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
    > Date:   Mon Jul 20 16:36:32 2015 +0300
    >
    >     Move all CAMP-related code to a single place - brooklyn-rest-server
    >
    >     Use existing utils methods wrapping CAMP calls - in EntityManagementUtils.
    >
    > commit 3b8bc66c35ba84db158f2018b967bf501f5811c8
    > Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
    > Date:   Mon Jul 20 16:02:36 2015 +0300
    >
    >     Move all CAMP-related code to a single place - brooklyn-core
    >
    >     EntityManagementUtils and CampCatalogUtils interface to the CAMP parser now.
    >

----


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

[GitHub] incubator-brooklyn pull request: YAML pluggable camp

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

    https://github.com/apache/incubator-brooklyn/pull/846#discussion_r37418622
  
    --- Diff: usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampCatalogUtils.java ---
    @@ -0,0 +1,244 @@
    +/*
    + * 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.camp.brooklyn.spi.creation;
    +
    +import static com.google.common.base.Preconditions.checkNotNull;
    +
    +import java.util.Map;
    +import java.util.Set;
    +
    +import org.apache.brooklyn.api.catalog.CatalogItem;
    +import org.apache.brooklyn.api.catalog.CatalogItem.CatalogItemType;
    +import org.apache.brooklyn.api.entity.EntitySpec;
    +import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec;
    +import org.apache.brooklyn.api.location.Location;
    +import org.apache.brooklyn.api.location.LocationSpec;
    +import org.apache.brooklyn.api.mgmt.ManagementContext;
    +import org.apache.brooklyn.api.mgmt.classloading.BrooklynClassLoadingContext;
    +import org.apache.brooklyn.api.policy.Policy;
    +import org.apache.brooklyn.api.policy.PolicySpec;
    +import org.apache.brooklyn.camp.CampPlatform;
    +import org.apache.brooklyn.camp.brooklyn.BrooklynCampConstants;
    +import org.apache.brooklyn.camp.brooklyn.api.AssemblyTemplateSpecInstantiator;
    +import org.apache.brooklyn.camp.spi.AssemblyTemplate;
    +import org.apache.brooklyn.camp.spi.instantiate.AssemblyTemplateInstantiator;
    +import org.apache.brooklyn.camp.spi.pdp.DeploymentPlan;
    +import org.apache.brooklyn.camp.util.yaml.Yamls;
    +import org.apache.brooklyn.core.catalog.internal.BasicBrooklynCatalog;
    +import org.apache.brooklyn.core.catalog.internal.BasicBrooklynCatalog.BrooklynLoaderTracker;
    +import org.apache.brooklyn.core.catalog.internal.CatalogUtils;
    +import org.apache.brooklyn.core.objs.BrooklynObjectInternal.ConfigurationSupportInternal;
    +import org.apache.brooklyn.util.collections.MutableSet;
    +import org.apache.brooklyn.util.exceptions.Exceptions;
    +import org.apache.brooklyn.util.guava.Maybe;
    +import org.apache.brooklyn.util.stream.Streams;
    +import org.apache.brooklyn.util.text.Strings;
    +
    +import com.google.common.base.Preconditions;
    +import com.google.common.collect.ImmutableMap;
    +import com.google.common.collect.Iterables;
    +
    +public class CampCatalogUtils {
    +
    +    public static AbstractBrooklynObjectSpec<?, ?> createSpec(ManagementContext mgmt, CatalogItem<?, ?> item) {
    --- End diff --
    
    Why put all of this in `CampCatalogUtils`, rather than in `CampToSpecTransformer`?


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

[GitHub] incubator-brooklyn pull request: YAML pluggable camp

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

    https://github.com/apache/incubator-brooklyn/pull/846#discussion_r37418113
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/plan/PlanToSpecTransformer.java ---
    @@ -0,0 +1,35 @@
    +/*
    + * 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.plan;
    +
    +import org.apache.brooklyn.api.catalog.CatalogItem;
    +import org.apache.brooklyn.api.entity.Application;
    +import org.apache.brooklyn.api.entity.EntitySpec;
    +import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec;
    +import org.apache.brooklyn.core.mgmt.ManagementContextInjectable;
    +
    +import com.google.common.annotations.Beta;
    +
    +public interface PlanToSpecTransformer extends ManagementContextInjectable {
    +    String getName();
    +    @Beta
    +    boolean accepts(String mime);
    +    <T extends Application> EntitySpec<T> createApplicationSpec(String plan);
    --- End diff --
    
    I never really like the use of generics like this - it basically allows the caller to avoid a cast by pretending that the method actually does something with the generic type passed in.
    
    But no strong feelings.


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

[GitHub] incubator-brooklyn pull request: YAML pluggable camp

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

    https://github.com/apache/incubator-brooklyn/pull/846#discussion_r37417404
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/plan/PlanToSpecTransformer.java ---
    @@ -0,0 +1,35 @@
    +/*
    + * 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.plan;
    +
    +import org.apache.brooklyn.api.catalog.CatalogItem;
    +import org.apache.brooklyn.api.entity.Application;
    +import org.apache.brooklyn.api.entity.EntitySpec;
    +import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec;
    +import org.apache.brooklyn.core.mgmt.ManagementContextInjectable;
    +
    +import com.google.common.annotations.Beta;
    +
    +public interface PlanToSpecTransformer extends ManagementContextInjectable {
    --- End diff --
    
    Worth adding some javadoc


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

[GitHub] incubator-brooklyn pull request: YAML pluggable camp

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

    https://github.com/apache/incubator-brooklyn/pull/846#issuecomment-132621121
  
    Have addressed, and will push.  Agree with all comments, except:
    
    * generics, I applied this but it doesn't help much as we abandon the generics in the impl; the Transformer interface is not marked `@Beta` as a class so we can experiement; also, we might want to support ZIPs and other items in future
    * use of `CampCatalogUtils` - no strong feelings, this is how @neykov did it originally.  it can move back and forth.  (this project should not really be part of the public API.  when we do OSGi we can make it `export-package: none` due to `ServiceLoader`!)
    
    pushing and merging now


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

[GitHub] incubator-brooklyn pull request: YAML pluggable camp

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

    https://github.com/apache/incubator-brooklyn/pull/846


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

[GitHub] incubator-brooklyn pull request: YAML pluggable camp

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

    https://github.com/apache/incubator-brooklyn/pull/846#discussion_r37417332
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/plan/PlanNotRecognizedException.java ---
    @@ -0,0 +1,40 @@
    +/*
    + * 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.plan;
    +
    +public class PlanNotRecognizedException extends RuntimeException {
    +    private static final long serialVersionUID = 1L;
    --- End diff --
    
    If going to include a serialVersionUID, then might as well let IDE generate a good one?


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

[GitHub] incubator-brooklyn pull request: YAML pluggable camp

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

    https://github.com/apache/incubator-brooklyn/pull/846#discussion_r37417288
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/plan/PlanNotRecognizedException.java ---
    @@ -0,0 +1,40 @@
    +/*
    + * 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.plan;
    +
    +public class PlanNotRecognizedException extends RuntimeException {
    +    private static final long serialVersionUID = 1L;
    +
    +    public PlanNotRecognizedException() {
    --- End diff --
    
    Delete the no-arg constructor.


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

[GitHub] incubator-brooklyn pull request: YAML pluggable camp

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

    https://github.com/apache/incubator-brooklyn/pull/846#discussion_r37418464
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/plan/PlanToSpecTransformer.java ---
    @@ -0,0 +1,35 @@
    +/*
    + * 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.plan;
    +
    +import org.apache.brooklyn.api.catalog.CatalogItem;
    +import org.apache.brooklyn.api.entity.Application;
    +import org.apache.brooklyn.api.entity.EntitySpec;
    +import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec;
    +import org.apache.brooklyn.core.mgmt.ManagementContextInjectable;
    +
    +import com.google.common.annotations.Beta;
    +
    +public interface PlanToSpecTransformer extends ManagementContextInjectable {
    +    String getName();
    +    @Beta
    +    boolean accepts(String mime);
    +    <T extends Application> EntitySpec<T> createApplicationSpec(String plan);
    +    AbstractBrooklynObjectSpec<?, ?> createCatalogSpec(CatalogItem<?, ?> item);
    --- End diff --
    
    Can we use generics here, such as:
    
        <T,SpecT> AbstractBrooklynObjectSpec<T, SpecT> createCatalogSpec(CatalogItem<T, SpecT> item);
    
    Or does it get much more complicated (e.g. needing something horrendous like `<T, SpecT extends AbstractBrooklynObjectSpec<T, SpecT>>`)?


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

[GitHub] incubator-brooklyn pull request: YAML pluggable camp

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

    https://github.com/apache/incubator-brooklyn/pull/846#issuecomment-132549047
  
    Now that #844 is merged this is clean.  @neykov any comments?


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

[GitHub] incubator-brooklyn pull request: YAML pluggable camp

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

    https://github.com/apache/incubator-brooklyn/pull/846#issuecomment-132613581
  
    Looks really good - have taken an initial look at the overall structure. My comments are mostly minor. Happy for you to merge once you've read through my comments.


---
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.
---