Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Rejected by License Issue]add cluster nacos support #2422

Closed
wants to merge 1 commit into from

Conversation

litian33
Copy link

Please answer these questions before submitting pull request

  • Why submit this pull request?
  • Bug fix
  • [ x ] New feature provided
  • Improve performance

New feature or improvement

  • Support for discovering cluster nodes through nacos.

This feature depends on alibaba/nacos (https://github.com/alibaba/nacos).

The LICENSE is APL 2.0 (https://github.com/alibaba/nacos/blob/master/LICENSE).

@litian33 litian33 mentioned this pull request Mar 29, 2019
2 tasks
@wu-sheng
Copy link
Member

This feature depends on alibaba/nacos (https://github.com/alibaba/nacos).

Does nacos-client depend on other 3rd-party libs? I assume YES. And what are they?

@wu-sheng wu-sheng added backend OAP backend related. feature New feature labels Mar 29, 2019
@litian33
Copy link
Author

This feature depends on alibaba/nacos (https://github.com/alibaba/nacos).

Does nacos-client depend on other 3rd-party libs? I assume YES. And what are they?

This is the list of 3rd-party libs depended by nacos-client:

        <dependency>
            <groupId>org.slf4j</groupId>
            <artifactId>slf4j-api</artifactId>
            <optional>true</optional>
        </dependency>

        <dependency>
            <groupId>org.apache.logging.log4j</groupId>
            <artifactId>log4j-core</artifactId>
            <optional>true</optional>
        </dependency>

        <dependency>
            <groupId>org.apache.logging.log4j</groupId>
            <artifactId>log4j-slf4j-impl</artifactId>
            <optional>true</optional>
        </dependency>

        <dependency>
            <groupId>${project.groupId}</groupId>
            <artifactId>nacos-common</artifactId>
        </dependency>

        <dependency>
            <groupId>${project.groupId}</groupId>
            <artifactId>nacos-api</artifactId>
        </dependency>

        <dependency>
            <groupId>ch.qos.logback</groupId>
            <artifactId>logback-classic</artifactId>
            <optional>true</optional>
        </dependency>

        <dependency>
            <groupId>ch.qos.logback</groupId>
            <artifactId>logback-core</artifactId>
            <optional>true</optional>
        </dependency>

        <dependency>
            <groupId>com.google.guava</groupId>
            <artifactId>guava</artifactId>
        </dependency>

        <dependency>
            <artifactId>commons-codec</artifactId>
            <groupId>commons-codec</groupId>
        </dependency>

        <dependency>
            <groupId>org.codehaus.jackson</groupId>
            <artifactId>jackson-mapper-lgpl</artifactId>
        </dependency>
        <dependency>
            <groupId>net.jcip</groupId>
            <artifactId>jcip-annotations</artifactId>
            <optional>true</optional>
        </dependency>

        <dependency>
            <groupId>com.github.spotbugs</groupId>
            <artifactId>spotbugs-annotations</artifactId>
            <optional>true</optional>
        </dependency>

        <dependency>
            <groupId>junit</groupId>
            <artifactId>junit</artifactId>
            <scope>test</scope>
        </dependency>

        <dependency>
            <groupId>io.prometheus</groupId>
            <artifactId>simpleclient</artifactId>
            <version>0.5.0</version>
        </dependency>

The license info:

component license
slf4j-api MIT License
junit EPL 1.0
spotbugs-annotations LGPL
jackson-mapper-lgpl LGPL 2.1
logback EPL 1.0 & LGPL 2.1

The other compotents license is APL 2.0.

@wu-sheng
Copy link
Member

The licenses look like good, but what does the optional mean? Does the end user need that or not?

The right list would be the jars added in skywalking dist tar. Could you cross check your new dist with skywalking's master dist?

@JaredTan95
Copy link
Member

Wow, It's a cool feature for skywalking cluster.

@litian33
Copy link
Author

The licenses look like good, but what does the optional mean? Does the end user need that or not?

The right list would be the jars added in skywalking dist tar. Could you cross check your new dist with skywalking's master dist?

'optional' means it will not be included in the dist jars.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 13.898% when pulling 29c6fae on litian33:cluster-nacos-support into 16aca37 on apache:master.

@wu-sheng
Copy link
Member

@JaredTan95 Could you test and verify this PR?

@litian33 You need to follow this LICENSE file, https://github.com/apache/incubator-skywalking/blob/master/apm-dist/release-docs/LICENSE#L204

  1. add license descriptions of your new jars(in dist lib)
  2. If LICENSE is not Apache 2.0, you need to copy the license file to license folder
  3. If LICENSE is Apache 2.0, and it includes NOTICE file in same folder, you need to copy the contents into our dist NOTICE

This is the legal way :) So you must follow. Also, after you have done, please provide a list in here to help us recheck these dependencies.

@peng-yongsheng
Copy link
Member

| spotbugs-annotations | LGPL |
| jackson-mapper-lgpl | LGPL 2.1 |

@wu-sheng GNU LGPL 2, 2.1, 3 may NOT be included within Apache products.

@wu-sheng wu-sheng added this to the 6.1.0 milestone Mar 30, 2019
@wu-sheng wu-sheng added the wontfix This will not be worked on label Mar 30, 2019
@wu-sheng wu-sheng changed the title add cluster nacos support [Rejected by License Issue]add cluster nacos support Mar 30, 2019
@wu-sheng
Copy link
Member

GNU LGPL 2, 2.1, 3 may NOT be included within Apache products.

@peng-yongsheng Oh, sorry I miss one. Dependency jackson-mapper-lgpl is not optional. Thank you for pointing this out.

@litian33 We can't accept this PR because it breaks the Apache foundation requirements. You can use them, but in Apache release and source code host, we can't. If you want to contribute this, I will recommend you open source in another repo, and release there, we could provide a document link to there. In that repo, you could do release by following your own wish and provide the document to guide user to use.

If you want to make this in Apache repo, then all GPL/LGPC must be removed, please read catalog-x

@wu-sheng wu-sheng closed this Mar 30, 2019
@wu-sheng wu-sheng removed backend OAP backend related. feature New feature labels Mar 30, 2019
@litian33
Copy link
Author

litian33 commented Apr 1, 2019

OK. I will matain the fork myself.

@wu-sheng
Copy link
Member

wu-sheng commented Apr 1, 2019

@litian33 I suggest you do something like https://github.com/SkyAPM/java-plugin-extensions . You deploy the plugin based on our release API, then release by yourself.

People could download from your page, and copy them into skywalking lib folder to active it.

The fork is unsearchable in GitHub. So, a new repo is a better choice from my perspective.

@wu-sheng
Copy link
Member

wu-sheng commented Apr 1, 2019

@litian33 After you released, we are welcome to accept your PR to submit documents in cluster management section, which could include a link to your repo or release page.

We are highly appreciated your contributions and extension.

@wu-sheng
Copy link
Member

wu-sheng commented Apr 1, 2019

@litian33 Nacos team is responding. You could follow the issue I just raised.

@wu-sheng
Copy link
Member

wu-sheng commented Apr 1, 2019

@litian33 I have got contact with Nacos team, they are working on removing the LGPL libs. So, could you try to update to next release, and submit a new PR again?

@wu-sheng
Copy link
Member

wu-sheng commented Apr 3, 2019

@litian33 Could you try to use the latest Nacos release to submit a new PR and list the new dependencies there?

@wu-sheng wu-sheng mentioned this pull request May 9, 2019
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants