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
