You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Manikumar Reddy O <ma...@gmail.com> on 2015/06/25 18:24:24 UTC
Review Request 35880: Patch for KAFKA-2295
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35880/
-----------------------------------------------------------
Review request for kafka.
Bugs: KAFKA-2295
https://issues.apache.org/jira/browse/KAFKA-2295
Repository: kafka
Description
-------
Implemente the approach given in JIRA description. This is to support dynamically loaded envirornments
Diffs
-----
clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java bae528d31516679bed88ee61b408f209f185a8cc
clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 4170bcc7def5b50d8aa20e8e84089c35b705b527
clients/src/main/java/org/apache/kafka/common/utils/Utils.java af9993cf9b3991f1e61e1201c94e19bc1bf76a68
core/src/main/scala/kafka/utils/CoreUtils.scala 168a18d380c200ee566eccb6988dd1ae85ed5b09
Diff: https://reviews.apache.org/r/35880/diff/
Testing
-------
Thanks,
Manikumar Reddy O
Re: Review Request 35880: Patch for KAFKA-2295
Posted by Guozhang Wang <wa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35880/#review91337
-----------------------------------------------------------
Ship it!
Ship It!
- Guozhang Wang
On July 6, 2015, 6:05 a.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35880/
> -----------------------------------------------------------
>
> (Updated July 6, 2015, 6:05 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-2295
> https://issues.apache.org/jira/browse/KAFKA-2295
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Addessing Guozhang's comments
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java bae528d31516679bed88ee61b408f209f185a8cc
> clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 4170bcc7def5b50d8aa20e8e84089c35b705b527
> clients/src/main/java/org/apache/kafka/common/utils/Utils.java af9993cf9b3991f1e61e1201c94e19bc1bf76a68
> clients/src/test/java/org/apache/kafka/common/config/AbstractConfigTest.java db1b0ee9113215b5ad7fda0f93915f3bdd34ac55
> core/src/main/scala/kafka/utils/CoreUtils.scala 168a18d380c200ee566eccb6988dd1ae85ed5b09
>
> Diff: https://reviews.apache.org/r/35880/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 35880: Patch for KAFKA-2295
Posted by Guozhang Wang <wa...@gmail.com>.
> On July 6, 2015, 3:50 p.m., Guozhang Wang wrote:
> > core/src/main/scala/kafka/utils/CoreUtils.scala, line 221
> > <https://reviews.apache.org/r/35880/diff/2/?file=999581#file999581line221>
> >
> > Can we use scala's Utils.createObject here?
>
> Manikumar Reddy O wrote:
> Are you sugessting to use Utils.newInstance()? We dont have scala's Utils.scala. Am I missing something?
My bad, this is not a issue.
- Guozhang
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35880/#review90488
-----------------------------------------------------------
On July 6, 2015, 6:05 a.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35880/
> -----------------------------------------------------------
>
> (Updated July 6, 2015, 6:05 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-2295
> https://issues.apache.org/jira/browse/KAFKA-2295
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Addessing Guozhang's comments
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java bae528d31516679bed88ee61b408f209f185a8cc
> clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 4170bcc7def5b50d8aa20e8e84089c35b705b527
> clients/src/main/java/org/apache/kafka/common/utils/Utils.java af9993cf9b3991f1e61e1201c94e19bc1bf76a68
> clients/src/test/java/org/apache/kafka/common/config/AbstractConfigTest.java db1b0ee9113215b5ad7fda0f93915f3bdd34ac55
> core/src/main/scala/kafka/utils/CoreUtils.scala 168a18d380c200ee566eccb6988dd1ae85ed5b09
>
> Diff: https://reviews.apache.org/r/35880/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 35880: Patch for KAFKA-2295
Posted by Manikumar Reddy O <ma...@gmail.com>.
> On July 6, 2015, 3:50 p.m., Guozhang Wang wrote:
> > clients/src/test/java/org/apache/kafka/common/config/AbstractConfigTest.java, line 57
> > <https://reviews.apache.org/r/35880/diff/2/?file=999580#file999580line57>
> >
> > Is this intentional?
yes. Now ClassNotFoundException will be thrown from Utils.newInstance() as KafkaException.
> On July 6, 2015, 3:50 p.m., Guozhang Wang wrote:
> > clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java, line 266
> > <https://reviews.apache.org/r/35880/diff/2/?file=999578#file999578line266>
> >
> > Can we use Utils.newInstance here as well?
Here we only need Class Object.
> On July 6, 2015, 3:50 p.m., Guozhang Wang wrote:
> > core/src/main/scala/kafka/utils/CoreUtils.scala, line 221
> > <https://reviews.apache.org/r/35880/diff/2/?file=999581#file999581line221>
> >
> > Can we use scala's Utils.createObject here?
Are you sugessting to use Utils.newInstance()? We dont have scala's Utils.scala. Am I missing something?
- Manikumar Reddy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35880/#review90488
-----------------------------------------------------------
On July 6, 2015, 6:05 a.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35880/
> -----------------------------------------------------------
>
> (Updated July 6, 2015, 6:05 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-2295
> https://issues.apache.org/jira/browse/KAFKA-2295
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Addessing Guozhang's comments
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java bae528d31516679bed88ee61b408f209f185a8cc
> clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 4170bcc7def5b50d8aa20e8e84089c35b705b527
> clients/src/main/java/org/apache/kafka/common/utils/Utils.java af9993cf9b3991f1e61e1201c94e19bc1bf76a68
> clients/src/test/java/org/apache/kafka/common/config/AbstractConfigTest.java db1b0ee9113215b5ad7fda0f93915f3bdd34ac55
> core/src/main/scala/kafka/utils/CoreUtils.scala 168a18d380c200ee566eccb6988dd1ae85ed5b09
>
> Diff: https://reviews.apache.org/r/35880/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 35880: Patch for KAFKA-2295
Posted by Guozhang Wang <wa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35880/#review90488
-----------------------------------------------------------
clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java (line 266)
<https://reviews.apache.org/r/35880/#comment143588>
Can we use Utils.newInstance here as well?
clients/src/test/java/org/apache/kafka/common/config/AbstractConfigTest.java (line 57)
<https://reviews.apache.org/r/35880/#comment143589>
Is this intentional?
core/src/main/scala/kafka/utils/CoreUtils.scala (line 221)
<https://reviews.apache.org/r/35880/#comment143590>
Can we use scala's Utils.createObject here?
- Guozhang Wang
On July 6, 2015, 6:05 a.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35880/
> -----------------------------------------------------------
>
> (Updated July 6, 2015, 6:05 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-2295
> https://issues.apache.org/jira/browse/KAFKA-2295
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Addessing Guozhang's comments
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java bae528d31516679bed88ee61b408f209f185a8cc
> clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 4170bcc7def5b50d8aa20e8e84089c35b705b527
> clients/src/main/java/org/apache/kafka/common/utils/Utils.java af9993cf9b3991f1e61e1201c94e19bc1bf76a68
> clients/src/test/java/org/apache/kafka/common/config/AbstractConfigTest.java db1b0ee9113215b5ad7fda0f93915f3bdd34ac55
> core/src/main/scala/kafka/utils/CoreUtils.scala 168a18d380c200ee566eccb6988dd1ae85ed5b09
>
> Diff: https://reviews.apache.org/r/35880/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 35880: Patch for KAFKA-2295
Posted by Manikumar Reddy O <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35880/
-----------------------------------------------------------
(Updated Aug. 20, 2015, 12:17 p.m.)
Review request for kafka.
Bugs: KAFKA-2295
https://issues.apache.org/jira/browse/KAFKA-2295
Repository: kafka
Description (updated)
-------
Code Rebase
Diffs (updated)
-----
clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java 156ec14c9c099448cc1d8347191eef4c02591faa
clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 4170bcc7def5b50d8aa20e8e84089c35b705b527
clients/src/main/java/org/apache/kafka/common/utils/Utils.java c58b74144ba01f04ec17d8ca7bd3d231fd6fb3a8
clients/src/test/java/org/apache/kafka/common/config/AbstractConfigTest.java db1b0ee9113215b5ad7fda0f93915f3bdd34ac55
core/src/main/scala/kafka/utils/CoreUtils.scala 168a18d380c200ee566eccb6988dd1ae85ed5b09
Diff: https://reviews.apache.org/r/35880/diff/
Testing
-------
Thanks,
Manikumar Reddy O
Re: Review Request 35880: Patch for KAFKA-2295
Posted by Manikumar Reddy O <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35880/
-----------------------------------------------------------
(Updated July 6, 2015, 6:05 a.m.)
Review request for kafka.
Bugs: KAFKA-2295
https://issues.apache.org/jira/browse/KAFKA-2295
Repository: kafka
Description (updated)
-------
Addessing Guozhang's comments
Diffs (updated)
-----
clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java bae528d31516679bed88ee61b408f209f185a8cc
clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 4170bcc7def5b50d8aa20e8e84089c35b705b527
clients/src/main/java/org/apache/kafka/common/utils/Utils.java af9993cf9b3991f1e61e1201c94e19bc1bf76a68
clients/src/test/java/org/apache/kafka/common/config/AbstractConfigTest.java db1b0ee9113215b5ad7fda0f93915f3bdd34ac55
core/src/main/scala/kafka/utils/CoreUtils.scala 168a18d380c200ee566eccb6988dd1ae85ed5b09
Diff: https://reviews.apache.org/r/35880/diff/
Testing
-------
Thanks,
Manikumar Reddy O
Re: Review Request 35880: Patch for KAFKA-2295
Posted by Manikumar Reddy O <ma...@gmail.com>.
> On July 3, 2015, 1:46 a.m., Guozhang Wang wrote:
> > clients/src/main/java/org/apache/kafka/common/utils/Utils.java, line 512
> > <https://reviews.apache.org/r/35880/diff/1/?file=992171#file992171line512>
> >
> > I think thread.getContextClassLoader() will never return null:
> >
> > http://docs.oracle.com/javase/7/docs/api/java/lang/Thread.html#getContextClassLoader()
I agree. javadoc says "Returns: the context ClassLoader for this Thread, or null indicating the system class loader (or, failing that, the bootstrap class loader)". I added this code for completeness.
> On July 3, 2015, 1:46 a.m., Guozhang Wang wrote:
> > clients/src/main/java/org/apache/kafka/common/utils/Utils.java, line 496
> > <https://reviews.apache.org/r/35880/diff/1/?file=992171#file992171line496>
> >
> > How about add a createObject function in Utils.java like coreUtils.scala and let other java code calls it?
we already have Utils.newInstance() method. If required, we can add createObject() method.
- Manikumar Reddy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35880/#review90305
-----------------------------------------------------------
On June 25, 2015, 4:25 p.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35880/
> -----------------------------------------------------------
>
> (Updated June 25, 2015, 4:25 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-2295
> https://issues.apache.org/jira/browse/KAFKA-2295
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Implemented the approach given in JIRA description. This is to support dynamically loaded envirornments
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java bae528d31516679bed88ee61b408f209f185a8cc
> clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 4170bcc7def5b50d8aa20e8e84089c35b705b527
> clients/src/main/java/org/apache/kafka/common/utils/Utils.java af9993cf9b3991f1e61e1201c94e19bc1bf76a68
> core/src/main/scala/kafka/utils/CoreUtils.scala 168a18d380c200ee566eccb6988dd1ae85ed5b09
>
> Diff: https://reviews.apache.org/r/35880/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 35880: Patch for KAFKA-2295
Posted by Guozhang Wang <wa...@gmail.com>.
> On July 3, 2015, 1:46 a.m., Guozhang Wang wrote:
> > clients/src/main/java/org/apache/kafka/common/utils/Utils.java, line 496
> > <https://reviews.apache.org/r/35880/diff/1/?file=992171#file992171line496>
> >
> > How about add a createObject function in Utils.java like coreUtils.scala and let other java code calls it?
>
> Manikumar Reddy O wrote:
> we already have Utils.newInstance() method. If required, we can add createObject() method.
How about adding a
public static Object newInstance(String className)
alsong with
public static Object newInstance(Class<?> c)
and make all references of Class.forName in java code calling this function?
- Guozhang
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35880/#review90305
-----------------------------------------------------------
On June 25, 2015, 4:25 p.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35880/
> -----------------------------------------------------------
>
> (Updated June 25, 2015, 4:25 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-2295
> https://issues.apache.org/jira/browse/KAFKA-2295
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Implemented the approach given in JIRA description. This is to support dynamically loaded envirornments
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java bae528d31516679bed88ee61b408f209f185a8cc
> clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 4170bcc7def5b50d8aa20e8e84089c35b705b527
> clients/src/main/java/org/apache/kafka/common/utils/Utils.java af9993cf9b3991f1e61e1201c94e19bc1bf76a68
> core/src/main/scala/kafka/utils/CoreUtils.scala 168a18d380c200ee566eccb6988dd1ae85ed5b09
>
> Diff: https://reviews.apache.org/r/35880/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 35880: Patch for KAFKA-2295
Posted by Guozhang Wang <wa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35880/#review90305
-----------------------------------------------------------
clients/src/main/java/org/apache/kafka/common/utils/Utils.java (line 496)
<https://reviews.apache.org/r/35880/#comment143323>
How about add a createObject function in Utils.java like coreUtils.scala and let other java code calls it?
clients/src/main/java/org/apache/kafka/common/utils/Utils.java (line 512)
<https://reviews.apache.org/r/35880/#comment143322>
I think thread.getContextClassLoader() will never return null:
http://docs.oracle.com/javase/7/docs/api/java/lang/Thread.html#getContextClassLoader()
- Guozhang Wang
On June 25, 2015, 4:25 p.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35880/
> -----------------------------------------------------------
>
> (Updated June 25, 2015, 4:25 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-2295
> https://issues.apache.org/jira/browse/KAFKA-2295
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Implemented the approach given in JIRA description. This is to support dynamically loaded envirornments
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java bae528d31516679bed88ee61b408f209f185a8cc
> clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 4170bcc7def5b50d8aa20e8e84089c35b705b527
> clients/src/main/java/org/apache/kafka/common/utils/Utils.java af9993cf9b3991f1e61e1201c94e19bc1bf76a68
> core/src/main/scala/kafka/utils/CoreUtils.scala 168a18d380c200ee566eccb6988dd1ae85ed5b09
>
> Diff: https://reviews.apache.org/r/35880/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 35880: Patch for KAFKA-2295
Posted by Manikumar Reddy O <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35880/
-----------------------------------------------------------
(Updated June 25, 2015, 4:25 p.m.)
Review request for kafka.
Bugs: KAFKA-2295
https://issues.apache.org/jira/browse/KAFKA-2295
Repository: kafka
Description (updated)
-------
Implemented the approach given in JIRA description. This is to support dynamically loaded envirornments
Diffs
-----
clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java bae528d31516679bed88ee61b408f209f185a8cc
clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 4170bcc7def5b50d8aa20e8e84089c35b705b527
clients/src/main/java/org/apache/kafka/common/utils/Utils.java af9993cf9b3991f1e61e1201c94e19bc1bf76a68
core/src/main/scala/kafka/utils/CoreUtils.scala 168a18d380c200ee566eccb6988dd1ae85ed5b09
Diff: https://reviews.apache.org/r/35880/diff/
Testing
-------
Thanks,
Manikumar Reddy O