Python style question

[John A Meinel]

Martin Pool wrote:
> Since we seem to have some experienced pythonists here:
> 
> I have a factory, find_branch, which can return instances of Branch or
> RemoteBranch depending on a parameter.  (Perhaps it should return
> Branches which contain a BranchStorage of various types, but ignore
> that for the moment.)  What is good Python style for an abstract
> factory?
> 
> One way is a simple function, as here.  Another way is a static method
> like Branch.open().  I suppose one might also override the call method
> for Branch so that it possibly constructs a subclass(?)
> 
> The function works fine but I just wondered if there were any comments
> on idioms that work well?
> 

I was thinking about it. In C++ I frequently use a static method so that
I can keep my functions grouped together. However, in python, they are
already grouped by file.

I would recommend just using a normal (factory) function, that isn't
part of the class.
The problem is that really you want a branch hierarchy. You can argue
that you want a standard Branch object and a BranchStorage hierarchy. It
doesn't really matter, but to me, that is just one more layer of
indirection that you don't really need.

I think what you want is a factory function that works off of a
dictionary/list object, so that if someone wants, they can add their own
branch type, say through a plugin.

Here is my rough idea:

branch_types = [(is_http, HttpBranch),
		(is_sftp, SftpBranch),
		(is_local, LocalBranch)]

def is_http(path):
	if path[:7] == 'http://' or path[:8] == 'https://':
		return True
	return False
def is_sftp(path): #Similar

# This one should always be the last entry in the list, as it
# is the catch-all, or we could look for either the file://
# or an absolute path, I'm just worried about windows where you
# start having stuff like C:, which messes with namespaces
# like http:
def is_local(path):
	return True

class HttpBranch(Branch):
	def get_revision(self, revision_id):
		# This is the overload of how to get a revision
		# over http.
...

def branch(path, find_root=True, init=False):
	for check_func, klass in branch_types:
		if check_func(path):
			return klass(path, init=init,
				find_root=find_root)

# What this lets a plugin do is something like
import bzrlib, bzrlib.Branch

def is_mybranchtype(path):
	pass
class MyBranchType(bzrlib.Branch):
	pass

bzrlib.Branch.branch_types.insert(0, (is_mybranchtype, MyBranchType))

-------------------

The list could be inside a base class, so you would have:

class Branch(object):
	branch_types = []

And then as children are defined, they add themselves
class HttpBranch(Branch):
	pass
Branch.branch_types.insert(0, (is_http, HttpBranch))

etc.

All this can simply be repeated if you decide to go with the
BranchStorage object instead of the Branch object.

I completely agree that we need to figure out some sort of asynchronous
request pipeline protocol. I'm not sure what functions would return a
list of entries that we could pipeline, but it would certainly be a
speed improvement if you could make a bunch of requests, and then wait
for them on the other end.

I know you can do it with select() or with threads, I'm not really sure
how you select() on an arbitrary object, probably it just needs a file
number associated with it, and then some sort of blocking on the file. I
think the threads method is a little bit more straightforward to
program. As then each object can have it's own downloading thread, and
you can just ask the object if it is ready. But I guess a lot of people
prefer the select() idiom.

I would also say that *usually* you might be able to make a bunch of
requests at once, but you really only want to process them serially. And
you probably know which one you want to start with. So you would say
"request all of these, but block until *this* one comes in". With a
thread model, that is just as simple as blocking on the object.read()
method.

John
=:->
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 251 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20050606/2495e495/attachment.pgp