Hey Greg, Let me just clarify for the connection manager. I don't know what's going on there, so I'm not trying to write the code there. The current code is temporary as I want to test out the concept of moving transports to an independent layer. I'm not sure how connection manager should work. So I didn't bother with correctly porting it over. (Which is why the get_http_cm exists)
Secondly, the project right now is more or less an experiment. It's for me to see what's possible without the constraint of the current API and to generate these discussions we are having. I don't have a plan of what will happen with it. I did envision pushing the code back. Right now I'm just trying out different things. Judging from what I have right now, a lot of these changes could be reintergrated. I still haven't explored things like map reduce yet, so I want to continue to explore Shuhao Sent from my phone. On Mar 16, 2012 2:07 PM, "Greg Stein" <gst...@gmail.com> wrote: > On Thu, Mar 15, 2012 at 11:13, Shuhao Wu <ad...@thekks.net> wrote: > >... > > Erlang. In my client, a chunk of the code actually comes from the > original > > client as they work with a few adaptations. > > Yes, I noticed that you retain transports/connection.py, but why did > you strip out all the comments?! That is definitely not a good path > towards a long-term maintainable codebase. I put a lot of commentary > into connection.py because there are explicit race conditions in > there. The documentation was necessary to clarify where the races > exist, what is being done about it, and why the code works properly. > With your stripped-down version, all of that information is LOST. > Future maintainers will not understand the issues and break it, or > they will think there are issues that don't really exist. I could just > see somebody saying "holy crap! in the presence of threads, this won't > work" and go and introduce a Queue in there. Totally senseless and > overkill. > > But because you stripped the code... no benefit, and probably harm. > > I'll take the somewhat-messy Basho client, over your version where you > explicitly remove useful knowledge. > > I would also note that you have ALREADY introduced bugs. Even in the > very simplest case. Consider the following: > > client = riak2.client.Client('server1') > bucket = client.bucket('test') > ob = bucket.get('key') > data = ob.get_data() > > client = riak2.client.Client('server2') > bucket = client.bucket('migrate') > bucket.new('key', data) > > Pretty simple, hm? Migrate some data from one server to another. > > It doesn't work. > > Your get_http_cm() class method is a broken idea. It is effectively a > global variable. Because the Client('server2') call does not provide a > connection manager, it "reuses" the one created for the 'server1' > connection. > > >... > > How long and how much energy are you willing to expend? Of your own > time, and of others who may want to use your code? > > Also, consider that you've turned something like bucket.get_r() into > just bucket.r. Sure, that looks good, but examine the current bucket > code: if the bucket doesn't provide a value, then it defers to the > client. These are not just simple attributes. What you really want is > to use Python's properties. That'll make them *look* like attributes, > but you can execute the fallback code. And you could do that on the > *existing* client, and benefit everybody, without introducing a bunch > of bugs by trying to do it yourself. > > You could then use riak.util.deprecated() to mark .get_r() as > deprecated. In a future revision, then you could clear them out. The > client that everybody uses would then benefit. > > > From a community/social dynamic, you're forking the project and going > your own away. That doesn't help the broader community. A few people > might use your client, but most will stick to Basho's client. It may > feel nice and fun for you, but applying your efforts to the official > client *will* help everybody. And after a round of deprecation, then > you can clear out all of the stuff you find messy. But spending some > time to do this right, and to *work with* the existing community will > produce a much larger benefit to all. > > -g >
_______________________________________________ riak-users mailing list riak-users@lists.basho.com http://lists.basho.com/mailman/listinfo/riak-users_lists.basho.com