I created ZOOKEEPER-1410 for C client as sub-task of 1407. Let me try adding support for GetChildren.
I may need some advice on adding unit test. Cheers On Thu, Mar 8, 2012 at 4:47 PM, Marshall McMullen < [email protected]> wrote: > So as to avoid not holding up your very appreciated work on 1407, how about > you create a separate jira case to add support to the C client for > supporting GetData in a Multi. > > Perhaps GetChildren should be captured as another jira case as well since > you didn't handle that in your patch. Up to you... > > On Thu, Mar 8, 2012 at 5:34 PM, Ted Yu <[email protected]> wrote: > > > I re-attached patch according to Ted's feedback. > > > > I am just trying to help push this issue forward. > > As Marshall pointed out, it would take sometime for me to handle the > > changes in C client. > > So I am willing to let Marshall or someone else take over. > > > > Before that happens, I will continue on > https://reviews.apache.org/r/4264/ > > . > > > > Cheers > > > > On Thu, Mar 8, 2012 at 4:20 PM, Marshall McMullen < > > [email protected]> wrote: > > > > > Obviously this patch also neglected to add support to the C client > > (though > > > honestly that's a lot harder than the java side). If you don't plan to > do > > > the C client, then I can pick up that work. > > > > > > On Thu, Mar 8, 2012 at 5:10 PM, Ted Dunning <[email protected]> > > wrote: > > > > > > > Looks like a cut and paste error in the first line of the GetData > > > > constructor: > > > > > > > > + super(ZooDefs.OpCode.setData, path); > > > > > > > > > > > > Perhaps you meant ZooDefs.OpCode.getData here? > > > > > > > > The check for permissions also seems wrong here: > > > > > > > > > > > > + checkACL(zks, nodeRecord.acl, ZooDefs.Perms.WRITE, > > > > request.authInfo); > > > > > > > > > > > > As far as really reviewing this, it would really help to have it in > > > > reviewboard instead of just reading a diff. > > > > > > > > Also, this only seems to handle getData and doesn't seem to include > any > > > > tests. Is the omission of getChildren intentional? Do you plan to > > have > > > > tests? > > > > > > > > On Thu, Mar 8, 2012 at 3:54 PM, Ted Yu <[email protected]> wrote: > > > > > > > > > Please take a look at the patch I attached to ZOOKEEPER-1407. > > > > > > > > > > Cheers > > > > > > > > > > On Thu, Mar 8, 2012 at 2:52 PM, Ted Dunning <[email protected] > > > > > > wrote: > > > > > > > > > > > Yes. > > > > > > > > > > > > It should be straightforward. There are just a number of places > to > > > > > touch. > > > > > > You need a factory in Op, a sub-class to hold the transaction, > > then > > > on > > > > > the > > > > > > server side there are 2-4 switch statements that need to be > > > inspected. > > > > > > Marshal can comment whether the commit code needs change, but I > > > would > > > > > > expect it. > > > > > > > > > > > > On Thu, Mar 8, 2012 at 2:39 PM, Ted Yu <[email protected]> > > wrote: > > > > > > > > > > > > > There are GetDataRequest / GetDataResponse in > src/zookeeper.jute > > > > > > > > > > > > > > Would the new GetDataTxn be able to reuse them ? > > > > > > > > > > > > > > Thanks > > > > > > > > > > > > > > On Wed, Mar 7, 2012 at 8:06 AM, Ted Yu <[email protected]> > > > wrote: > > > > > > > > > > > > > > > https://issues.apache.org/jira/browse/ZOOKEEPER-1407 has > been > > > > > logged. > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Mar 6, 2012 at 10:41 PM, Ted Dunning < > > > > [email protected] > > > > > > > >wrote: > > > > > > > > > > > > > > > >> The use cases that I came up with for multi all involved > reads > > > > done > > > > > > > before > > > > > > > >> multiple writes as with a read-modify-write operation. In > > that > > > > > case, > > > > > > > the > > > > > > > >> reads need to be done before the updates. Those use cases > > > > motivated > > > > > > the > > > > > > > >> Check operation so that you can ensure that nothing has > > changed > > > > > before > > > > > > > the > > > > > > > >> updates are done. > > > > > > > >> > > > > > > > >> But as Marshal says, there isn't any reason not to have > them. > > > > > > > >> > > > > > > > >> On Tue, Mar 6, 2012 at 10:10 PM, Ted Yu < > [email protected]> > > > > > wrote: > > > > > > > >> > > > > > > > >> > On first glance, Op doesn't seem to cover getData(). > > > > > > > >> > > > > > > > > >> > Is this intentional ? > > > > > > > >> > > > > > > > > >> > How do I read the data for multiple znodes in one > > transaction > > > ? > > > > > > > >> > > > > > > > > >> > Thanks > > > > > > > >> > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
