Thanks Matthieu.
-- Yes ZKClient has lot of helper methods and makes coding easier. It does
have some bugs but nothing major. I had contacted the author for fixes but
he did not respond, so i had to write a ZkClient extension so that I can
add additional functionality and I can fix any bugs.
We need testUtils for starting and stopping a node, this will be useful for
testing distributed mode but starting/stopping Zk lets use Zkclient
-- Sorry dint pay attention to junit version, I wanted to suggest moving to
testNG.
-- java.io.tmpdir is fine. will do that change.
-- I am fine with any convention, it helps having same package name.
-- yes I was referring to 0.5 version.
If functionality is ok, then I will integrate with the emitter and listener.
thanks,
Kishore G
On Sun, Oct 23, 2011 at 9:40 AM, Matthieu Morel <mmo...@apache.org> wrote:
> Hi Kishore,
>
> This looks great!
>
> I'll plan to use the zookeeper integration in some tests at the application
> level, i.e. in the core subproject (probably extending existing ones). I'll
> try to provide more feedback then.
>
> A few comments:
>
> - zkclient: I've noticed you are using an external client library for
> avoiding quite a bit of boilerplate code when using zookeeper. I had
> already
> added things to facilitate usage of zookeeper in core/TestUtils, but if
> zkclient really "makes life easier", as it says, and if you recommend using
> it, I'll refactor my own code to use that client. Looks nice!
>
> - junit version: it seems you are using the api from junit3. I think it's
> better to use junit 4, since there is already such a dependency in existing
> tests. There are also more features available.
>
> - zookeeper data directories in tests: you seem to be doing like myself in
> my initialization code for zookeeper fixtures, i.e. using directories in
> {user.dir}/tmp . Leo suggested to use java.io.tmpdir... so we should both
> use that directory!
>
> - test package names: I started using prefixing by test.s4, in order to
> avoid any confusion with other classes from code. I'm ok for prefixing
> tests
> with org.apache.s4, but I think it should at least be org.apache.s4.test.
> Which naming scheme do we choose?
>
> - The design is very similar to what we had in earlier version of s4. There
> will be more changes on this in the next release.
> --> you are referring to release 0.5 right?
>
> Matthieu
>
> On Sat, Oct 22, 2011 at 7:31 PM, kishore g <g.kish...@gmail.com> wrote:
>
> > Hi,
> >
> > I pushed in the initial code for s4-zk integration for s4-piper.
> >
> > https://github.com/leoneu/s4-piper/compare/17a2547499...9ba3013a01
> >
> > Please review and provide feedback.
> >
> > The design is very similar to what we had in earlier version of s4. There
> > will be more changes on this in the next release.
> >
> > thanks,
> > Kishore G
> >
>
