You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by revans2 <gi...@git.apache.org> on 2015/11/17 20:53:40 UTC
[GitHub] storm pull request: STORM-1202: Migrate APIs to org.apache.storm, ...
GitHub user revans2 opened a pull request:
https://github.com/apache/storm/pull/889
STORM-1202: Migrate APIs to org.apache.storm, but try to provide some form of backwards compatability
This is not a typical pull request, because changing the packages is a huge task, and keeping it upmerged while this is reviewed and other patches goes in is a huge task, and error prone.
Instead the actual changes to the package/namespace is done by running
```./dev-tools/move_packages.sh ./```
you can wipe all of the changes clean again (Including any modifications on your branch that you have not checked in) by running
```./dev-tools/cleanup.sh ./```
once the changes are approved/committed these two scripts should be removed.
The other code is intended to provide a single executable that can shade a user jar so instead of using `backtype.storm` and `storm.trident` it uses `org.apache.storm` and `org.apache.storm.trident`.
These are off by default by can be enabled by adding
```
client.jartransformer.class: "org.apache.storm.hack.StormShadeTransformer"
```
to storm.yaml. I would expect all of the code related to this to be removed when we go to the 2.0 release.
I have tested this by running a 0.10.0-SNAPSHOT storm starter topologies against a cluster running this patch (along with the move_package.sh executed).
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/revans2/incubator-storm STORM-1202
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/889.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 #889
----
commit dab5b0de51a6c83562d18ab8d823f77ff5c4d757
Author: Robert (Bobby) Evans <ev...@yahoo-inc.com>
Date: 2015-11-13T19:42:13Z
STORM-1202: Migrate APIs to org.apache.storm, but try to provide some form of backwards compatability
----
---
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] storm pull request: STORM-1202: Migrate APIs to org.apache.storm, ...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:
https://github.com/apache/storm/pull/889#issuecomment-170073038
* Downloaded 0.11 source & pulled in this branch
* ran the move_packages.sh, build and unpacked the tarball
* launched daemons from the unpacked 0.11+hack build
* downloaded 0.10 release tarball and unpacked it separately
* Used the 0.11+hack bin/storm to launch the 0.10 storm.starter.clj.word_count
* -> Client failed because of missing StormSubmitter package (good/expected)
* Did the same adding `-c client.jartransformer.class=org.apache.storm.hack.StormShadeTransformer`
* -> topology submitted and ran OK
* Checked the stormjar.jar in the supervisor's stormdist directory and looked at the word_count.clj source file inside it, and the source had been rewritten from backtype.storm.... to org.apache.storm...
The one suggestion is to add to Config what the hack implementation class is (the intended value of the config) for the use case. Otherwise users might need to hunt around to find the class we provide.
Once that's added, +1
---
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] storm pull request: STORM-1202: Migrate APIs to org.apache.storm, ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/889#issuecomment-169780775
I just filed http://dev.clojure.org/jira/browse/CLJ-1876 for the issue I was seeing with the require.
---
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] storm pull request: STORM-1202: Migrate APIs to org.apache.storm, ...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/889#discussion_r49078953
--- Diff: storm-rename-hack/pom.xml ---
@@ -0,0 +1,107 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ 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.
+-->
+<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">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <artifactId>storm</artifactId>
+ <groupId>org.apache.storm</groupId>
+ <version>0.11.0-SNAPSHOT</version>
+ <relativePath>../pom.xml</relativePath>
+ </parent>
+
+ <groupId>org.apache.storm</groupId>
+ <artifactId>storm-rename-hack</artifactId>
+ <packaging>jar</packaging>
+
+ <name>storm-rename-hack</name>
+
+ <properties>
+ <mavenVersion>3.0</mavenVersion>
+ <asmVersion>5.0.2</asmVersion>
+ </properties>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.storm</groupId>
+ <artifactId>storm-core</artifactId>
+ <version>${project.version}</version>
+ <scope>provided</scope>
+ </dependency>
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.ow2.asm</groupId>
+ <artifactId>asm</artifactId>
+ <version>${asmVersion}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.ow2.asm</groupId>
+ <artifactId>asm-commons</artifactId>
+ <version>${asmVersion}</version>
+ </dependency>
+ </dependencies>
--- End diff --
nit: check indentation
---
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] storm pull request: STORM-1202: Migrate APIs to org.apache.storm, ...
Posted by kishorvpatil <gi...@git.apache.org>.
Github user kishorvpatil commented on the pull request:
https://github.com/apache/storm/pull/889#issuecomment-158100747
I am a +1. I like this backward compatibility feature for smooth migration to package renaming. The changes look good to me. I ran some tests with topology jar compiled on 0.10.x and it works as 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] storm pull request: STORM-1202: Migrate APIs to org.apache.storm, ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/889
---
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] storm pull request: STORM-1202: Migrate APIs to org.apache.storm, ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/889#issuecomment-169775907
OK I finally tracked down some intermittent failures I was seeing when running the clojure word count example. It looks like there is an issue in clojure where how we are doing the require does not seem to be thread safe.
We should be good to go now if @d2r you want to take a final look. The previous failure is not related and is one of the random storm-kafka failures.
---
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] storm pull request: STORM-1202: Migrate APIs to org.apache.storm, ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/889#issuecomment-163754893
The test failures appear to be unrelated.
---
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] storm pull request: STORM-1202: Migrate APIs to org.apache.storm, ...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:
https://github.com/apache/storm/pull/889#issuecomment-169690819
Looks OK to me overall.
It would be nice to have a follow-on Jira Issue or Sub-Task to remove/revert the changes we need to bootstrap the testing for this (dev-tools, etc.).
> I have tested this by running a 0.10.0-SNAPSHOT storm starter topologies against a cluster running this patch (along with the move_package.sh executed).
We found that with word_count, the .clj source file had been removed from the transformed jar, but we expected that this .clj would be present, but transformed.
---
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] storm pull request: STORM-1202: Migrate APIs to org.apache.storm, ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/889#issuecomment-157487487
Because this is a non-backwards compatible change, and the mailing lists have discussed moving the version numbers over to 1.0.0-SNAPSHOT I am happy to make that change here too, but I wanted to hold off and get feedback from others before doing so.
---
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] storm pull request: STORM-1202: Migrate APIs to org.apache.storm, ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/889#issuecomment-170088345
@d2r done I updated the comment in 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.
---