On 08/12/2014 10:25 AM, Alan Conway wrote:
On Mon, 2014-08-11 at 16:47 +0100, Gordon Sim wrote:
Thanks very much for the comments, Alan, some responses inline...
On 08/11/2014 03:01 PM, Alan Conway wrote:
Naming: I hate things called *_tools, *_utils etc. We should find a more
descriptive name. Not foo or fred or whatchymacallit or dingdong or
doofer either. Ideally these classes might all just end up in the proton
package, or maybe as a subpackage like proton.events or proton.reactor.
I personally think they should (generally) be kept distinct from the
current proton package to make it clear that they are additive and
optional. A subpackage would be ok, I just didn't want to mix this with
dependent edits to the proton tree itself to begin with. It may even be
that not all the additional pieces belong in the same package.
Fair enough. I like the sub-package but it needs to be a little more
descriptive than proton.utils. proton.reactor is growing on me a bit.
proton.events is not so bad - it's all about handling a proton.Event
after all.
Yes, 'proton events' is not bad.
I did want to convey the notion that these are various optional pieces
of a 'toolkit' for making the core engine API easier to use rather than
a fixed API in their own right. That is perhaps a subtle (or
meaningless?!) point, and may not be something to be conveyed in a
package name... How do you feel about 'eventkit'?
I'll change to proton_events for now to see how that feels.
Runtime is a bit vague as a name as well though I haven't got a better
idea.
Yes, this was unpopular with other I spoke to also. I reverted to my
original name - Container - which isn't ideal either. Any suggestions
from other gratefully considered.
Its funny how often we run into this problem in this business. "Object",
"Entity", "Thing", "Class", "Type",
"IHaveToCallItSomethingButIReallyDontWantToBeSpecific" :)
Looking at the code, Runtime does nothing but add connect and accept to
SelectLoop, and SelectLoop is basically an Event pump. I get that we
need to allow other forms of event pump in future, e.g. EPollLoop or
whatever.
So I would first suggest changing the name to
EventLoop { connect(); accept(); run(); }
Yes, I toyed with that as well and I do agree it is a more descriptive
name. I felt it was adding something to an event loop by tying things
together such that the event loop was operating on sockets boiund to
AMQP connections. However as you say perhaps its really just making it
more 'fully functional'...
I'll go with EventLoop for now, it's better than anything else.
Since this is really just a "fully functional" general event loop, based
on some concrete type of event pump implementation (e.g. SelectLoop)
class EventLoop:
def __init__self(self, handlers=None, PumpClass=SelectLoop):
self.handlers = handlers or [ default_handlers...]
self.loop = PumpClass(Events(ScopedDispatcher(), *handlers)
Note that as it stands the current Runtime/SelectLoop contract is not
really suitable for adding new types of pump since RunTime.connect knows
about Selectables,
I _think_ selectables could be used with other types of loop also. The
key is that they have a file descriptor associated with them, can be
readable or writable and can read or write respectively when that is the
case.
but that contract is something for integrators or
advanced users adding new types of pump so lets get the general user
interface right first.
Agreed
I did also consider flipping it and making EventLoop an abstract base
for SelectLoop and future pumps, but I'm not sure that helps anything. I
like a separate class that is the end user interface and entirely
separate classes for implementation details.
--- python tips (apologies if obvious these are things I only figured
out recently)
1. don't use import *. E.g. use "from proton import Events,...."
import * makes it easier to write but harder to read since you have to
consult other files to figure out where things come from.
Yes, I'll change that.
2. NEVER use map/list literals as keyword argument defaults!
def this_is_bad(handlers=[]): ....
because the list/map is created at module load time NOT each time the
function is called!!! Since lists & maps are mutable the default can
easily be inadvertently changed . Very hard to debug!
Good point. I don't think I have anything like that at present but will
avoid adding it.
[...]
Messages and threads: once received an app will want to dispatch
messages based on address and other properties and allocate processing
to threads in various ways. This is (should be) orthogonal to the API
you are designing here but we need to make sure we have adequate thread
safety and don't create limitations in the API that make things
difficult.
I agree that more thought and examples on how to interact with other
threads is important. However at the same time, I want to avoid trying
to make everything threadsafe.
Agreed.
E.g. the app may want to process messages from a single
proton connection (i.e. a single event stream) in multiple threads and
send reply messages back to the same connection from arbitrary threads.
The proton connection object is not threadsafe, and I don't think that
should change.
Agreed. What I'm getting at is there's a need for more tools (thread
pools, thread safe queues) to allow people to write multi-threaded
message servers that aren't necessarily tied to the threading model of
proton.
Yes, we are in complete agreement I think.
But I think that should be an add-on not a change to what you
are doing here, so probably a distraction at this point.
We shouldn't delay it too long though, I agree.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
