You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by Parth-Brahmbhatt <gi...@git.apache.org> on 2015/11/05 21:58:02 UTC
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
GitHub user Parth-Brahmbhatt opened a pull request:
https://github.com/apache/storm/pull/862
STORM-1098: Nimbus hook for topology actions.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Parth-Brahmbhatt/incubator-storm STORM-1098
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/862.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 #862
----
commit e49e8c52e5f8814e8ea89c6c58347913ed313fde
Author: Parth Brahmbhatt <br...@gmail.com>
Date: 2015-11-05T20:55:13Z
STORM-1098: Nimbus hook for topology actions.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by venkateshs <gi...@git.apache.org>.
Github user venkateshs commented on a diff in the pull request:
https://github.com/apache/storm/pull/862#discussion_r44706794
--- Diff: storm-core/src/jvm/backtype/storm/nimbus/ITopologyActionNotifierPlugin.java ---
@@ -0,0 +1,43 @@
+/**
+ * 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 backtype.storm.nimbus;
+
+import java.util.Map;
+
+/**
+ * A plugin interface that gets invoked any time there is an action for a topology.
+ */
+public interface ITopologyActionNotifierPlugin {
+ /**
+ * Called once during nimbus initialization.
+ * @param StormConf
+ */
+ void prepare(Map StormConf);
+
+ /**
+ * When a new actions is executed for a topology, this method will be called.
+ * @param topologyName
+ * @param action
+ */
+ void notify(String topologyName, String action);
--- End diff --
Sorry, quick question on the hook interface - Can I get the StormTopology object instead of the topologyName so I do not need to interact with Nimbus?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the pull request:
https://github.com/apache/storm/pull/862#issuecomment-154238985
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by Parth-Brahmbhatt <gi...@git.apache.org>.
Github user Parth-Brahmbhatt commented on the pull request:
https://github.com/apache/storm/pull/862#issuecomment-154580884
Upmerged. I will merge the patch in once someone from Atlas team reviews and confirms this is sufficient for 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by Parth-Brahmbhatt <gi...@git.apache.org>.
Github user Parth-Brahmbhatt commented on a diff in the pull request:
https://github.com/apache/storm/pull/862#discussion_r44189187
--- Diff: storm-core/src/clj/backtype/storm/daemon/nimbus.clj ---
@@ -814,6 +816,11 @@
(.assignSlots inimbus topologies)))
(log-message "not a leader, skipping assignments")))
+(defn notify-topology-action-listener [nimbus storm-id action]
+ (let [topology-action-notifier (:nimbus-topology-action-notifier nimbus)]
+ (when (not-nil? topology-action-notifier)
+ (.notify topology-action-notifier storm-id action))))
--- End diff --
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by Parth-Brahmbhatt <gi...@git.apache.org>.
Github user Parth-Brahmbhatt commented on a diff in the pull request:
https://github.com/apache/storm/pull/862#discussion_r44189170
--- Diff: storm-core/src/clj/backtype/storm/daemon/nimbus.clj ---
@@ -124,6 +124,8 @@
:id->sched-status (atom {})
:cred-renewers (AuthUtils/GetCredentialRenewers conf)
:nimbus-autocred-plugins (AuthUtils/getNimbusAutoCredPlugins conf)
+ :nimbus-topology-action-notifier (when-not (clojure.string/blank? (conf NIMBUS-TOPOLOGY-ACTION-NOTIFIER-PLUGIN))
+ (new-instance (conf NIMBUS-TOPOLOGY-ACTION-NOTIFIER-PLUGIN)))
--- End diff --
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by venkateshs <gi...@git.apache.org>.
Github user venkateshs commented on a diff in the pull request:
https://github.com/apache/storm/pull/862#discussion_r44347677
--- Diff: storm-core/test/jvm/backtype/storm/nimbus/InMemoryTopologyAcitonNotifier.java ---
@@ -0,0 +1,53 @@
+/**
+ * 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 backtype.storm.nimbus;
+
+import java.util.Map;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.HashMap;
+
+public class InMemoryTopologyAcitonNotifier implements ITopologyActionNotifierPlugin {
+
+ //static to ensure eventhough the class is created using reflection we can still get
+ //the topology to actions
+ private static final Map<String, LinkedList<String>> topologyToActions = new HashMap<>();
+
+
+ @Override
+ public void prepare(Map StormConf) {
+ //no-op
+ }
+
+ @Override
+ public synchronized void notify(String topologyName, String action) {
--- End diff --
In reality, Should this be executed async?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/862#discussion_r44148807
--- Diff: storm-core/src/clj/backtype/storm/daemon/nimbus.clj ---
@@ -124,6 +124,8 @@
:id->sched-status (atom {})
:cred-renewers (AuthUtils/GetCredentialRenewers conf)
:nimbus-autocred-plugins (AuthUtils/getNimbusAutoCredPlugins conf)
+ :nimbus-topology-action-notifier (when-not (clojure.string/blank? (conf NIMBUS-TOPOLOGY-ACTION-NOTIFIER-PLUGIN))
+ (new-instance (conf NIMBUS-TOPOLOGY-ACTION-NOTIFIER-PLUGIN)))
--- End diff --
we need call prepare and catch any errors so that we don't bring nimbus down.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/862#issuecomment-155226362
+1 Great
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by Parth-Brahmbhatt <gi...@git.apache.org>.
Github user Parth-Brahmbhatt commented on a diff in the pull request:
https://github.com/apache/storm/pull/862#discussion_r44373750
--- Diff: storm-core/test/jvm/backtype/storm/nimbus/InMemoryTopologyAcitonNotifier.java ---
@@ -0,0 +1,53 @@
+/**
+ * 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 backtype.storm.nimbus;
+
+import java.util.Map;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.HashMap;
+
+public class InMemoryTopologyAcitonNotifier implements ITopologyActionNotifierPlugin {
+
+ //static to ensure eventhough the class is created using reflection we can still get
+ //the topology to actions
+ private static final Map<String, LinkedList<String>> topologyToActions = new HashMap<>();
+
+
+ @Override
+ public void prepare(Map StormConf) {
+ //no-op
+ }
+
+ @Override
+ public synchronized void notify(String topologyName, String action) {
--- End diff --
This is not being invoked in critical (tuple execution) path so I think we don't really care if this method is sync. Do you anticipate huge latencies here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/862#issuecomment-154437228
The code looks fine to me +1. The CI failures look to be unrelated.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/862#discussion_r44148739
--- Diff: storm-core/src/jvm/backtype/storm/nimbus/ITopologyActionNotifierPlugin.java ---
@@ -0,0 +1,25 @@
+/**
+ * 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 backtype.storm.nimbus;
+
+/**
+ * A plugin interface that gets invoked any time there is an action for a topology.
+ */
+public interface ITopologyActionNotifierPlugin {
--- End diff --
lets have prepare() and close() for external data sources.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/862
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/862#issuecomment-155089572
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by Parth-Brahmbhatt <gi...@git.apache.org>.
Github user Parth-Brahmbhatt commented on the pull request:
https://github.com/apache/storm/pull/862#issuecomment-156209629
Venketesh did give some comments. I am going to merge this in given the comments would not block 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/862#discussion_r44346382
--- Diff: storm-core/test/jvm/backtype/storm/nimbus/InMemoryTopologyAcitonNotifier.java ---
@@ -0,0 +1,53 @@
+/**
+ * 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 backtype.storm.nimbus;
+
+import java.util.Map;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.HashMap;
+
+public class InMemoryTopologyAcitonNotifier implements ITopologyActionNotifierPlugin {
--- End diff --
Nits: typo Aciton -> Action
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/862#discussion_r44283896
--- Diff: storm-core/test/jvm/backtype/storm/nimbus/InMemoryTopologyAcitonNotifier.java ---
@@ -0,0 +1,53 @@
+/**
+ * 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 backtype.storm.nimbus;
+
+import java.util.Map;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.HashMap;
+
+public class InMemoryTopologyAcitonNotifier implements ITopologyActionNotifierPlugin {
--- End diff --
I'm not really sure that I like this. It feels like a built in memory leak. If you want to mark it as an example or for testing purposes using the comments that is fine with me, but I don't think this is something that we should consider for a production storm cluster.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by venkateshs <gi...@git.apache.org>.
Github user venkateshs commented on a diff in the pull request:
https://github.com/apache/storm/pull/862#discussion_r44347530
--- Diff: storm-core/src/jvm/backtype/storm/nimbus/ITopologyActionNotifierPlugin.java ---
@@ -0,0 +1,43 @@
+/**
+ * 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 backtype.storm.nimbus;
+
+import java.util.Map;
+
+/**
+ * A plugin interface that gets invoked any time there is an action for a topology.
+ */
+public interface ITopologyActionNotifierPlugin {
+ /**
+ * Called once during nimbus initialization.
+ * @param StormConf
+ */
+ void prepare(Map StormConf);
+
+ /**
+ * When a new actions is executed for a topology, this method will be called.
+ * @param topologyName
+ * @param action
+ */
+ void notify(String topologyName, String action);
--- End diff --
Is topologyName and topology ID the same?
Should action be an enum?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by venkateshs <gi...@git.apache.org>.
Github user venkateshs commented on a diff in the pull request:
https://github.com/apache/storm/pull/862#discussion_r44706590
--- Diff: storm-core/src/jvm/backtype/storm/nimbus/ITopologyActionNotifierPlugin.java ---
@@ -0,0 +1,43 @@
+/**
+ * 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 backtype.storm.nimbus;
+
+import java.util.Map;
+
+/**
+ * A plugin interface that gets invoked any time there is an action for a topology.
+ */
+public interface ITopologyActionNotifierPlugin {
+ /**
+ * Called once during nimbus initialization.
+ * @param StormConf
+ */
+ void prepare(Map StormConf);
+
+ /**
+ * When a new actions is executed for a topology, this method will be called.
+ * @param topologyName
+ * @param action
+ */
+ void notify(String topologyName, String action);
--- End diff --
I'm +1, thanks Parth.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by Parth-Brahmbhatt <gi...@git.apache.org>.
Github user Parth-Brahmbhatt commented on a diff in the pull request:
https://github.com/apache/storm/pull/862#discussion_r44373870
--- Diff: storm-core/test/jvm/backtype/storm/nimbus/InMemoryTopologyAcitonNotifier.java ---
@@ -0,0 +1,53 @@
+/**
+ * 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 backtype.storm.nimbus;
+
+import java.util.Map;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.HashMap;
+
+public class InMemoryTopologyAcitonNotifier implements ITopologyActionNotifierPlugin {
--- End diff --
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/862#discussion_r44283982
--- Diff: storm-core/test/jvm/backtype/storm/nimbus/InMemoryTopologyAcitonNotifier.java ---
@@ -0,0 +1,53 @@
+/**
+ * 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 backtype.storm.nimbus;
+
+import java.util.Map;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.HashMap;
+
+public class InMemoryTopologyAcitonNotifier implements ITopologyActionNotifierPlugin {
--- End diff --
Oops never mind, I just saw the "test" directory. Bobby you need to stop putting your foot in your mouth.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by Parth-Brahmbhatt <gi...@git.apache.org>.
Github user Parth-Brahmbhatt commented on a diff in the pull request:
https://github.com/apache/storm/pull/862#discussion_r44304729
--- Diff: storm-core/test/jvm/backtype/storm/nimbus/InMemoryTopologyAcitonNotifier.java ---
@@ -0,0 +1,53 @@
+/**
+ * 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 backtype.storm.nimbus;
+
+import java.util.Map;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.HashMap;
+
+public class InMemoryTopologyAcitonNotifier implements ITopologyActionNotifierPlugin {
--- End diff --
I don't like this class either. I tried just stubbing out but given the config is just the string and the actual instance is created using reflection my only option were to either stub notify-topology-action-listener, which is the method I am trying to test. I also tried stubbing nimbus-data which holds the reference to the actual instance but for one reason or another nimbus just failed to start with that stubbing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by Parth-Brahmbhatt <gi...@git.apache.org>.
Github user Parth-Brahmbhatt commented on a diff in the pull request:
https://github.com/apache/storm/pull/862#discussion_r44373716
--- Diff: storm-core/src/jvm/backtype/storm/nimbus/ITopologyActionNotifierPlugin.java ---
@@ -0,0 +1,43 @@
+/**
+ * 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 backtype.storm.nimbus;
+
+import java.util.Map;
+
+/**
+ * A plugin interface that gets invoked any time there is an action for a topology.
+ */
+public interface ITopologyActionNotifierPlugin {
+ /**
+ * Called once during nimbus initialization.
+ * @param StormConf
+ */
+ void prepare(Map StormConf);
+
+ /**
+ * When a new actions is executed for a topology, this method will be called.
+ * @param topologyName
+ * @param action
+ */
+ void notify(String topologyName, String action);
--- End diff --
They are not,are you seeing TopologyId somewhere? I will double check if I incorrectly named some variable. Id is a unique identifier which can change when a topology is killed and re-submitted where as name can remain same.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the pull request:
https://github.com/apache/storm/pull/862#issuecomment-154556254
+1. @Parth-Brahmbhatt can you upmerge. Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/862#discussion_r44148868
--- Diff: storm-core/src/clj/backtype/storm/daemon/nimbus.clj ---
@@ -814,6 +816,11 @@
(.assignSlots inimbus topologies)))
(log-message "not a leader, skipping assignments")))
+(defn notify-topology-action-listener [nimbus storm-id action]
+ (let [topology-action-notifier (:nimbus-topology-action-notifier nimbus)]
+ (when (not-nil? topology-action-notifier)
+ (.notify topology-action-notifier storm-id action))))
--- End diff --
same here notify errors should be caught and logged.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by Parth-Brahmbhatt <gi...@git.apache.org>.
Github user Parth-Brahmbhatt commented on a diff in the pull request:
https://github.com/apache/storm/pull/862#discussion_r44373855
--- Diff: storm-core/src/jvm/backtype/storm/nimbus/ITopologyActionNotifierPlugin.java ---
@@ -0,0 +1,43 @@
+/**
+ * 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 backtype.storm.nimbus;
+
+import java.util.Map;
+
+/**
+ * A plugin interface that gets invoked any time there is an action for a topology.
+ */
+public interface ITopologyActionNotifierPlugin {
+ /**
+ * Called once during nimbus initialization.
+ * @param StormConf
+ */
+ void prepare(Map StormConf);
+
+ /**
+ * When a new actions is executed for a topology, this method will be called.
+ * @param topologyName
+ * @param action
+ */
+ void notify(String topologyName, String action);
--- End diff --
I actually thought about making it an Enum but I think that will make it less flexible, no personal preference though so if you think Enum makes things easier for you to reason I can change 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1098: Nimbus hook for topology actions.
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/862#issuecomment-156171586
Still +1
@Parth-Brahmbhatt did you get any feedback from anyone on Atlas yet?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---