[GitHub] metron pull request #607: METRON-982 add new rest api for storm supervisor s...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/metron/pull/607


---
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] metron pull request #607: METRON-982 add new rest api for storm supervisor s...

[ottobackwards]

Github user ottobackwards commented on a diff in the pull request:

https://github.com/apache/metron/pull/607#discussion_r129946569
  
--- Diff: 
metron-interface/metron-rest/src/test/java/org/apache/metron/rest/controller/StormControllerIntegrationTest.java
 ---
@@ -182,6 +185,16 @@ public void test() throws Exception {
 .andExpect(jsonPath("$.status").value("SUCCESS"))
 
.andExpect(jsonPath("$.message").value(TopologyStatusCode.STARTED.name()));
 
+this.mockMvc.perform(get(stormUrl + 
"/supervisors").with(httpBasic(user,password)))
+.andExpect(status().isOk())
+
.andExpect(content().contentType(MediaType.parseMediaType("application/json;charset=UTF-8")))
+.andExpect(jsonPath("$.supervisors[0]").exists())
+.andExpect(jsonPath("$.supervisors[0].id").exists())
+.andExpect(jsonPath("$.supervisors[0].host").exists())
+.andExpect(jsonPath("$.supervisors[0].upTime").exists())
--- End diff --

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] metron pull request #607: METRON-982 add new rest api for storm supervisor s...

[merrimanr]

Github user merrimanr commented on a diff in the pull request:

https://github.com/apache/metron/pull/607#discussion_r129930004
  
--- Diff: 
metron-interface/metron-rest/src/test/java/org/apache/metron/rest/controller/StormControllerIntegrationTest.java
 ---
@@ -182,6 +185,16 @@ public void test() throws Exception {
 .andExpect(jsonPath("$.status").value("SUCCESS"))
 
.andExpect(jsonPath("$.message").value(TopologyStatusCode.STARTED.name()));
 
+this.mockMvc.perform(get(stormUrl + 
"/supervisors").with(httpBasic(user,password)))
+.andExpect(status().isOk())
+
.andExpect(content().contentType(MediaType.parseMediaType("application/json;charset=UTF-8")))
+.andExpect(jsonPath("$.supervisors[0]").exists())
+.andExpect(jsonPath("$.supervisors[0].id").exists())
+.andExpect(jsonPath("$.supervisors[0].host").exists())
+.andExpect(jsonPath("$.supervisors[0].upTime").exists())
--- End diff --

I changed the getter/setter methods in my test.  You can try leaving them 
the same but I know "getUptime()" works.


---
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] metron pull request #607: METRON-982 add new rest api for storm supervisor s...

[ottobackwards]

Github user ottobackwards commented on a diff in the pull request:

https://github.com/apache/metron/pull/607#discussion_r129929213
  
--- Diff: 
metron-interface/metron-rest/src/test/java/org/apache/metron/rest/controller/StormControllerIntegrationTest.java
 ---
@@ -182,6 +185,16 @@ public void test() throws Exception {
 .andExpect(jsonPath("$.status").value("SUCCESS"))
 
.andExpect(jsonPath("$.message").value(TopologyStatusCode.STARTED.name()));
 
+this.mockMvc.perform(get(stormUrl + 
"/supervisors").with(httpBasic(user,password)))
+.andExpect(status().isOk())
+
.andExpect(content().contentType(MediaType.parseMediaType("application/json;charset=UTF-8")))
+.andExpect(jsonPath("$.supervisors[0]").exists())
+.andExpect(jsonPath("$.supervisors[0].id").exists())
+.andExpect(jsonPath("$.supervisors[0].host").exists())
+.andExpect(jsonPath("$.supervisors[0].upTime").exists())
--- End diff --

ok, i don't know how that got formatted to upTime, I have a comment in 
there that I copied out of that page to go by.

The getter and setter names can stay though?  getUpTime() 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] metron pull request #607: METRON-982 add new rest api for storm supervisor s...

[merrimanr]

Github user merrimanr commented on a diff in the pull request:

https://github.com/apache/metron/pull/607#discussion_r129926344
  
--- Diff: 
metron-interface/metron-rest-client/src/main/java/org/apache/metron/rest/model/SupervisorStatus.java
 ---
@@ -0,0 +1,127 @@
+/**
+ * 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.metron.rest.model;
+
+public class SupervisorStatus {
+  /*
+/api/v1/supervisor/summary (GET)
+returns all supervisors summary
+
+Response Fields:
+
+Field  Value   Description
+id String  Supervisor's id
+host   String  Supervisor's host name
+upTime String  Shows how long the supervisor is running
+slotsTotal Integer Total number of available worker slots for this 
supervisor
+slotsUsed  Integer Number of worker slots used on this supervisor
+Sample Response:
+json { "supervisors": [ { "id": 
"0b879808-2a26-442b-8f7d-23101e0c3696", "host": "10.11.1.7", "upTime": "5m 
58s", "slotsTotal": 4, "slotsUsed": 3 } ] }
+
+  */
+  private String id;
+  private String host;
+  private String upTime;
--- End diff --

When I tested this in full dev with Swagger, the upTime field in the 
response was null.  Here's an example response when I hit 
http://node1:8744/api/v1/supervisor/summary:
```

{"supervisors":[{"totalMem":3072.0,"host":"node1","id":"70bd7170-160f-4215-8f18-d9088993fb47","uptime":"47m
 
20s","totalCpu":400.0,"usedCpu":0.0,"logLink":"http:\/\/node1:8000\/daemonlog?file=supervisor.log","usedMem":3328.0,"slotsUsed":4,"version":"1.0.1.2.5.6.0-40","slotsTotal":4,"uptimeSeconds":2840}],"schedulerDisplayResource":false,"logviewerPort":8000}
```
Notice the field is actually "uptime".  I changed the upTime variable name 
and getter/setting methods and that fixed it.  

The StormControllerIntegrationTest uses a mock Storm REST API so you're not 
going to catch it there.  You would only catch this by hitting a real Storm 
REST API.


---
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] metron pull request #607: METRON-982 add new rest api for storm supervisor s...

[ottobackwards]

Github user ottobackwards commented on a diff in the pull request:

https://github.com/apache/metron/pull/607#discussion_r129923353
  
--- Diff: 
metron-interface/metron-rest/src/test/java/org/apache/metron/rest/controller/StormControllerIntegrationTest.java
 ---
@@ -182,6 +185,16 @@ public void test() throws Exception {
 .andExpect(jsonPath("$.status").value("SUCCESS"))
 
.andExpect(jsonPath("$.message").value(TopologyStatusCode.STARTED.name()));
 
+this.mockMvc.perform(get(stormUrl + 
"/supervisors").with(httpBasic(user,password)))
+.andExpect(status().isOk())
+
.andExpect(content().contentType(MediaType.parseMediaType("application/json;charset=UTF-8")))
+.andExpect(jsonPath("$.supervisors[0]").exists())
+.andExpect(jsonPath("$.supervisors[0].id").exists())
+.andExpect(jsonPath("$.supervisors[0].host").exists())
+.andExpect(jsonPath("$.supervisors[0].upTime").exists())
--- End diff --

I don't understand, the API spec says it is upTime


---
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] metron pull request #607: METRON-982 add new rest api for storm supervisor s...

[ottobackwards]

Github user ottobackwards commented on a diff in the pull request:

https://github.com/apache/metron/pull/607#discussion_r129920992
  
--- Diff: 
metron-interface/metron-rest-client/src/main/java/org/apache/metron/rest/model/SupervisorStatus.java
 ---
@@ -0,0 +1,127 @@
+/**
+ * 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.metron.rest.model;
+
+public class SupervisorStatus {
+  /*
+/api/v1/supervisor/summary (GET)
+returns all supervisors summary
+
+Response Fields:
+
+Field  Value   Description
+id String  Supervisor's id
+host   String  Supervisor's host name
+upTime String  Shows how long the supervisor is running
+slotsTotal Integer Total number of available worker slots for this 
supervisor
+slotsUsed  Integer Number of worker slots used on this supervisor
+Sample Response:
+json { "supervisors": [ { "id": 
"0b879808-2a26-442b-8f7d-23101e0c3696", "host": "10.11.1.7", "upTime": "5m 
58s", "slotsTotal": 4, "slotsUsed": 3 } ] }
+
+  */
+  private String id;
+  private String host;
+  private String upTime;
+  private int slotsTotal;
+  private int slotsUsed;
+
+  public SupervisorStatus(){}
+  public SupervisorStatus(String id, String host, String upTime, int 
slotsTotal, int slotsUsed) {
+this.id = id;
+this.host = host;
+this.upTime = upTime;
+this.slotsTotal = slotsTotal;
+this.slotsUsed = slotsUsed;
+  }
+
+  public String getId() {
+return id;
+  }
+
+  public void setId(String id) {
+this.id = id;
+  }
+
+  public String getHost() {
+return host;
+  }
+
+  public void setHost(String host) {
+this.host = host;
+  }
+
+  public String getUpTime() {
+return upTime;
+  }
+
+  public void setUpTime(String upTime) {
+this.upTime = upTime;
+  }
+
+  public int getSlotsTotal() {
+return slotsTotal;
+  }
+
+  public void setSlotsTotal(int slotsTotal) {
+this.slotsTotal = slotsTotal;
+  }
+
+  public int getSlotsUsed() {
+return slotsUsed;
+  }
+
+  public void setSlotsUsed(int slotsUsed) {
+this.slotsUsed = slotsUsed;
+  }
+
+  @Override
+  public boolean equals(Object o) {
+if (this == o) {
+  return true;
+}
+if (o == null || getClass() != o.getClass()) {
+  return false;
+}
+
+SupervisorStatus that = (SupervisorStatus) o;
+
+if (getSlotsTotal() != that.getSlotsTotal()) {
+  return false;
+}
+if (getSlotsUsed() != that.getSlotsUsed()) {
+  return false;
+}
+if (!getId().equals(that.getId())) {
+  return false;
+}
+if (!getHost().equals(that.getHost())) {
+  return false;
+}
+return getUpTime().equals(that.getUpTime());
+  }
+
+  @Override
+  public int hashCode() {
+int result = id != null? id.hashCode() : 0;
+result = 31 * result + host != null? host.hashCode() : 0;
--- End diff --

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] metron pull request #607: METRON-982 add new rest api for storm supervisor s...

[ottobackwards]

Github user ottobackwards commented on a diff in the pull request:

https://github.com/apache/metron/pull/607#discussion_r129920967
  
--- Diff: 
metron-interface/metron-rest-client/src/main/java/org/apache/metron/rest/model/SupervisorStatus.java
 ---
@@ -0,0 +1,127 @@
+/**
+ * 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.metron.rest.model;
+
+public class SupervisorStatus {
+  /*
+/api/v1/supervisor/summary (GET)
+returns all supervisors summary
+
+Response Fields:
+
+Field  Value   Description
+id String  Supervisor's id
+host   String  Supervisor's host name
+upTime String  Shows how long the supervisor is running
+slotsTotal Integer Total number of available worker slots for this 
supervisor
+slotsUsed  Integer Number of worker slots used on this supervisor
+Sample Response:
+json { "supervisors": [ { "id": 
"0b879808-2a26-442b-8f7d-23101e0c3696", "host": "10.11.1.7", "upTime": "5m 
58s", "slotsTotal": 4, "slotsUsed": 3 } ] }
+
+  */
+  private String id;
+  private String host;
+  private String upTime;
--- End diff --

Where would I see that?  Am I missing a 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] metron pull request #607: METRON-982 add new rest api for storm supervisor s...

[ottobackwards]

Github user ottobackwards commented on a diff in the pull request:

https://github.com/apache/metron/pull/607#discussion_r129908796
  
--- Diff: 
metron-interface/metron-rest/src/main/java/org/apache/metron/rest/service/impl/StormStatusServiceImpl.java
 ---
@@ -17,10 +17,7 @@
  */
 package org.apache.metron.rest.service.impl;
 
-import org.apache.metron.rest.model.TopologyResponse;
-import org.apache.metron.rest.model.TopologyStatus;
-import org.apache.metron.rest.model.TopologyStatusCode;
-import org.apache.metron.rest.model.TopologySummary;
+import org.apache.metron.rest.model.*;
--- End diff --

Not to interrupt you guys

I'm OK with changing this I have tried to setup intellij to not do this 
( and I am already checking style ), but I think this PR came before I set 
things up. 

--- OK, GO!


---
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] metron pull request #607: METRON-982 add new rest api for storm supervisor s...

[justinleet]

Github user justinleet commented on a diff in the pull request:

https://github.com/apache/metron/pull/607#discussion_r129904369
  
--- Diff: 
metron-interface/metron-rest/src/main/java/org/apache/metron/rest/service/impl/StormStatusServiceImpl.java
 ---
@@ -17,10 +17,7 @@
  */
 package org.apache.metron.rest.service.impl;
 
-import org.apache.metron.rest.model.TopologyResponse;
-import org.apache.metron.rest.model.TopologyStatus;
-import org.apache.metron.rest.model.TopologyStatusCode;
-import org.apache.metron.rest.model.TopologySummary;
+import org.apache.metron.rest.model.*;
--- End diff --

@merrimanr @mattf-horton We did not have an official vote.  Given the 
widespread acceptance of it, since it's not a dev guildelines and not a bylaw 
thing, my impression was we didn't need it. Having said that, if it needs or 
should have an actual vote, I'm definitely okay with it and it should be done 
before https://github.com/apache/metron/pull/577 goes in.

However, 577 is still outstanding pending additional feedback I requested.  
I was inclined to not update the wiki, because we hadn't built out and 
validated the tooling at the time.  I personally lean towards updating the wiki 
once we're good with that PR going 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] metron pull request #607: METRON-982 add new rest api for storm supervisor s...

[merrimanr]

Github user merrimanr commented on a diff in the pull request:

https://github.com/apache/metron/pull/607#discussion_r129897123
  
--- Diff: 
metron-interface/metron-rest/src/main/java/org/apache/metron/rest/service/impl/StormStatusServiceImpl.java
 ---
@@ -17,10 +17,7 @@
  */
 package org.apache.metron.rest.service.impl;
 
-import org.apache.metron.rest.model.TopologyResponse;
-import org.apache.metron.rest.model.TopologyStatus;
-import org.apache.metron.rest.model.TopologyStatusCode;
-import org.apache.metron.rest.model.TopologySummary;
+import org.apache.metron.rest.model.*;
--- End diff --

Thanks for checking me on this @mattf-horton.  I was under the impression 
we already voted to switch our coding standard to Google's based on the [Code 
Style](http://mail-archives.apache.org/mod_mbox/metron-dev/201705.mbox/%3CCADu=u3a23q-aehhim6r0acezapaj9qjvg8hnurhr2hkdtvx...@mail.gmail.com%3E)
 discussion on the dev list back in the beginning of May.  There is also a 
pending [PR](https://github.com/apache/metron/pull/577) that sets up Checkstyle 
with the Google Code Style.

Was this not an official vote or was the wiki just not updated?  If we did 
switch, [Google Coding 
Style](https://google.github.io/styleguide/javaguide.html#s3.3-import-statements)
 does not allow wildcard imports.

This is probably one of those topics with no clear correct answer, similar 
to spaces vs tabs.  I personally am not a fan of wildcard statements because I 
prefer to see exactly what is being imported.  



---
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] metron pull request #607: METRON-982 add new rest api for storm supervisor s...

[mattf-horton]

Github user mattf-horton commented on a diff in the pull request:

https://github.com/apache/metron/pull/607#discussion_r129718460
  
--- Diff: 
metron-interface/metron-rest/src/main/java/org/apache/metron/rest/service/impl/StormStatusServiceImpl.java
 ---
@@ -17,10 +17,7 @@
  */
 package org.apache.metron.rest.service.impl;
 
-import org.apache.metron.rest.model.TopologyResponse;
-import org.apache.metron.rest.model.TopologyStatus;
-import org.apache.metron.rest.model.TopologyStatusCode;
-import org.apache.metron.rest.model.TopologySummary;
+import org.apache.metron.rest.model.*;
--- End diff --

@merrimanr , heh, you made me go look this up!

As far as I can see, our coding standards, both 
[here](https://cwiki.apache.org/confluence/display/METRON/Development+Guidelines#DevelopmentGuidelines-2.2CodeStyle)
 and [here](http://www.oracle.com/technetwork/java/codeconvtoc-136057.html) are 
silent on this topic.
There is a good discussion of the pros and cons 
[here](https://stackoverflow.com/questions/147454/why-is-using-a-wild-card-with-a-java-import-statement-bad).
 Despite the latter being a fairly old article, google did not show me any more 
recent definitive policy statements.

I must admit that I think the "wildcard imports are evil" side have 
slightly more weight in the argument, yet I personally find long lists of 
explicit imports annoying and appreciate IntelliJ's default settings (more than 
5 imports of the same package collapses to wildcard).  Can you provide cites 
that support your assertion regarding "most coding standards"?

If you are so moved, you might bring this up on the mailing list, perhaps 
providing the citations above, and propose a vote to modify the Coding Style.


---
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] metron pull request #607: METRON-982 add new rest api for storm supervisor s...

[merrimanr]

Github user merrimanr commented on a diff in the pull request:

https://github.com/apache/metron/pull/607#discussion_r129692618
  
--- Diff: 
metron-interface/metron-rest/src/test/java/org/apache/metron/rest/controller/StormControllerIntegrationTest.java
 ---
@@ -182,6 +185,16 @@ public void test() throws Exception {
 .andExpect(jsonPath("$.status").value("SUCCESS"))
 
.andExpect(jsonPath("$.message").value(TopologyStatusCode.STARTED.name()));
 
+this.mockMvc.perform(get(stormUrl + 
"/supervisors").with(httpBasic(user,password)))
+.andExpect(status().isOk())
+
.andExpect(content().contentType(MediaType.parseMediaType("application/json;charset=UTF-8")))
+.andExpect(jsonPath("$.supervisors[0]").exists())
+.andExpect(jsonPath("$.supervisors[0].id").exists())
+.andExpect(jsonPath("$.supervisors[0].host").exists())
+.andExpect(jsonPath("$.supervisors[0].upTime").exists())
--- End diff --

This also needs to be "uptime"


---
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] metron pull request #607: METRON-982 add new rest api for storm supervisor s...

[merrimanr]

Github user merrimanr commented on a diff in the pull request:

https://github.com/apache/metron/pull/607#discussion_r129691439
  
--- Diff: 
metron-interface/metron-rest-client/src/main/java/org/apache/metron/rest/model/SupervisorStatus.java
 ---
@@ -0,0 +1,127 @@
+/**
+ * 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.metron.rest.model;
+
+public class SupervisorStatus {
+  /*
+/api/v1/supervisor/summary (GET)
+returns all supervisors summary
+
+Response Fields:
+
+Field  Value   Description
+id String  Supervisor's id
+host   String  Supervisor's host name
+upTime String  Shows how long the supervisor is running
+slotsTotal Integer Total number of available worker slots for this 
supervisor
+slotsUsed  Integer Number of worker slots used on this supervisor
+Sample Response:
+json { "supervisors": [ { "id": 
"0b879808-2a26-442b-8f7d-23101e0c3696", "host": "10.11.1.7", "upTime": "5m 
58s", "slotsTotal": 4, "slotsUsed": 3 } ] }
+
+  */
+  private String id;
+  private String host;
+  private String upTime;
+  private int slotsTotal;
+  private int slotsUsed;
+
+  public SupervisorStatus(){}
+  public SupervisorStatus(String id, String host, String upTime, int 
slotsTotal, int slotsUsed) {
+this.id = id;
+this.host = host;
+this.upTime = upTime;
+this.slotsTotal = slotsTotal;
+this.slotsUsed = slotsUsed;
+  }
+
+  public String getId() {
+return id;
+  }
+
+  public void setId(String id) {
+this.id = id;
+  }
+
+  public String getHost() {
+return host;
+  }
+
+  public void setHost(String host) {
+this.host = host;
+  }
+
+  public String getUpTime() {
+return upTime;
+  }
+
+  public void setUpTime(String upTime) {
+this.upTime = upTime;
+  }
+
+  public int getSlotsTotal() {
+return slotsTotal;
+  }
+
+  public void setSlotsTotal(int slotsTotal) {
+this.slotsTotal = slotsTotal;
+  }
+
+  public int getSlotsUsed() {
+return slotsUsed;
+  }
+
+  public void setSlotsUsed(int slotsUsed) {
+this.slotsUsed = slotsUsed;
+  }
+
+  @Override
+  public boolean equals(Object o) {
+if (this == o) {
+  return true;
+}
+if (o == null || getClass() != o.getClass()) {
+  return false;
+}
+
+SupervisorStatus that = (SupervisorStatus) o;
+
+if (getSlotsTotal() != that.getSlotsTotal()) {
+  return false;
+}
+if (getSlotsUsed() != that.getSlotsUsed()) {
+  return false;
+}
+if (!getId().equals(that.getId())) {
+  return false;
+}
+if (!getHost().equals(that.getHost())) {
+  return false;
+}
+return getUpTime().equals(that.getUpTime());
+  }
+
+  @Override
+  public int hashCode() {
+int result = id != null? id.hashCode() : 0;
+result = 31 * result + host != null? host.hashCode() : 0;
--- End diff --

I get a warning in Intellij.  Should there be parenthesis around `host != 
null? host.hashCode() : 0`, similar to other classes in metron-rest-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] metron pull request #607: METRON-982 add new rest api for storm supervisor s...

[merrimanr]

Github user merrimanr commented on a diff in the pull request:

https://github.com/apache/metron/pull/607#discussion_r129690884
  
--- Diff: 
metron-interface/metron-rest-client/src/main/java/org/apache/metron/rest/model/SupervisorStatus.java
 ---
@@ -0,0 +1,127 @@
+/**
+ * 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.metron.rest.model;
+
+public class SupervisorStatus {
+  /*
+/api/v1/supervisor/summary (GET)
+returns all supervisors summary
+
+Response Fields:
+
+Field  Value   Description
+id String  Supervisor's id
+host   String  Supervisor's host name
+upTime String  Shows how long the supervisor is running
+slotsTotal Integer Total number of available worker slots for this 
supervisor
+slotsUsed  Integer Number of worker slots used on this supervisor
+Sample Response:
+json { "supervisors": [ { "id": 
"0b879808-2a26-442b-8f7d-23101e0c3696", "host": "10.11.1.7", "upTime": "5m 
58s", "slotsTotal": 4, "slotsUsed": 3 } ] }
+
+  */
+  private String id;
+  private String host;
+  private String upTime;
--- End diff --

This needs to be "uptime" instead of "upTime" or jackson doesn't map it 
correctly.


---
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] metron pull request #607: METRON-982 add new rest api for storm supervisor s...

[ottobackwards]

Github user ottobackwards commented on a diff in the pull request:

https://github.com/apache/metron/pull/607#discussion_r122703811
  
--- Diff: 
metron-interface/metron-rest/src/main/java/org/apache/metron/rest/service/impl/StormStatusServiceImpl.java
 ---
@@ -49,6 +44,11 @@ public StormStatusServiceImpl(Environment environment, 
RestTemplate restTemplate
   }
 
   @Override
+  public SupervisorSummary getSupervisorSummary(){
+return restTemplate.getForObject("http://; + 
environment.getProperty(STORM_UI_SPRING_PROPERTY) + SUPERVISOR_SUMMARY_URL, 
SupervisorSummary.class);
--- End diff --

Thanks!  I'll hook that up


---
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] metron pull request #607: METRON-982 add new rest api for storm supervisor s...

[justinleet]

Github user justinleet commented on a diff in the pull request:

https://github.com/apache/metron/pull/607#discussion_r122700117
  
--- Diff: 
metron-interface/metron-rest/src/main/java/org/apache/metron/rest/service/impl/StormStatusServiceImpl.java
 ---
@@ -49,6 +44,11 @@ public StormStatusServiceImpl(Environment environment, 
RestTemplate restTemplate
   }
 
   @Override
+  public SupervisorSummary getSupervisorSummary(){
+return restTemplate.getForObject("http://; + 
environment.getProperty(STORM_UI_SPRING_PROPERTY) + SUPERVISOR_SUMMARY_URL, 
SupervisorSummary.class);
--- End diff --

Should have pointed this out the other day, but that PR is in now, so this 
probably should be:
```
return restTemplate.getForObject(getStormUiProperty() + 
SUPERVISOR_SUMMARY_URL, SupervisorSummary.class);
```

Ping me if there's any issues and I'm happy to do more than just glance at 
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] metron pull request #607: METRON-982 add new rest api for storm supervisor s...

[justinleet]

Github user justinleet commented on a diff in the pull request:

https://github.com/apache/metron/pull/607#discussion_r120367143
  
--- Diff: 
metron-interface/metron-rest/src/main/java/org/apache/metron/rest/service/impl/StormStatusServiceImpl.java
 ---
@@ -49,6 +44,11 @@ public StormStatusServiceImpl(Environment environment, 
RestTemplate restTemplate
   }
 
   @Override
+  public SupervisorSummary getSupervisorSummary(){
+return restTemplate.getForObject("http://; + 
environment.getProperty(STORM_UI_SPRING_PROPERTY) + SUPERVISOR_SUMMARY_URL, 
SupervisorSummary.class);
--- End diff --

https://github.com/apache/metron/pull/602 changes how that works slightly.  
Whichever one goes in second will need to adjust for the other one (which 
shouldn't be much work either way).  I don't have a preference on ordering, 
personally.


---
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] metron pull request #607: METRON-982 add new rest api for storm supervisor s...

[cestella]

Github user cestella commented on a diff in the pull request:

https://github.com/apache/metron/pull/607#discussion_r120106798
  
--- Diff: 
metron-interface/metron-rest/src/main/java/org/apache/metron/rest/service/impl/StormStatusServiceImpl.java
 ---
@@ -49,6 +44,11 @@ public StormStatusServiceImpl(Environment environment, 
RestTemplate restTemplate
   }
 
   @Override
+  public SupervisorSummary getSupervisorSummary(){
+return restTemplate.getForObject("http://; + 
environment.getProperty(STORM_UI_SPRING_PROPERTY) + SUPERVISOR_SUMMARY_URL, 
SupervisorSummary.class);
--- End diff --

What if the storm UI is https?


---
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] metron pull request #607: METRON-982 add new rest api for storm supervisor s...

[ottobackwards]

GitHub user ottobackwards opened a pull request:

https://github.com/apache/metron/pull/607

METRON-982 add new rest api for storm supervisor status

This pr adds a new rest endpoint for retrieving storm supervisor summary 
information from the "/api/v1/supervisor/summary" storm ui rest endpoint.

This information contains the currently configured slots for each 
supervisor, and how many are used.

This is in support of METRON-981, which will be able to use this endpoint 
to enable errors and warnings based on the lack of available slots.

###TESTING

In full_dev environment execute the new endpoint from swagger, you should 
see the supervisor information from storm

/supervisors  off of the storm controller


### For all changes:
- [ x] Is there a JIRA ticket associated with this PR? If not one needs to 
be created at [Metron 
Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel).
 
- [ x] Does your PR title start with METRON- where  is the JIRA 
number you are trying to resolve? Pay particular attention to the hyphen "-" 
character.
- [ x] Has your PR been rebased against the latest commit within the target 
branch (typically master)?


### For code changes:
- [NA ] Have you included steps to reproduce the behavior or problem that 
is being changed or addressed?
- [ x ] Have you included steps or a guide to how the change may be 
verified and tested manually?
- [ x ] Have you ensured that the full suite of tests and checks have been 
executed in the root incubating-metron folder via:
  ```
  mvn -q clean integration-test install && build_utils/verify_licenses.sh 
  ```

- [ x ] Have you written or updated unit tests and or integration tests to 
verify your changes?
- [ NA ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)? 
- [ x ] Have you verified the basic functionality of the build by building 
and running locally with Vagrant full-dev environment or the equivalent?




You can merge this pull request into a Git repository by running:

$ git pull https://github.com/ottobackwards/metron METRON-982

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/metron/pull/607.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 #607


commit 1593a0fe0fe03ba8f51cd1f711854bdae6a234ab
Author: Otto Fowler 
Date:   2017-06-03T15:22:01Z

add new rest api for supervisor status

commit 5337d8f294bbe8df739070a49187fc3e4dc711b7
Author: Otto Fowler 
Date:   2017-06-03T16:22:31Z

add asf headers




---
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.
---