[GitHub] storm pull request #2148: Let Topology query if blobs have changed
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2148#discussion_r120129559
--- Diff: storm-client/src/jvm/org/apache/storm/daemon/worker/Worker.java
---
@@ -246,7 +248,19 @@ public void start() throws Exception {
}
}
});
-
+
+workerState.checkForUpdatedBlobsTimer.scheduleRecurring(0,
+(Integer)
conf.getOrDefault(Config.WORKER_BLOB_UPDATE_POLL_INTERVAL_SECS, 10), new
Runnable() {
+@Override public void run() {
+try {
+LOG.info("Checking if blobs have
updated");
+updateBlobUpdates();
+} catch (IOException e) {
+e.printStackTrace();
--- End diff --
Can we log this instead of printing it to stderr? Also it might be nice to
have a comment explaining why you are ignoring the exception.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2148: Let Topology query if blobs have changed
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2148#discussion_r120129659
--- Diff: storm-client/src/jvm/org/apache/storm/daemon/worker/Worker.java
---
@@ -299,6 +313,33 @@ public void doExecutorHeartbeats() {
}
}
+public Map<String, Long> getCurrentBlobVersions() throws IOException {
+Map<String, Long> results = new HashMap();
--- End diff --
The new HashMap should have the generics on it `new HashMap<>();`
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2149: STORM-2503: Fix lgtm.com alerts on equality and co...
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2149#discussion_r120122106
--- Diff:
storm-core/test/jvm/org/apache/storm/scheduler/resource/strategies/scheduling/TestDefaultResourceAwareStrategy.java
---
@@ -111,55 +112,23 @@ public void testDefaultResourceAwareStrategy() {
rs.prepare(conf);
rs.schedule(topologies, cluster);
-Map<String, List> nodeToComps = new HashMap<String,
List>();
-for (Map.Entry<ExecutorDetails, WorkerSlot> entry :
cluster.getAssignments().get("testTopology-id").getExecutorToSlot().entrySet())
{
-WorkerSlot ws = entry.getValue();
-ExecutorDetails exec = entry.getKey();
-if (!nodeToComps.containsKey(ws.getNodeId())) {
-nodeToComps.put(ws.getNodeId(), new LinkedList());
-}
-
nodeToComps.get(ws.getNodeId()).add(topo.getExecutorToComponent().get(exec));
+HashSet<HashSet> expectedScheduling = new
HashSet<>();
+expectedScheduling.add(new HashSet<>(Arrays.asList(new
ExecutorDetails(0, 0; //Spout
+expectedScheduling.add(new HashSet<>(Arrays.asList(
+new ExecutorDetails(2, 2), //bolt-1
+new ExecutorDetails(4, 4), //bolt-2
+new ExecutorDetails(6, 6; //bolt-3
+expectedScheduling.add(new HashSet<>(Arrays.asList(
+new ExecutorDetails(1, 1), //bolt-1
+new ExecutorDetails(3, 3), //bolt-2
--- End diff --
The test is failing with 3, 3 switched with 4, 4. I am not sure what
caused the switch. Both are valid scheduling so I am inclined to just have you
make the change here in the test to let it pass.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2113: STORM-2497: Let Supervisor enforce memory and add in supp...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2113 @kishorvpatil Could you look again. I had to rebase becase of a minor conflict in the DefaultResourceAwareStrategy. It ended up resulting in only some comment changes to this code, but I wanted to be sure you took a 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2142: MINOR: Fix pacemaker_state_factory.clj not compile...
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2142#discussion_r119911355
--- Diff:
storm-core/src/clj/org/apache/storm/pacemaker/pacemaker_state_factory.clj ---
@@ -106,16 +106,15 @@
(defn get-pacemaker-write-client [conf servers client-pool]
;; Client should be created in case of an exception or first write call
;; Shutdown happens in the retry loop
- (try
-(.waitUntilReady
- (let [client (get @client-pool (first @servers))]
- (if (nil? client)
- (do
- (swap! client-pool merge {(first @servers) (PacemakerClient.
conf (first @servers))})
- (get @client-pool (first @servers)))
- client)))
-(catch Exception e
- (throw e
+ (let [client (get @client-pool (first @servers))]
+(try
+(.waitUntilReady
+(let [] (if (nil? client)
+ (do (swap! client-pool merge {(first @servers)
(PacemakerClient. conf (first @servers))})
+ (get @client-pool (first @servers)))
+ client)))
+ (catch Exception e (throw e)))
+(get @client-pool (first @servers
--- End diff --
waitUntilReady is not actually needed because send will call it internally.
We can just skip calling it here all together. This then lets us have
something like ...
```
(defn get-pacemaker-write-client [conf servers client-pool]
;; Client should be created in case of an exception or first write call
;; Shutdown happens in the retry loop
(let [server (first @servers)]
(or (get @client-pool server)
(get (swap! client-pool merge {server (PacemakerClient. conf
server)}) server
```
and we can then remove the update to PacemakerClient.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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2143: STORM-2503: Restore comparator logic in `DefaultResourceA...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2143 @jerrypeng Like I said in the previous pull request. The unit test that failed was not cause by this change. It was cause by an actual fix to the comparison logic for `sortComponents`. In my tests I saw the output of `sortComponents` having duplicate entries prior to that fix. I am +1 for this change. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2100: STORM-2503: Fix lgtm.com alerts on equality and compariso...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2100 @jerrypeng The reason that particular piece of code was flagged was because the logic and the return values were not consistent with each other. returning -1 means less than (but the code was checking for a greater than). As for me I was mostly looking at the tests and as all of the tests passed (except for one that was not actually impacted by this change but another change that was a really serious bug). If we need to switch this particular part of the change back I am happy to do 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2127: STORM-2525: Fix flaky integration tests
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2127 Well no one else has looked at it, and it does cut 30 mins off of a build, so I am going to check it in, and if we run into issues in the future we can fix them then. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2136: [STORM-2206] - replacing visualization with viz.js
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2136 It is looking good. A few notes. Ackers (__acker) are not spouts. They are bolts (but it might be nice to distinguish between system bolts and other bolts) Ackers are not showing any instances even when there are some. The System Bolt (__system) is a bolt but not really, and probably does not need to show up here at all. It is created as a way to collect metrics and may go away in the future. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2135: Leave checkstyle violation to each module's target direct...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2135 +1 Although I think this is already happening. As a part of https://github.com/apache/storm/pull/2113 I added in an xslt script to make reading the XML format better (like plain) and a python script to help search through it to help do things like look at violations only for files I touched with this patch. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2052: STORM-2448: Report storm and jdk versions to nimbus on to...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2052 But I am happy to only add this into our own release of storm if you don't think this is something you want to try and support. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2052: STORM-2448: Report storm and jdk versions to nimbus on to...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2052 @HeartSaVioR We do have a large number on 0.10 and we are hoping that this will be a good way to transition them off to 2.x when we are ready to go to that version. The plan is not to have 0.10 run under a 1.x cluster, but to have 0.10 or 1.x run under a 2.x cluster. I have manually tested this and it does work. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2129: [STORM-2206] - initial checkin of visualization us...
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2129#discussion_r117826289
--- Diff: storm-core/src/ui/public/js/visualization.js ---
@@ -16,434 +16,330 @@
* limitations under the License.
*/
-// Inspired by
-//
https://github.com/samizdatco/arbor/blob/master/docs/sample-project/main.js
-
-function renderGraph(elem) {
-
-var canvas = $(elem).get(0);
-canvas.width = $(window).width();
-canvas.height = $(window).height();
-var ctx = canvas.getContext("2d");
-var gfx = arbor.Graphics(canvas);
-var psys;
-
-var totaltrans = 0;
-var weights = {};
-var texts = {};
-var update = false;
-
-var myRenderer = {
-init: function(system){
-psys = system;
-psys.screenSize(canvas.width, canvas.height)
-psys.screenPadding(20);
-myRenderer.initMouseHandling();
-},
-
-signal_update: function() {
-update = true;
-},
-
-redraw: function() {
-
-if(!psys)
-return;
-
-if(update) {
-totaltrans = calculate_total_transmitted(psys);
-weights = calculate_weights(psys, totaltrans);
-texts = calculate_texts(psys, totaltrans);
-update = false;
+var _showSystem = false;
+var _showAcker = false;
+var _showMetrics = false;
+var container;
+
+var options = {
+edges:{
+arrows: {
+to: {enabled: true, scaleFactor:1}
+},
+hoverWidth: 1.5,
+shadow:{
+enabled: true,
+color: 'rgba(0,0,0,0.5)',
+size:10,
+x:5,
+y:5
+}
+},
+nodes: {
+color: {
+border: '#2B7CE9',
+background: '#97C2FC',
+highlight: {
+border: '#2B7CE9',
+background: '#D2E5FF'
+},
+hover: {
+border: '#2B7CE9',
+background: '#D2E5FF'
+}
+},
+shadow:{
+ enabled: true,
+ color: 'rgba(0,0,0,0.5)',
+ size:10,
+ x:5,
+ y:5
+},
+},
+physics:{
+enabled: false
+},
+layout: {
+randomSeed: 31337,
+improvedLayout:true,
+hierarchical: {
+enabled: true,
+levelSeparation: 150,
+nodeSpacing: 300,
+treeSpacing: 200,
+blockShifting: true,
+edgeMinimization: true,
+parentCentralization: true,
+direction: 'UD',// UD, DU, LR, RL
+sortMethod: 'directed' // hubsize, directed
+}
+},
+interaction: {
+navigationButtons: false
}
+};
+// Holds all stream names
+var availableStreamsHash = { }
+// Holds our network
+var network;
-ctx.fillStyle = "white";
-ctx.fillRect(0, 0, canvas.width, canvas.height);
-var x = 0;
-
+// Holds nodes and edge definitions
+var nodes = new vis.DataSet();
+var edges = new vis.DataSet();
-psys.eachEdge(function(edge, pt1, pt2) {
+// Update/refresh settings
+var should_update = true
+var update_freq_ms = 3
-var len = Math.sqrt(Math.pow(pt2.x - pt1.x,2) +
Math.pow(pt2.y - pt1.y,2));
-var sublen = len - (Math.max(50, 20 +
gfx.textWidth(edge.target.name)) / 2);
-var thirdlen = len/3;
-var theta = Math.atan2(pt2.y - pt1.y, pt2.x - pt1.x);
-
-var newpt2 = {
-x : pt1.x + (Math.cos(theta) * sublen),
-y : pt1.y + (Math.sin(theta) * sublen)
-};
+function parseResponse(json) {
+console.l
[GitHub] storm pull request #2129: [STORM-2206] - initial checkin of visualization us...
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2129#discussion_r117822198 --- Diff: storm-core/src/ui/public/js/vis.min.js --- @@ -0,0 +1,45 @@ +/** + * vis.js + * https://github.com/almende/vis + * + * A dynamic, browser-based visualization library. + * + * @version 4.16.1 + * @date2016-04-18 + * + * @license + * Copyright (C) 2011-2016 Almende B.V, http://almende.com + * + * Vis.js is dual licensed under both + * + * * The Apache 2.0 License + * http://www.apache.org/licenses/LICENSE-2.0 + * + * and + * + * * The MIT License + * http://opensource.org/licenses/MIT + * + * Vis.js may be distributed under either license. --- End diff -- With this we need to make sure that we update LICENSE.txt to include this file and the license that we pick for it. We probably also want to white list this and vis.min.css for rat, so it stops complaining about them. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2129: [STORM-2206] - initial checkin of visualization using vis...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2129 @Crim Don't feel bad. When I was playing around with this I found (https://issues.apache.org/jira/browse/STORM-2526) which was my own face palm moment. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2125: nothing but fix one word
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2125 Thanks @lijiansong I merged this in keep up the good work. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2126: Add the option to set client.id to storm-kafka and storm-...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2126 Also please upmerge there is a merge conflict. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2126: Add the option to set client.id to storm-kafka and...
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2126#discussion_r117774461 --- Diff: external/storm-kafka/pom.xml --- @@ -57,7 +57,7 @@ maven-checkstyle-plugin -557 +558 --- End diff -- I would prefer to see the violations fixed instead of adding in new ones. In this case I suspect that it is missing javadocs for the new SpoutConfig 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2129: [STORM-2206] - initial checkin of visualization using vis...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2129 My biggest concern right now it that it looks like you missed checking in a few files. I see `vis.min.js`, `vis.min.css`, `streams.png`, and `bolt_info.png` all getting a 404 when I try to 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2130: STORM-2526: Revert changes mistakenly made to gene...
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2130 STORM-2526: Revert changes mistakenly made to generated files I regenerated all of the generated thrift files to be sure I hadn't mistakenly changed anything else. Then I updated the one call to those APIs in the java code. You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2526 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2130.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 #2130 commit 185101dd8cd4955e37279670d8ae3f61e43f37a6 Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-05-22T15:02:48Z STORM-2526: Revert changes mistakenly made to generated files --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2083: STORM-2421: support lists of childopts in DaemonConfig.
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2083 @hmcc looks like there is a merge conflict now. If you could resolve it I would be happy to check this in. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2072: STORM-2477: Add generics to Config types
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2072 @HeartSaVioR Thanks I'll make the change --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2072: STORM-2477: Add generics to Config types
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2072#discussion_r117247959 --- Diff: .travis.yml --- @@ -27,6 +27,7 @@ before_install: - rvm use 2.1.5 --install - nvm install 0.12.2 - nvm use 0.12.2 + - ./transform.sh --- End diff -- If you just want to see the fully transformed code I can do that, but it is as of right now it ends up being ``` 593 files changed, 1481 insertions(+), 1481 deletions(-) ``` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2072: STORM-2477: Add generics to Config types
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2072#discussion_r117247297 --- Diff: .travis.yml --- @@ -27,6 +27,7 @@ before_install: - rvm use 2.1.5 --install - nvm install 0.12.2 - nvm use 0.12.2 + - ./transform.sh --- End diff -- Sorry Maybe I didn't make myself clear in the initial comments. Because this is touching a lot of different places in the code I thought it would be best to have the transformations be done by a script, that way if other stuff is merged in, we don't have to worry as much about merge conflicts. If I get the needed +1s on the change I will run the script, remove it and any mention of it in the code and check in the result before merging it to master. I added this line so that travis could test the fully transformed code. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2083: STORM-2421: support lists of childopts in DaemonCo...
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2083#discussion_r117245034
--- Diff: storm-client/src/jvm/org/apache/storm/utils/ConfigUtils.java ---
@@ -313,6 +317,33 @@ public static File getWorkerDirFromRoot(String
logRoot, String id, Integer port)
return new File((logRoot + FILE_SEPARATOR + id + FILE_SEPARATOR +
port));
}
+/**
+ * Get the given config value as a List String, if possible.
+ * @param name - the config key
+ * @param conf - the config map
+ * @return - the config value converted to a List String if
found, otherwise null.
+ * @throws IllegalArgumentException if conf is null
+ * @throws NullPointerException if name is null and the conf map
doesn't support null keys
+ */
+public static List getValueAsList(String name, Map conf)
{
--- End diff --
Minor nit: we tend to use Map<String, Object> for conf.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2109: STORM-2506: Print mapping between Task ID and Kafka Parti...
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/2109
@erikdw and @srishtyagrawal I know that we are still working through issues
with checkstyle being new, but I get a little nervous with making too many
changes to the "standard".
The more changes we make to our style the harder it is to explain to people
what that style is and the harder it is to setup an IDE to conform with it. As
it is right now we can say (and we should update the docs for this) that we
conform to the google standard except that we use 4 spaces for indentation and
have a max line length of 140. But if we start adding in more changes it gets
harder to explain and we end up being like HBase where some impossible to read
file is the documentation. If it is not too much of a problem would we rename
{{taskID}} to {{taskId}}?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2100: STORM-2503: Fix lgtm.com alerts on equality and compariso...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2100 I am +1 on the change, but because my fix for the test is a part of this please consider my +1 non-binding. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2049: STORM-2448: Add in Storm and JDK versions when submitting...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2049 @kishorvpatil @knusbaum Could you two take a look at this and also the patches for 1.x 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2072: STORM-2477: Add generics to Config types
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2072 @ptgoetz @HeartSaVioR if either of you could take a look at this I would appreciate 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2074: Storm 1290:port backtype.storm.local-state-test to...
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2074#discussion_r116600864
--- Diff: storm-server/src/test/java/org/apache/storm/LocalStateTest.java
---
@@ -0,0 +1,61 @@
+package org.apache.storm;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.storm.generated.GlobalStreamId;
+import org.apache.storm.testing.TmpPath;
+import org.apache.storm.utils.LocalState;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+
+public class LocalStateTest {
+
+@Test
+public void testLocalState() throws Exception{
+try(TmpPath dir1_tmp = new TmpPath(); TmpPath dir2_tmp = new
TmpPath() ) {
+GlobalStreamId globalStreamId_a = new GlobalStreamId("a", "a");
+GlobalStreamId globalStreamId_b = new GlobalStreamId("b", "b");
+GlobalStreamId globalStreamId_c = new GlobalStreamId("c", "c");
+GlobalStreamId globalStreamId_d = new GlobalStreamId("d", "d");
+
+LocalState ls1 = new LocalState(dir1_tmp.getPath());
+LocalState ls2 = new LocalState(dir2_tmp.getPath());
+Assert.assertTrue(ls1.snapshot().isEmpty());
+
+ls1.put("a", globalStreamId_a);
+ls1.put("b", globalStreamId_b);
+Assert.assertTrue(ls1.snapshot().containsKey("a"));
+
Assert.assertTrue(ls1.snapshot().containsValue(globalStreamId_a));
+Assert.assertTrue(ls1.snapshot().containsKey("b"));
+
Assert.assertTrue(ls1.snapshot().containsValue(globalStreamId_b));
--- End diff --
I would prefer to have to code more closely follow the test.
```
//(is (= {"a" gs-a "b" gs-b} (.snapshot ls1)))
Map<String, TBase> expected = new HashMap<>();
expected.put("a", globalStreamId_a);
expected.put("b", globalStreamId_b);
Assert.assertEquals(expected, ls1.snapshot());
```
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2074: Storm 1290:port backtype.storm.local-state-test to...
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2074#discussion_r116601117
--- Diff: storm-server/src/test/java/org/apache/storm/LocalStateTest.java
---
@@ -0,0 +1,61 @@
+package org.apache.storm;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.storm.generated.GlobalStreamId;
+import org.apache.storm.testing.TmpPath;
+import org.apache.storm.utils.LocalState;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+
+public class LocalStateTest {
+
+@Test
+public void testLocalState() throws Exception{
+try(TmpPath dir1_tmp = new TmpPath(); TmpPath dir2_tmp = new
TmpPath() ) {
+GlobalStreamId globalStreamId_a = new GlobalStreamId("a", "a");
+GlobalStreamId globalStreamId_b = new GlobalStreamId("b", "b");
+GlobalStreamId globalStreamId_c = new GlobalStreamId("c", "c");
+GlobalStreamId globalStreamId_d = new GlobalStreamId("d", "d");
+
+LocalState ls1 = new LocalState(dir1_tmp.getPath());
+LocalState ls2 = new LocalState(dir2_tmp.getPath());
+Assert.assertTrue(ls1.snapshot().isEmpty());
+
+ls1.put("a", globalStreamId_a);
+ls1.put("b", globalStreamId_b);
+Assert.assertTrue(ls1.snapshot().containsKey("a"));
+
Assert.assertTrue(ls1.snapshot().containsValue(globalStreamId_a));
+Assert.assertTrue(ls1.snapshot().containsKey("b"));
+
Assert.assertTrue(ls1.snapshot().containsValue(globalStreamId_b));
+Assert.assertEquals(globalStreamId_a, ls1.get("a"));
+Assert.assertEquals(null, ls1.get("c"));
+Assert.assertEquals(ls1.snapshot(), new
LocalState(dir1_tmp.getPath()).snapshot());
--- End diff --
Similar here I would prefer to see it match the test
```
//(is (= {"a" gs-a "b" gs-b} (.snapshot (LocalState. dir1
Assert.assertEquals(expected, new
LocalState(dir1_tmp.getPath()).snapshot());
```
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2100: STORM-2503: Fix lgtm.com alerts on equality and compariso...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2100 @adityasharad I put up https://github.com/adityasharad/storm/pull/1 to your repo that should fix the failing test. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2100: STORM-2503: Fix lgtm.com alerts on equality and compariso...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2100 @adityasharad so the test itself was testing for the wrong behavior, and when you fixed it the test stated to fail. I will try to make a pull request to your repo with a fix for the test. Please let me know if you want any help getting this in as I really would like to see these fixed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2100: STORM-2503: Fix lgtm.com alerts on equality and compariso...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2100 @adityasharad Some of the tests are failing now specifically TestDefaultResourceAwareStrategy. I'll try and spend some time looking into it to see if I can see why it is failing, because the test itself is not giving any useful information at all. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2111: Fix minor typos in storm-hdfs and storm-hbase docs that s...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2111 Thanks, @arunmahadevan I merged this in --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2113: STORM-2497: Let Supervisor enforce memory and add ...
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2113 STORM-2497: Let Supervisor enforce memory and add in support for shared memory regions This is based off of #2112 so that the formatting is simpler. You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2497 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2113.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 #2113 commit d8903b8358497c1b7c0ddb3588e779d5ce7c13b5 Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-04-28T21:10:29Z STORM-2497: Let Supervisor enforce memory and add in support for shared memory regions --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2112: [STORM-2510] update checkstyle configuration to lower vio...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2112 Oops we need a license in `storm-buildtools/storm_checkstyle.xml` or we need to white list it for rat. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2112: [STORM-2510] update checkstyle configuration to lower vio...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2112 Like I said before I am fine with whatever convention we have, so long as we have one. +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2093: STORM-2495:Integrate checkstyle check during build
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2093 Still +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2083: STORM-2421: support lists of childopts in DaemonCo...
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2083#discussion_r114172094
--- Diff: storm-client/src/jvm/org/apache/storm/utils/ConfigUtils.java ---
@@ -313,6 +315,34 @@ public static File getWorkerDirFromRoot(String
logRoot, String id, Integer port)
return new File((logRoot + FILE_SEPARATOR + id + FILE_SEPARATOR +
port));
}
+/**
+ * Get the given config value as a String, if possible.
+ * @param name - the config key
+ * @param conf - the config map
+ * @return - the config value converted to a String if found,
otherwise null.
+ * @throws IllegalArgumentException if conf is null
+ * @throws NullPointerException if name is null and the conf map
doesn't support null keys
+ */
+public static String getConfigValueAsString(String name, Map
conf) {
--- End diff --
@hmcc Yes, except the caller should not be converting it to a space
separated string because it will break some use cases.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2093: STORM-2495:Integrate checkstyle check during build
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2093 @vinodkc Looks good we are likely going to need to modify the total number of violations when this is ready to be merged in. I would love to see something that can find the checkstyle-result.xml that caused a failure (or at least has some of the failures in it) and will output some of the violations. But that may be above and beyond. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2076: [STORM-1642] Catch Exception when deserialization failed ...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2076 @govind-menon Actually I think it is SASL not SSL that you want. if you just set ``` storm.messaging.netty.authentication=true ``` Either as the default for your cluster or even on a per topology bases (I think) each of the netty connections between workers will do a handshake and validate that each of them have the same secret, that is generated when the topology is submitted. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2093: STORM-2495:Integrate checkstyle check during build
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2093 @vinodkc We cannot increase the size of the log without paying money to travisci, or switching the build to the internal Jenkins servers at apache. The later has been proposed but is not likely to happen soon. My advice would be to set the useFile config to move the warnings to a text file that we can output the first X lines of if the build fails. Then add in a maxAllowedViolations for each project with their current number of violations along with a JIRA to fix the warnings in the project and remove the limit. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2094: [STORM-2191] shorten classpaths by using wildcards
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2094#discussion_r113919131 --- Diff: docs/Setting-up-a-Storm-cluster.md --- @@ -102,9 +102,9 @@ The time to allow any given healthcheck script to run before it is marked failed storm.health.check.timeout.ms: 5000 ``` -### Configure external libraries and environmental variables (optional) +### Configure external libraries and environment variables (optional) -If you need support from external libraries or custom plugins, you can place such jars into the extlib/ and extlib-daemon/ directories. Note that the extlib-daemon/ directory stores jars used only by daemons (Nimbus, Supervisor, DRPC, UI, Logviewer), e.g., HDFS and customized scheduling libraries. Accordingly, two environmental variables STORM_EXT_CLASSPATH and STORM_EXT_CLASSPATH_DAEMON can be configured by users for including the external classpath and daemon-only external classpath. +If you need support from external libraries or custom plugins, you can place such jars into the extlib/ and extlib-daemon/ directories. Note that the extlib-daemon/ directory stores jars used only by daemons (Nimbus, Supervisor, DRPC, UI, Logviewer), e.g., HDFS and customized scheduling libraries. Accordingly, two environment variables STORM_EXT_CLASSPATH and STORM_EXT_CLASSPATH_DAEMON can be configured by users for including the external classpath and daemon-only external classpath. See [Classpath handling](Classpath-handling.html)] for more details on using external libraries. --- End diff -- It might be good to include a link to Classpath-handling.html in Running-topologies-on-a-production-cluster.md or some other page that talks about running a topology. Perhaps Command-line-client.md would be good to, so it can explain how this works for users launching workers. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2083: STORM-2421: support lists of childopts in DaemonCo...
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2083#discussion_r113841362
--- Diff: storm-client/src/jvm/org/apache/storm/utils/ConfigUtils.java ---
@@ -313,6 +315,34 @@ public static File getWorkerDirFromRoot(String
logRoot, String id, Integer port)
return new File((logRoot + FILE_SEPARATOR + id + FILE_SEPARATOR +
port));
}
+/**
+ * Get the given config value as a String, if possible.
+ * @param name - the config key
+ * @param conf - the config map
+ * @return - the config value converted to a String if found,
otherwise null.
+ * @throws IllegalArgumentException if conf is null
+ * @throws NullPointerException if name is null and the conf map
doesn't support null keys
+ */
+public static String getConfigValueAsString(String name, Map
conf) {
--- End diff --
If we are going to a common type for *childopts it should be a
`List` not `String`. The reason for a list is that something with
whitespace in it can be properly represented this breaks 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2049: STORM-2448: Add in Storm and JDK versions when submitting...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2049 @ptgoetz @HeartSaVioR ping again. This should give us the ability to do a rolling upgrade from 1.x to 2.x --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2060: STORM-2468: Remove clojure from storm-client
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2060 @HeartSaVioR @ptgoetz If you have time to take a look I would appreciate it. It looks like @knusbaum might be a bit too busy to take a look in a timely manor. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2076: [STORM-1642] Catch Exception when deserialization ...
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2076#discussion_r111955758
--- Diff:
storm-core/src/jvm/org/apache/storm/serialization/KryoTupleDeserializer.java ---
@@ -50,7 +54,8 @@ public Tuple deserialize(byte[] ser) {
List values = _kryo.deserializeFrom(_kryoInput);
return new TupleImpl(_context, values, taskId, streamName, id);
} catch(IOException e) {
-throw new RuntimeException(e);
+LOG.error("Failed to deserialize tuple.", e);
--- End diff --
Then we need to secure the connection against attach and not mask real
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2072: STORM-2477: Add generics to Config types
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2072 @srdo please continue to nit pick. I like good clean code and I am happy to see people call me out on things that should be fixed. I find that people who read the code closely and nit pick find more serious bugs during the review which saves me a lot of time debugging and fixing them later on. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2072: STORM-2477: Add generics to Config types
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2072 @srdo done --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2072: STORM-2477: Add generics to Config types
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2072 @srdo can do. Wow you really don't like ugly code :) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2076: [STORM-1642] Catch Exception when deserialization ...
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2076#discussion_r111740922
--- Diff:
storm-core/src/jvm/org/apache/storm/serialization/KryoTupleDeserializer.java ---
@@ -50,7 +54,8 @@ public Tuple deserialize(byte[] ser) {
List values = _kryo.deserializeFrom(_kryoInput);
return new TupleImpl(_context, values, taskId, streamName, id);
} catch(IOException e) {
-throw new RuntimeException(e);
+LOG.error("Failed to deserialize tuple.", e);
--- End diff --
Storm is designed to be a fail fast system, this fix appears to be doing
the opposite of that. If we cannot deserialize a tuple we need to make it very
very clear to the user that something bad is happening. Sadly in my experience
even crashing the worker is not always enough, but in most cases it is. Can we
do a System.exit after logging the error instead of returning null?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2072: STORM-2477: Add generics to Config types
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2072 @srdo I addressed your review comments about empty map. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2072: STORM-2477: Add generics to Config types
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2072#discussion_r111739215
--- Diff:
external/storm-hbase/src/main/java/org/apache/storm/hbase/bolt/AbstractHBaseBolt.java
---
@@ -51,11 +51,11 @@ public AbstractHBaseBolt(String tableName, HBaseMapper
mapper) {
}
@Override
-public void prepare(Map map, TopologyContext topologyContext,
OutputCollector collector) {
+public void prepare(Map topoConf, TopologyContext topologyContext,
OutputCollector collector) {
this.collector = collector;
final Configuration hbConfig = HBaseConfiguration.create();
-Map<String, Object> conf = (Map<String,
Object>)map.get(this.configKey);
+Map<String, Object> conf = (Map<String,
Object>)topoConf.get(this.configKey);
--- End diff --
No because of the order of operations. It is casting the value returned by
`topoConf.get`, not `topoConf` itself.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2072: STORM-2477: Add generics to Config types
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2072 STORM-2477: Add generics to Config types Because this touches so much of the code to avoid having to constantly upmerge I have a script that will transform most of the code automatically. transform.sh . I don't plan on checking it in. If everyone is OK with the change I will run the script and just check those changes in. The other changes and small updates to make the result of running the script not produce compile errors. Overall this drops more than 800 warnings from the code base (according to eclipse). You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2477 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2072.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 #2072 commit dc6d2bbd540304552e9c56eba5d24eb0e4d0f74b Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-04-15T21:21:20Z Fixed issues with generics to make script work commit 0b088ce1716ffabb6bace2c58e39220f187c1769 Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-04-16T00:17:27Z Script to change add proper types to configs and make stormConf topoConf --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2069: STORM-2475: Fix parsing of host:port to deal with ...
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2069 STORM-2475: Fix parsing of host:port to deal with IPv6 addresses You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2475 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2069.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 #2069 commit 2823682d1de37676b76011535fdbe3dd311208cd Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-04-14T00:34:11Z STORM-2475: Fix parsing of host:port to deal with IPv6 addresses --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2063: [STORM-2471] Add metric for thread count.
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2063 @jacobtolar please remove the line from the changelog. It is too hard to keep it in sync while the code is reviewed. We will add it back in when we merge it in. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2064: STORM-1114: Handle race condition in Storm/Trident transa...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2064 @ptgoetz and yes lets please do port this back --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2065: STORM-832: Allow config validation to be used by p...
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2065 STORM-832: Allow config validation to be used by plugins/etc. I did this because hard coding the usage of DaemonConf.java into a few places just didn't look right, and I thought it would be better to just do this feature and make it generic so others can start using this too, and config settings for a plugin can be with the plugin, and not forced to be in Config or DaemonConfig You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-832 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2065.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 #2065 commit aec187f4302ac81f0bc308278beed6e9c56b7d3e Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-04-12T18:31:20Z STORM-832: Allow config validation to be used by plugins/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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2063: [STORM-2471] Add metric for thread count.
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2063 +1 looks good to me. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2054: STORM-2462 Adding regex mapper to KerberosPrincipalToLoca...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2054 I have been thinking about this, and I am not sure if a REGEX is really what we want. Hadoop does this in a semi-standard way using auth_to_local rules like PAM does. It might be worth lifting some of that code from hadoop and reusing it here instead of just a regular expression. https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/KerberosName.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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2054: STORM-2462 Adding regex mapper to KerberosPrincipa...
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2054#discussion_r91074
--- Diff:
storm-client/src/jvm/org/apache/storm/security/auth/RegexKerberosPrincipalToLocal.java
---
@@ -0,0 +1,56 @@
+/**
+ * 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.storm.security.auth;
+
+import java.util.Map;
+import java.security.Principal;
+
+/**
+ * Map a kerberos principal to a local user
+ */
+public class RegexKerberosPrincipalToLocal extends
KerberosPrincipalToLocal {
--- End diff --
Really there are two ways to make this optional. You can do it like this
and making it a separate class or you can have the configs be optional, and if
they are not set then no modification is done. I kind of prefer the other one,
as it feels more intuitive to me.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2060: STORM-2468: Remove clojure from storm-client
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2060 @knusbaum please take a look at this. There were a number of changes here. Some to replace functionality being used from clojure runtime with java code. Some of it was moving/deduplicating dependencies, but the two big ones were adding in support to register the clojure kryo serialization transparently and making a tuple have the same clojure integration it had before by replacing it with a new tuple. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2060: STORM-2468: Remove clojure from storm-client
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2060 Now that STORM-2447 is in I have rebased this on master. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2059: STORM-2463: fix DRPCTest.testDequeueAfterTimeout test fai...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2059 OK Still +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2049: STORM-2448: Add in Storm and JDK versions when submitting...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2049 @ptgoetz @HeartSaVioR I would appreciate a review of this. I would like to know what people think about it. If it is good I am happy to add in more documentation and to rip out the rename hack code, as it would no longer really be needed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2028: Fix headers
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2028 +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2060: STORM-2468: Remove clojure from storm-client
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2060 Only the last commit is the one to look at. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2060: STORM-2468: Remove clojure from storm-client
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2060 STORM-2468: Remove clojure from storm-client This is based off STORM-2447 You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2468 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2060.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 #2060 commit b254ede46a25466749cd48ebd4bcb56dd791ec4a Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-04-06T18:58:41Z STORM-2447: add in storm local to avoid having server on worker classpath commit 9eced5310464f87ca52eb92b6224ef5abe531514 Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-04-06T19:50:14Z STORM-2447: Fixed compilation issue with int test commit 71dc2e019d84580fbcdd5eabf267016393324872 Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-04-07T13:31:56Z Merge branch 'apache-master' into STORM-2447 commit 979d0b89f1b76342a25091dd381d3faeefa71ad0 Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-04-10T12:51:02Z STORM-2468: Remove clojure from storm-client --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2032: [STORM-2093] Fix permissions in multi-tenant, secure mode
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2032 @ppoulosk Sadly with the latest refactoring of the code `SupervisorUtils.processLauncherAndWait` is not on the classpath for storm-core any more. It is a part of storm-server. You may need to do some refactoring to make this work. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2032: [STORM-2093] Fix permissions in multi-tenant, secure mode
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2032 +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2054: STORM-2462 Adding regex mapper to KerberosPrincipa...
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2054#discussion_r110968429
--- Diff:
storm-client/src/jvm/org/apache/storm/security/auth/RegexKerberosPrincipalToLocal.java
---
@@ -0,0 +1,56 @@
+/**
+ * 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.storm.security.auth;
+
+import java.util.Map;
+import java.security.Principal;
+
+/**
+ * Map a kerberos principal to a local user
+ */
+public class RegexKerberosPrincipalToLocal extends
KerberosPrincipalToLocal {
--- End diff --
I would prefer to see this be inside KerberosPrincipalToLocal instead of
having a separate class.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2054: STORM-2462 Adding regex mapper to KerberosPrincipa...
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2054#discussion_r110968735
--- Diff:
storm-client/src/jvm/org/apache/storm/security/auth/RegexKerberosPrincipalToLocal.java
---
@@ -0,0 +1,56 @@
+/**
+ * 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.storm.security.auth;
+
+import java.util.Map;
+import java.security.Principal;
+
+/**
+ * Map a kerberos principal to a local user
+ */
+public class RegexKerberosPrincipalToLocal extends
KerberosPrincipalToLocal {
+
+private String inputRegex;
+ private String replacementString;
+
+ /**
+ * Invoked once immediately after construction
+ * @param storm_conf Storm configuration
+ */
+public void prepare(@SuppressWarnings("rawtypes") Map storm_conf) {
+Object object = storm_conf.get("storm.principal.mapper.regex");
+if (object != null) {
+inputRegex = object.toString();
+}
+object = storm_conf.get("storm.principal.mapper.replacement");
--- End diff --
Could we put all of the configs here inside of Config.java and then tag
them with the annotations to make it a string so we don't need to call
.toString on the things passed in.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2054: STORM-2462 Adding regex mapper to KerberosPrincipa...
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2054#discussion_r110967560
--- Diff:
storm-client/src/jvm/org/apache/storm/security/auth/RegexKerberosPrincipalToLocal.java
---
@@ -0,0 +1,56 @@
+/**
+ * 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.storm.security.auth;
+
+import java.util.Map;
+import java.security.Principal;
+
+/**
+ * Map a kerberos principal to a local user
+ */
+public class RegexKerberosPrincipalToLocal extends
KerberosPrincipalToLocal {
+
+private String inputRegex;
+ private String replacementString;
--- End diff --
nit: The indentation her and in general in some of the files appears to be
off.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2047: STORM-2447: add in storm local to avoid having server on ...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2047 @HeartSaVioR could you please take a look at this? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2052: STORM-2448: Report storm and jdk versions to nimbu...
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2052 STORM-2448: Report storm and jdk versions to nimbus on topology submission 0.10.x support for #2049 You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2448-0.10.x Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2052.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 #2052 commit 77985a0756d099c379059dbef021ade4af0d4e88 Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-04-04T19:32:18Z STORM-2448: Report storm and jdk versions to nimbus on topology submission --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2051: STORM-2448: Add in Storm and JDK versions when sub...
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2051 STORM-2448: Add in Storm and JDK versions when submitting a topology 1.0.x support for #2049 You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2448-1.0.x Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2051.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 #2051 commit f3ccc05f765de9fbf9df67c3218db4d17fa76982 Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-04-04T17:34:49Z STORM-2448: Add in Storm and JDK versions when submitting a topology --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2050: STORM-2448: Add in Storm and JDK versions when sub...
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2050 STORM-2448: Add in Storm and JDK versions when submitting a topology 1.x support for #2049 You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2448-1.x Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2050.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 #2050 commit 23ed16adb7fa8d937fdd225d529658fe1a5cee2f Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-04-04T17:34:49Z STORM-2448: Add in Storm and JDK versions when submitting a topology --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2049: STORM-2448: Add in Storm and JDK versions when sub...
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2049 STORM-2448: Add in Storm and JDK versions when submitting a topology This is based off of the local mode patch #2047 once it goes in I'll rebase it again, so it is simpler to see the changes. All of the changes are in the last commit 7d1f346. I'll be putting up pull requests to the other branches so that they can submit topologies with a version in them. I have tested this with workers on the latest versions of branch-1.x, branch-1.0.x, and branch-0.10.x. I am not totally sure how useful this will be for other groups so I would like feedback if we want the UI changes to stay or not, and how much documentation should I add. Right now the docs are only in Config.java You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2448 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2049.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 #2049 commit b254ede46a25466749cd48ebd4bcb56dd791ec4a Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-04-06T18:58:41Z STORM-2447: add in storm local to avoid having server on worker classpath commit 9eced5310464f87ca52eb92b6224ef5abe531514 Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-04-06T19:50:14Z STORM-2447: Fixed compilation issue with int test commit 71dc2e019d84580fbcdd5eabf267016393324872 Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-04-07T13:31:56Z Merge branch 'apache-master' into STORM-2447 commit 7d1f346c45cb998209da7606f2600541d3cbfac7 Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-04-04T17:34:49Z STORM-2448: Add in Storm and JDK versions when submitting a topology --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2047: STORM-2447: add in storm local to avoid having ser...
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2047 STORM-2447: add in storm local to avoid having server on worker classpath You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2447 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2047.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 #2047 commit b254ede46a25466749cd48ebd4bcb56dd791ec4a Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-04-06T18:58:41Z STORM-2447: add in storm local to avoid having server on worker classpath --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2034: STORM-2441 Break down 'storm-core' to extract client (wor...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2034 +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2042: STORM-2453 Move non-connectors into the top directory
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2042 Still +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2042: STORM-2453 Move non-connectors into the top directory
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2042 Oops I missed that the build failed because of ``` [WARNING] Files with unapproved licenses: flux/flux-wrappers/src/main/resources/resources/__init__.py ``` We should fix this. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2038: STORM-2450: Write resources into correct local directory
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2038 @HeartSaVioR it actually took me way too long beating my head against it thinking I broke the thing as part of trying to make local mode transparent. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2034: STORM-2441 Break down 'storm-core' to extract client (wor...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2034 @HeartSaVioR Yes I gave these changes a +1 because they were as good as I could expect without having storm server removed completely from the storm jar classpath. To make that happen we really had to have a different way to submit a local topology. Now that I have addressed it I was able to finish removing storm server from the classpath and I am working on removing it from all of the external packages as well. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2034: STORM-2441 Break down 'storm-core' to extract client (wor...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2034 Just to let you know that I have been working on making local mode transparent. I have it working, but I have a lot of work to do on the examples and docs left. https://github.com/HeartSaVioR/storm/compare/STORM-2441...revans2:STORM-2447 To make a long story short if you run `storm local` instead of `storm jar` it will bring up local mode cluster and local mode DRPC and hide it all fairly well from the end user. Once I finish that I will start looking at working on letting a user select an alternative worker classpath. @ptgoetz and @HeartSaVioR I would like to merge this into master sooner rather then later so we don't have to keep up-merging it. Please let me know if you have any concerns about this. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2038: STORM-2450: Write resources into correct local dir...
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2038 STORM-2450: Write resources into correct local directory You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2450 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2038.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 #2038 commit 547cb108254e144bdf5a7dc0ccad95cbb7f4c4d6 Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-04-01T15:35:39Z STORM-2450: Write resources into correct local directory --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2034: STORM-2441 Break down 'storm-core' to extract client (wor...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2034 +1 it looks good to me. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2034: STORM-2441 Break down 'storm-core' to extract client (wor...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2034 I filed STORM-2447 and STORM-2448 to allow for us to separate out the classpath and make the change transparent for end users. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2034: STORM-2441 Break down 'storm-core' to extract client (wor...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2034 I am +1 for the general concept. I need to finish going through the code, but I am probably going to do it outside of github because the patch is so huge. As a side note git used to use a config to detect if a file moved. It would diff the files and if a small amount changed between the two it would consider it a move, but it was inexact and changed between clients (because the configured leniency could be different). Now I believe that it is metadata stored with the commit. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2019: STORM-2423 - Join Bolt should use explicit instead of def...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2019 +1 for the concept, would like to understand the test failures though. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2016: STORM-2422: Reduce the size of a serialized trident topol...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2016 @arunmahadevan yes this is showing up in a real topology (part of monitoring and alerting at Yahoo). And yes some of these topologies are really large. They are machine generated from a number of different user provided configurations, which is why it can grow to be so large. Having fewer big topologies instead of many small topologies makes them more efficient because they only have to read/process the metrics once. There are ways to work around it on the user side, but this felt like a cleaner solution overall. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2016: STORM-2422: Reduce the size of a serialized triden...
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2016 STORM-2422: Reduce the size of a serialized trident topology For now I only plan to do this for master. If someone else runs into this issue I am happy to backport it to other versions of storm. You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2422 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2016.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 #2016 commit 525f14f64b112cb6e73788a0e97dad56048c876d Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-03-17T18:25:27Z STORM-2422: Reduce the size of a serialized trident topology --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2014: STORM-2038: Disable symlinks with a config option (1.x)
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2014 @HeartSaVioR could you also take a look at #2015 it is almost identical, except some white space changes in nimbus.clj made it so the cherry-pick was not 100% clean. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1974: STORM-2038: Disable symlinks with a config option
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1974 @ptgoetz yes #1972 was a clean cherry pick so I pulled it into 1.x and 1.0.x. This is not so I have #2014 and #2015. The only real changes for them is that I had to translate the code in Nimbus.java to clojure and modify the many getOrDefault calls to be java 7 compatible. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2014: STORM-2038: Disable symlinks with a config option ...
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2014 STORM-2038: Disable symlinks with a config option (1.x) You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2038-sym-link-alternative-1.x Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2014.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 #2014 commit f9bfb2107ca6753c0fcf51c7381df7166eac0fd8 Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-02-27T21:06:43Z STORM-2038: Disable symlinks with a config option --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2015: STORM-2038: Disable symlinks with a config option ...
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2015 STORM-2038: Disable symlinks with a config option (1.0.x) You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2038-sym-link-alternative-1.0.x Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2015.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 #2015 commit 2aa178d7b40eeb82383799e867e33a30cffa5fea Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-02-27T21:06:43Z STORM-2038: Disable symlinks with a config option --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2013: STORM-2420: fix build
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2013 I manually ran the tests and they passed, so I am not going to wait for travis to finish. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2013: STORM-2420: fix build
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2013 STORM-2420: fix build You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2420 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2013.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 #2013 commit e57d5f3f5540e4cc7756dbf9eb428c2e2ba1826d Author: Robert (Bobby) Evans <ev...@yahoo-inc.com> Date: 2017-03-16T17:14:18Z STORM-2420: fix 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1974: STORM-2038: Disable symlinks with a config option
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1974 @HeartSaVioR I have rebased the changes to remove the conflict (it was just in the documentation). I'll start working on backporting 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
