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 2017/01/05 15:58:33 UTC
[GitHub] brooklyn-dist pull request #75: [WIP] Feature/brooklyn server config
GitHub user tbouron opened a pull request:
https://github.com/apache/brooklyn-dist/pull/75
[WIP] Feature/brooklyn server config
This uses https://github.com/apache/brooklyn-server/pull/504 to get the brooklyn server configuration files.
This is a follow up of https://github.com/apache/brooklyn-dist/pull/72
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/tbouron/brooklyn-dist feature/brooklyn-server-config
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-dist/pull/75.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 #75
----
commit 0921b9a5beb37f6dec9249cf09f488aea21fc8ee
Author: Thomas Bouron <th...@cloudsoftcorp.com>
Date: 2016-12-20T18:03:32Z
WIP
commit 50dff2acf4765302ee6500c4cdcbc288ae915f97
Author: Thomas Bouron <th...@cloudsoftcorp.com>
Date: 2016-12-22T10:16:25Z
Exclude karaf configuration key from properties
commit dede125a7548f12a62e584d9ce332fc8cd056612
Author: Thomas Bouron <th...@cloudsoftcorp.com>
Date: 2017-01-05T15:54:32Z
Make the brooklyn server config discoverable by OSGi and downstream projects
----
---
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-dist issue #75: Use new brooklyn-server-config feature
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-dist/pull/75
@m4rkmckenna @neykov as per the comment https://github.com/apache/brooklyn-server/pull/504#issuecomment-271535835, I'd appreciate your comments as to whether this looks right to you 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] brooklyn-dist pull request #75: Use new brooklyn-server-config feature
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-dist/pull/75#discussion_r95331050
--- Diff: karaf/features/src/main/feature/feature.xml ---
@@ -55,7 +55,12 @@
<bundle>mvn:javax.annotation/javax.annotation-api/${cxf.javax.annotation-api.version}</bundle>
</feature>
+ <feature name="brooklyn-config" version="${project.version}">
+ <feature>brooklyn-server-config</feature>
+ </feature>
+
<feature name="brooklyn-headless" version="${project.version}" description="All Brooklyn bundles witht the exception of the launcher">
+ <feature prerequisite="true">brooklyn-config</feature>
--- End diff --
Gut feel is that we should move this line to after the feature `brooklyn-guava-optional-deps`. I think that the "prerequisite" features are resolved/loaded in-order. We presumably want all of the standard karaf stuff in place before we try installing/starting the `brooklyn-server-config` feature.
---
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-dist issue #75: Use new brooklyn-server-config feature
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-dist/pull/75
@neykov @m4rkmckenna @aledsage I put back the files within `brooklyn-dist` and import them directly into the `brooklyn-config` feature (also closed https://github.com/apache/brooklyn-server/issues/504 as it's not required anymore)
Should be good to go 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] brooklyn-dist issue #75: Use new brooklyn-server-config feature
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-dist/pull/75
Jenkins is failing with
```
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-dependency-plugin:2.8:copy (unpack) on project apache-brooklyn: Unable to find artifact. Could not find artifact org.apache.brooklyn:brooklyn-server-config:cfg:classrename:0.11.0-SNAPSHOT in jclouds-snapshots (https://repository.apache.org/content/repositories/snapshots)
```
Is this expected?
---
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-dist issue #75: Use new brooklyn-server-config feature
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-dist/pull/75
I should mention that the new version extract files during the maven build
---
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-dist pull request #75: Use new brooklyn-server-config feature
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-dist/pull/75#discussion_r95332681
--- Diff: karaf/features/src/main/feature/feature.xml ---
@@ -55,7 +55,12 @@
<bundle>mvn:javax.annotation/javax.annotation-api/${cxf.javax.annotation-api.version}</bundle>
</feature>
+ <feature name="brooklyn-config" version="${project.version}">
+ <feature>brooklyn-server-config</feature>
+ </feature>
+
<feature name="brooklyn-headless" version="${project.version}" description="All Brooklyn bundles witht the exception of the launcher">
+ <feature prerequisite="true">brooklyn-config</feature>
--- End diff --
It does not matter here as it will self extract the files on first boot. As in, the configuration files will be installed before everything else which is what we want.
---
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-dist pull request #75: Use new brooklyn-server-config feature
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-dist/pull/75#discussion_r95332454
--- Diff: karaf/features/src/main/feature/feature.xml ---
@@ -55,7 +55,12 @@
<bundle>mvn:javax.annotation/javax.annotation-api/${cxf.javax.annotation-api.version}</bundle>
</feature>
+ <feature name="brooklyn-config" version="${project.version}">
--- End diff --
We could but I found it cleaner like this. Also, we may have more than one configuration features like a `brooklyn-dist-config`, `jetty-config`, etc.
With this structure, we can just add it to `brooklyn-config`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-dist pull request #75: Use new brooklyn-server-config feature
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-dist/pull/75
---
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-dist issue #75: Use new brooklyn-server-config feature
Posted by m4rkmckenna <gi...@git.apache.org>.
Github user m4rkmckenna commented on the issue:
https://github.com/apache/brooklyn-dist/pull/75
@drigodwin the classic launcher (to my knowledge) doesn't use any of this config
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-dist issue #75: Use new brooklyn-server-config feature
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-dist/pull/75
@neykov Comment 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-dist issue #75: Use new brooklyn-server-config feature
Posted by m4rkmckenna <gi...@git.apache.org>.
Github user m4rkmckenna commented on the issue:
https://github.com/apache/brooklyn-dist/pull/75
I think we need to have these files in place before boot for discoverability
I would go with the `maven-dependency-plugin` to add them to the etc directory at the correct phase
@neykov thoughts?
---
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-dist pull request #75: Use new brooklyn-server-config feature
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-dist/pull/75#discussion_r95624101
--- Diff: karaf/config/pom.xml ---
@@ -0,0 +1,62 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <parent>
+ <artifactId>brooklyn-dist-karaf</artifactId>
+ <groupId>org.apache.brooklyn</groupId>
+ <version>0.11.0-SNAPSHOT</version>
+ </parent>
+ <modelVersion>4.0.0</modelVersion>
+
+ <artifactId>brooklyn-server-config</artifactId>
--- End diff --
Don't disagree that this is brooklyn-server configuration, but for convention would be better to name it something like `brooklyn-dist-config`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-dist issue #75: Use new brooklyn-server-config feature
Posted by drigodwin <gi...@git.apache.org>.
Github user drigodwin commented on the issue:
https://github.com/apache/brooklyn-dist/pull/75
I don't think we can merge this as it will break the classic launch Brooklyn.
---
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-dist issue #75: Use new brooklyn-server-config feature
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-dist/pull/75
@neykov Urgh, forgot to update one `pom.xml`, should be fine 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] brooklyn-dist issue #75: Use new brooklyn-server-config feature
Posted by m4rkmckenna <gi...@git.apache.org>.
Github user m4rkmckenna commented on the issue:
https://github.com/apache/brooklyn-dist/pull/75
@aledsage This is the correct way to go IMO.
**BUT** before this can be merged I would want the config files put in place at compile not at first boot
---
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-dist issue #75: Use new brooklyn-server-config feature
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-dist/pull/75
+1 for extracting the files during maven build.
Slight preference to having the shared project live under `brooklyn-dist`. It's all files used by the Karaf distribution. Also worth packaging all configuration files - downstream dists can then select which files to ignore.
---
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-dist issue #75: Use new brooklyn-server-config feature
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-dist/pull/75
Other than that, looks great.
---
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-dist issue #75: Use new brooklyn-server-config feature
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-dist/pull/75
Thanks @tbouron. Jenkins is happy now. 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-dist pull request #75: Use new brooklyn-server-config feature
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-dist/pull/75#discussion_r95330574
--- Diff: karaf/features/src/main/feature/feature.xml ---
@@ -55,7 +55,12 @@
<bundle>mvn:javax.annotation/javax.annotation-api/${cxf.javax.annotation-api.version}</bundle>
</feature>
+ <feature name="brooklyn-config" version="${project.version}">
--- End diff --
Is it worth declaring the feature `brooklyn-config`, rather than just using `brooklyn-server-config` directly? Did you include it for backwards compatibility of downstream projects, or for another reason?
---
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.
---