[GitHub] tomee pull request #218: TOMEE-2290 - Metrics timer example

2018-11-26 Thread rmannibucau
Github user rmannibucau commented on a diff in the pull request:

https://github.com/apache/tomee/pull/218#discussion_r236342694
  
--- Diff: examples/mp-metrics-timed/pom.xml ---
@@ -0,0 +1,71 @@
+
--- End diff --

header ;)


---


[GitHub] tomee pull request #218: TOMEE-2290 - Metrics timer example

2018-11-26 Thread rmannibucau
Github user rmannibucau commented on a diff in the pull request:

https://github.com/apache/tomee/pull/218#discussion_r236314041
  
--- Diff: examples/mp-metrics-timed/src/test/java/WeatherServiceTest.java 
---
@@ -0,0 +1,151 @@
+/**
+ * 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.
+ */
+
+import org.apache.cxf.jaxrs.client.WebClient;
+import org.jboss.arquillian.container.test.api.Deployment;
+import org.jboss.arquillian.junit.Arquillian;
+import org.jboss.arquillian.test.api.ArquillianResource;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.asset.StringAsset;
+import org.jboss.shrinkwrap.api.spec.WebArchive;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import javax.json.Json;
+import javax.json.JsonObject;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import java.io.StringReader;
+import java.net.URL;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(Arquillian.class)
+public class WeatherServiceTest {
+
+@Deployment(testable = false)
+public static WebArchive createDeployment() {
+final WebArchive webArchive = ShrinkWrap.create(WebArchive.class, 
"test.war")
+.addClass(WeatherService.class)
+.addAsWebInfResource(new StringAsset(""), 
"beans.xml");
+return webArchive;
+}
+
+@ArquillianResource
+private URL base;
+
+@Test
+public void testTimedMetric() {
+WebClient.create(base.toExternalForm())
--- End diff --

WebClient is a CXF API, you can use the standard JAXRS Client 
https://docs.oracle.com/javaee/7/api/javax/ws/rs/client/Client.html


---


[GitHub] tomee pull request #218: TOMEE-2290 - Metrics timer example

2018-11-26 Thread rmannibucau
Github user rmannibucau commented on a diff in the pull request:

https://github.com/apache/tomee/pull/218#discussion_r236249672
  
--- Diff: examples/mp-metrics-timed/src/test/java/WeatherServiceTest.java 
---
@@ -0,0 +1,151 @@
+/**
+ * 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.
+ */
+
+import org.apache.cxf.jaxrs.client.WebClient;
+import org.jboss.arquillian.container.test.api.Deployment;
+import org.jboss.arquillian.junit.Arquillian;
+import org.jboss.arquillian.test.api.ArquillianResource;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.asset.StringAsset;
+import org.jboss.shrinkwrap.api.spec.WebArchive;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import javax.json.Json;
+import javax.json.JsonObject;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import java.io.StringReader;
+import java.net.URL;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(Arquillian.class)
+public class WeatherServiceTest {
+
+@Deployment(testable = false)
+public static WebArchive createDeployment() {
+final WebArchive webArchive = ShrinkWrap.create(WebArchive.class, 
"test.war")
+.addClass(WeatherService.class)
+.addAsWebInfResource(new StringAsset(""), 
"beans.xml");
+return webArchive;
+}
+
+@ArquillianResource
+private URL base;
+
+@Test
+public void testTimedMetric() {
+WebClient.create(base.toExternalForm())
+.path("/weather/day/status")
+.get(String.class);
+
+final String metricPath = 
"/metrics/application/weather_day_status";
+assertPrometheusFormat(metricPath);
+assertJsonFormat(metricPath);
+}
+
+private void assertPrometheusFormat(final String metricPath) {
+final String metric = WebClient.create(base.toExternalForm())
+.path(metricPath)
+.accept(MediaType.TEXT_PLAIN)
+.get(String.class);
+
+assertTrue(metric.contains("# TYPE 
application:weather_day_status_seconds summary timer"));
--- End diff --

as a general rule put a small description and the actual value in "message" 
parameter of the assert

proposal:

 Stream.of("expected text 1", "expected text 2")
.forEach(text -> assertTrue("Expected: " + text + " to be present 
in " + metric, metric.contains(text)));

Also for an example you don't need to assert that much so just checking 
"application:weather_day_status_seconds_count 1.0" can be sufficient for the 
demonstration IMHO


---


[GitHub] tomee pull request #218: TOMEE-2290 - Metrics timer example

2018-11-26 Thread rmannibucau
Github user rmannibucau commented on a diff in the pull request:

https://github.com/apache/tomee/pull/218#discussion_r236248226
  
--- Diff: examples/mp-metrics-timed/src/test/java/WeatherServiceTest.java 
---
@@ -0,0 +1,151 @@
+/**
+ * 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.
+ */
+
+import org.apache.cxf.jaxrs.client.WebClient;
+import org.jboss.arquillian.container.test.api.Deployment;
+import org.jboss.arquillian.junit.Arquillian;
+import org.jboss.arquillian.test.api.ArquillianResource;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.asset.StringAsset;
+import org.jboss.shrinkwrap.api.spec.WebArchive;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import javax.json.Json;
+import javax.json.JsonObject;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import java.io.StringReader;
+import java.net.URL;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(Arquillian.class)
+public class WeatherServiceTest {
+
+@Deployment(testable = false)
+public static WebArchive createDeployment() {
+final WebArchive webArchive = ShrinkWrap.create(WebArchive.class, 
"test.war")
+.addClass(WeatherService.class)
+.addAsWebInfResource(new StringAsset(""), 
"beans.xml");
+return webArchive;
+}
+
+@ArquillianResource
+private URL base;
+
+@Test
+public void testTimedMetric() {
+WebClient.create(base.toExternalForm())
--- End diff --

maybe use Client (jaxrs) instead of a cxf api - without forgetting to close 
the client ;)


---


[GitHub] tomee pull request #218: TOMEE-2290 - Metrics timer example

2018-11-26 Thread rmannibucau
Github user rmannibucau commented on a diff in the pull request:

https://github.com/apache/tomee/pull/218#discussion_r236248396
  
--- Diff: examples/mp-metrics-timed/src/test/java/WeatherServiceTest.java 
---
@@ -0,0 +1,151 @@
+/**
+ * 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.
+ */
+
+import org.apache.cxf.jaxrs.client.WebClient;
+import org.jboss.arquillian.container.test.api.Deployment;
+import org.jboss.arquillian.junit.Arquillian;
+import org.jboss.arquillian.test.api.ArquillianResource;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.asset.StringAsset;
+import org.jboss.shrinkwrap.api.spec.WebArchive;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import javax.json.Json;
+import javax.json.JsonObject;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import java.io.StringReader;
+import java.net.URL;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(Arquillian.class)
+public class WeatherServiceTest {
+
+@Deployment(testable = false)
+public static WebArchive createDeployment() {
+final WebArchive webArchive = ShrinkWrap.create(WebArchive.class, 
"test.war")
+.addClass(WeatherService.class)
+.addAsWebInfResource(new StringAsset(""), 
"beans.xml");
+return webArchive;
+}
+
+@ArquillianResource
+private URL base;
+
+@Test
+public void testTimedMetric() {
+WebClient.create(base.toExternalForm())
+.path("/weather/day/status")
+.get(String.class);
+
+final String metricPath = 
"/metrics/application/weather_day_status";
+assertPrometheusFormat(metricPath);
+assertJsonFormat(metricPath);
+}
+
+private void assertPrometheusFormat(final String metricPath) {
+final String metric = WebClient.create(base.toExternalForm())
+.path(metricPath)
+.accept(MediaType.TEXT_PLAIN)
+.get(String.class);
+
+assertTrue(metric.contains("# TYPE 
application:weather_day_status_seconds summary timer"));
+assertTrue(metric.contains("# TYPE 
application:weather_day_status_seconds_count timer"));
+
assertTrue(metric.contains("application:weather_day_status_seconds_count 1.0"));
+assertTrue(metric.contains("# TYPE 
application:weather_day_status_rate_per_second timer"));
+
assertTrue(metric.contains("application:weather_day_status_rate_per_second"));
+assertTrue(metric.contains("# TYPE 
application:weather_day_status_one_min_rate_per_second timer"));
+
assertTrue(metric.contains("application:weather_day_status_one_min_rate_per_second"));
+assertTrue(metric.contains("# TYPE 
application:weather_day_status_five_min_rate_per_second timer"));
+
assertTrue(metric.contains("application:weather_day_status_five_min_rate_per_second"));
+assertTrue(metric.contains("# TYPE 
application:weather_day_status_fifteen_min_rate_per_second time"));
+
assertTrue(metric.contains("application:weather_day_status_fifteen_min_rate_per_second"));
+assertTrue(metric.contains("# TYPE 
application:weather_day_status_min_seconds timer"));
+
assertTrue(metric.contains("application:weather_day_status_min_seconds"));
+assertTrue(metric.contains("# TYPE 
application:weather_day_status_max_seconds timer"));
+
assertTrue(metric.contains("application:weather_day_status_max_seconds"));
+assertTrue(metric.contains("# TYPE 
application:weather_day_status_mean_seconds timer"));
+

[GitHub] tomee pull request #218: TOMEE-2290 - Metrics timer example

2018-11-26 Thread rmannibucau
Github user rmannibucau commented on a diff in the pull request:

https://github.com/apache/tomee/pull/218#discussion_r236248868
  
--- Diff: examples/mp-metrics-timed/src/test/java/WeatherServiceTest.java 
---
@@ -0,0 +1,151 @@
+/**
+ * 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.
+ */
+
+import org.apache.cxf.jaxrs.client.WebClient;
+import org.jboss.arquillian.container.test.api.Deployment;
+import org.jboss.arquillian.junit.Arquillian;
+import org.jboss.arquillian.test.api.ArquillianResource;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.asset.StringAsset;
+import org.jboss.shrinkwrap.api.spec.WebArchive;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import javax.json.Json;
+import javax.json.JsonObject;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import java.io.StringReader;
+import java.net.URL;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(Arquillian.class)
+public class WeatherServiceTest {
+
+@Deployment(testable = false)
+public static WebArchive createDeployment() {
+final WebArchive webArchive = ShrinkWrap.create(WebArchive.class, 
"test.war")
+.addClass(WeatherService.class)
+.addAsWebInfResource(new StringAsset(""), 
"beans.xml");
+return webArchive;
+}
+
+@ArquillianResource
+private URL base;
+
+@Test
+public void testTimedMetric() {
+WebClient.create(base.toExternalForm())
+.path("/weather/day/status")
+.get(String.class);
+
+final String metricPath = 
"/metrics/application/weather_day_status";
+assertPrometheusFormat(metricPath);
+assertJsonFormat(metricPath);
+}
+
+private void assertPrometheusFormat(final String metricPath) {
+final String metric = WebClient.create(base.toExternalForm())
+.path(metricPath)
+.accept(MediaType.TEXT_PLAIN)
+.get(String.class);
+
+assertTrue(metric.contains("# TYPE 
application:weather_day_status_seconds summary timer"));
+assertTrue(metric.contains("# TYPE 
application:weather_day_status_seconds_count timer"));
+
assertTrue(metric.contains("application:weather_day_status_seconds_count 1.0"));
+assertTrue(metric.contains("# TYPE 
application:weather_day_status_rate_per_second timer"));
+
assertTrue(metric.contains("application:weather_day_status_rate_per_second"));
+assertTrue(metric.contains("# TYPE 
application:weather_day_status_one_min_rate_per_second timer"));
+
assertTrue(metric.contains("application:weather_day_status_one_min_rate_per_second"));
+assertTrue(metric.contains("# TYPE 
application:weather_day_status_five_min_rate_per_second timer"));
+
assertTrue(metric.contains("application:weather_day_status_five_min_rate_per_second"));
+assertTrue(metric.contains("# TYPE 
application:weather_day_status_fifteen_min_rate_per_second time"));
+
assertTrue(metric.contains("application:weather_day_status_fifteen_min_rate_per_second"));
+assertTrue(metric.contains("# TYPE 
application:weather_day_status_min_seconds timer"));
+
assertTrue(metric.contains("application:weather_day_status_min_seconds"));
+assertTrue(metric.contains("# TYPE 
application:weather_day_status_max_seconds timer"));
+
assertTrue(metric.contains("application:weather_day_status_max_seconds"));
+assertTrue(metric.contains("# TYPE 
application:weather_day_status_mean_seconds timer"));
+

[GitHub] tomee issue #180: Apache BVal - BV certification feedback

2018-10-30 Thread rmannibucau
Github user rmannibucau commented on the issue:

https://github.com/apache/tomee/pull/180
  
@gsmet updated master to pass the tck with bval 2.0.1-SNAPSHOT, do you care 
rebasing your PR and checking it works for you?


---


[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...

2018-10-25 Thread rmannibucau
Github user rmannibucau commented on the issue:

https://github.com/apache/tomee/pull/176
  
Thnking to users having a java -cp command launching Cli (we have a few).

Side note: i think you still close the classloader before it is finished to 
be used - after cli execution pby


---


[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...

2018-10-24 Thread rmannibucau
Github user rmannibucau commented on the issue:

https://github.com/apache/tomee/pull/176
  
@danielsoro if you change the classloader it can impact the users relying 
on that (it can be the case if they tune java or use system classloader only 
libs. This is rare but already saw it. This is why i think it can be worth not 
changing the default behavior.


---


[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...

2018-10-23 Thread rmannibucau
Github user rmannibucau commented on the issue:

https://github.com/apache/tomee/pull/176
  
Hmm, you close the classloader before it is used so it is no more usable 
normally - or it will leak.

Side question: any issue having an impl os ClassPath using the classloader 
to keep existing one for java 8 users?


---


[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...

2018-10-23 Thread rmannibucau
Github user rmannibucau commented on the issue:

https://github.com/apache/tomee/pull/176
  
Hope i didnt misread the diff (starts to be late ;)) but it misses a close 
and the urlclassloader cast will not work on all java versions right?


---


[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...

2018-10-23 Thread rmannibucau
Github user rmannibucau commented on the issue:

https://github.com/apache/tomee/pull/176
  
Yes, this is what i had in mind as well. That said if it is just for our 
CLI your hack is fine, we should just lock it for other cases like 
openejb-standalone


---


[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...

2018-10-23 Thread rmannibucau
Github user rmannibucau commented on the issue:

https://github.com/apache/tomee/pull/176
  
well some thoughts:

1. most of the time we dont need this dynamism so maybe we can just rework 
the distro and delete it
2. we already hack equals/hashcode in cxf container loader but it is not 
super robust (this is why we have a flag to disable it) so maybe not the best 
solution but current impl clearly breaks some apps
3. you can open java modules to get the URLClassPath, even of the system 
classloader in j11 so maybe we just document how to open it and replace the 
cast by some reflection?




---


[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...

2018-10-10 Thread rmannibucau
Github user rmannibucau commented on the issue:

https://github.com/apache/tomee/pull/176
  
Java 6 didnt have that method but we should close all our classloaders once 
yes.


---


[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...

2018-10-10 Thread rmannibucau
Github user rmannibucau commented on the issue:

https://github.com/apache/tomee/pull/176
  
@danielsoro classloader.close() must be called to not leak and enable to 
read files after the end of the container.


---


[GitHub] tomee pull request #175: [tomee-7.0.x][TOMEE-2249] Fix EclipseLink NPE on in...

2018-10-08 Thread rmannibucau
Github user rmannibucau commented on a diff in the pull request:

https://github.com/apache/tomee/pull/175#discussion_r223478552
  
--- Diff: 
container/openejb-jpa-integration/src/main/java/org/apache/openejb/jpa/integration/eclipselink/OpenEJBServerPlatform.java
 ---
@@ -26,13 +27,14 @@
 import javax.transaction.Synchronization;
 import javax.transaction.TransactionManager;
 
-public class OpenEJBServerPlatform extends JMXServerPlatformBase {
+public class OpenEJBServerPlatform extends JMXServerPlatformBase 
implements JMXEnabledPlatform {
 public OpenEJBServerPlatform(final DatabaseSession newDatabaseSession) 
{
 super(newDatabaseSession);
 try {
 mBeanServer = MBeanServer.class.cast(
 
OpenEJBServerPlatform.class.getClassLoader().loadClass("org.apache.openejb.monitoring.LocalMBeanServer")
 .getMethod("get").invoke(null));
+this.prepareServerSpecificServicesMBean();
 } catch (final Exception e) {
 // no-op
--- End diff --

Fail instead of noop?


---


[GitHub] tomee pull request #175: [tomee-7.0.x][TOMEE-2249] Fix EclipseLink NPE on in...

2018-10-08 Thread rmannibucau
Github user rmannibucau commented on a diff in the pull request:

https://github.com/apache/tomee/pull/175#discussion_r223478337
  
--- Diff: 
container/openejb-jpa-integration/src/main/java/org/apache/openejb/jpa/integration/eclipselink/MBeanOpenEJBRuntimeServices.java
 ---
@@ -0,0 +1,26 @@
+/*
+ * 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.openejb.jpa.integration.eclipselink;
+
+import org.eclipse.persistence.internal.sessions.AbstractSession;
+import org.eclipse.persistence.sessions.Session;
+
+public class MBeanOpenEJBRuntimeServices extends OpenEJBRuntimeServices 
implements MBeanOpenEJBRuntimeServicesMBean {
--- End diff --

Was more thinking about dropping this and keeping only 
OpenEJBRuntimeServices


---


[GitHub] tomee pull request #175: [tomee-7.0.x][TOMEE-2249] Fix EclipseLink NPE on in...

2018-09-30 Thread rmannibucau
Github user rmannibucau commented on a diff in the pull request:

https://github.com/apache/tomee/pull/175#discussion_r221449894
  
--- Diff: 
container/openejb-jpa-integration/src/main/java/org/apache/openejb/jpa/integration/eclipselink/OpenEJBServerPlatform.java
 ---
@@ -63,4 +65,11 @@ protected void 
registerSynchronization_impl(AbstractSynchronizationListener list
 transaction.registerInterposedSynchronization(synchronization);
 }
 }
+
+@Override
+public void prepareServerSpecificServicesMBean() {
+if (getDatabaseSession() != null && shouldRegisterRuntimeBean) 
{
+ this.setRuntimeServicesMBean(new 
MBeanOpenEJBRuntimeServices(getDatabaseSession()));
--- End diff --

ensure it uses mBeanServer which has a toggle "skip jmx" on the instance


---


[GitHub] tomee pull request #175: [tomee-7.0.x][TOMEE-2249] Fix EclipseLink NPE on in...

2018-09-30 Thread rmannibucau
Github user rmannibucau commented on a diff in the pull request:

https://github.com/apache/tomee/pull/175#discussion_r221449834
  
--- Diff: 
container/openejb-jpa-integration/src/main/java/org/apache/openejb/jpa/integration/eclipselink/MBeanOpenEJBRuntimeServices.java
 ---
@@ -0,0 +1,26 @@
+/*
+ * 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.openejb.jpa.integration.eclipselink;
+
+import org.eclipse.persistence.internal.sessions.AbstractSession;
+import org.eclipse.persistence.sessions.Session;
+
+public class MBeanOpenEJBRuntimeServices extends OpenEJBRuntimeServices 
implements MBeanOpenEJBRuntimeServicesMBean {
--- End diff --

ideally we would have a single impl, cant we reuse one existing impl since 
they do nothing here?

only PLATFORM_NAME is set and it is not really needed since only an error 
thing


---


[GitHub] tomee issue #171: [Master Branch] TOMEE-2240 ManagedScheduledExecutorService...

2018-09-26 Thread rmannibucau
Github user rmannibucau commented on the issue:

https://github.com/apache/tomee/pull/171
  
No issue, thanks for taking time to PR and recheck it.


---


[GitHub] tomee issue #171: [Master Branch] TOMEE-2240 ManagedScheduledExecutorService...

2018-09-26 Thread rmannibucau
Github user rmannibucau commented on the issue:

https://github.com/apache/tomee/pull/171
  
Isnt 5s too small?
Also, dont forget to add new params to service-jar.xml in openejb-core.
Otherwise looks good!


---