Re: RFR : 8131061 - Use of -Dcom.sun.management.snmp needs to be examined for modules

2016-08-25 Thread Mandy Chung
> On Aug 24, 2016, at 11:40 PM, Harsha Wardhana B > wrote: > > Hi All, > > Please review modified webrev located at, > > http://cr.openjdk.java.net/~hb/8131061/webrev.02/ +1 Mandy

Re: RFR : 8131061 - Use of -Dcom.sun.management.snmp needs to be examined for modules

2016-08-25 Thread Harsha Wardhana B
Thanks for the review Daniel. -Harsha On Thursday 25 August 2016 02:15 PM, Daniel Fuchs wrote: On 25/08/16 07:40, Harsha Wardhana B wrote: Hi All, Please review modified webrev located at, http://cr.openjdk.java.net/~hb/8131061/webrev.02/ Regards Harsha Hi Harsha, Looks good to me! be

Re: RFR : 8131061 - Use of -Dcom.sun.management.snmp needs to be examined for modules

2016-08-25 Thread Daniel Fuchs
On 25/08/16 07:40, Harsha Wardhana B wrote: Hi All, Please review modified webrev located at, http://cr.openjdk.java.net/~hb/8131061/webrev.02/ Regards Harsha Hi Harsha, Looks good to me! best regards, -- daniel On Wednesday 24 August 2016 11:28 AM, Harsha Wardhana B wrote: On Wed

Re: RFR : 8131061 - Use of -Dcom.sun.management.snmp needs to be examined for modules

2016-08-24 Thread Harsha Wardhana B
Hi All, Please review modified webrev located at, http://cr.openjdk.java.net/~hb/8131061/webrev.02/ Regards Harsha On Wednesday 24 August 2016 11:28 AM, Harsha Wardhana B wrote: On Wednesday 24 August 2016 01:10 AM, Mandy Chung wrote: On Aug 23, 2016, at 8:21 AM, Daniel Fuchs wrote: O

Re: RFR : 8131061 - Use of -Dcom.sun.management.snmp needs to be examined for modules

2016-08-23 Thread Harsha Wardhana B
On Wednesday 24 August 2016 01:10 AM, Mandy Chung wrote: On Aug 23, 2016, at 8:21 AM, Daniel Fuchs wrote: On 23/08/16 15:45, Harsha Wardhana B wrote: Hi Daniel, The focus of this issue was to decouple hard-dependency between java.management module and jdk.snmp module since jdk.snmp is not a

Re: RFR : 8131061 - Use of -Dcom.sun.management.snmp needs to be examined for modules

2016-08-23 Thread Mandy Chung
> On Aug 23, 2016, at 8:21 AM, Daniel Fuchs wrote: > > On 23/08/16 15:45, Harsha Wardhana B wrote: >> Hi Daniel, >> >> The focus of this issue was to decouple hard-dependency between >> java.management module and jdk.snmp module since jdk.snmp is not a core >> module. There was not much focus o

Re: RFR : 8131061 - Use of -Dcom.sun.management.snmp needs to be examined for modules

2016-08-23 Thread Daniel Fuchs
On 23/08/16 15:45, Harsha Wardhana B wrote: Hi Daniel, The focus of this issue was to decouple hard-dependency between java.management module and jdk.snmp module since jdk.snmp is not a core module. There was not much focus on an 'Agent' Interface that could allow plugging in any Agent. Hence th

Re: RFR : 8131061 - Use of -Dcom.sun.management.snmp needs to be examined for modules

2016-08-23 Thread Harsha Wardhana B
Hi Daniel, The focus of this issue was to decouple hard-dependency between java.management module and jdk.snmp module since jdk.snmp is not a core module. There was not much focus on an 'Agent' Interface that could allow plugging in any Agent. Hence there was no discussions around the structu

Re: RFR : 8131061 - Use of -Dcom.sun.management.snmp needs to be examined for modules

2016-08-23 Thread Daniel Fuchs
Hi, On 22/08/16 22:58, Mandy Chung wrote: sun/management/spi/AgentProvider.java 78 public abstract void startAgent(String port, Properties props); The port parameter should be “int”. I wonder about that. I wonder if the name of the agent provider should be the name of the property that

Re: RFR : 8131061 - Use of -Dcom.sun.management.snmp needs to be examined for modules

2016-08-22 Thread Mandy Chung
Hi Harsha, This looks reasonable. Some comments: sun/management/Agent.java 567ServiceLoader providers = ServiceLoader.load(AgentProvider.class); It should call ServiceLoader::loadInstalled since it should only load the providers from the platform modules. You can also simplify line

Re: RFR : 8131061 - Use of -Dcom.sun.management.snmp needs to be examined for modules

2016-08-21 Thread Harsha Wardhana B
Hello All, Please find revised webrev located at, http://cr.openjdk.java.net/~hb/8131061/webrev.01/ The new patch has below changes. 1. AgentProvider is made abstract class. 2. Loading SNMP Agent provider is done in a AccessController.doPreviliged block. Thanks Harsha On Friday 12 August 2

RFR : 8131061 - Use of -Dcom.sun.management.snmp needs to be examined for modules

2016-08-11 Thread Harsha Wardhana B
Hi All, Please review fix for issue, JDK-8131061 - Use of -Dcom.sun.management.snmp needs to be examined for modules with webrev located at, http://cr.openjdk.java.net/~hb/8131061/webrev.00/ Regards Harsha