On Wed, Feb 18, 2015 at 4:18 AM, Gordon Sim <g...@redhat.com> wrote:
> On 02/17/2015 08:49 PM, Justin Ross wrote:
>
>> I've made a few changes to the api layout proposal:
>>
>>
>> https://cwiki.apache.org/confluence/display/qpid/
>> Proton+API+layout+proposal
>>
>> - Renamed proton.reactors to proton.reactor
>> - Added proton.security, to clean up the "core" space. This would be
>> home
>> to future additions such as message encryption as well.
>> - Went with proton.extras over proton.contrib. This was to avoid any
>> appearance that we are disowning the pieces there. Instead, they are
>> simply extensions outside the core Proton functionality.
>>
>
A couple of comments in no particular order...
The "Core user model and uitility classes" description could be
misinterpreted on a quick read. Maybe go with "Core protocol model..." or
something like that? (Really all these APIs are for users, so it seems
weird to mention it.) Also the utility classes mentioned here sound like
they overlap with the util package. What is that actually referring to?
I like the proton.security addition, I think that makes sense as security
is generally handled in one contained place in any application.
I'm not so hot on the types subpackage. I don't think it actually buys us a
lot in terms of uncluttering core since there are only a few very simple
things in that category. It basically consists of about 5 or so pure
constructors that extend builtin python types with extra information, e.g.
timestamp which is just a subclass of long that gets encoded on the wire as
a timestamp. It's implementation is literally just this[1], as you can see
it adds very little surface area to the API. The other items in this
category are similarly simple. Conceptually these types really are part of
the core protocol model, and unlike in the case of security, if your app
does need to deal with timestamps or unsigned numbers or whatever, it is
pretty likely that it will need to do it all over the place which means
that you basically always need to do two imports in order to get the full
protocol model.
[1]:
class timestamp(long):
def __repr__(self):
return "timestamp(%s)" % long.__repr__(self)
>
>> Gordon also put forward the notion of proton.adapters in lieu of
>> proton.extras. I like that as well, but I think it has a narrower sense,
>> and I was aiming to keep the buckets bigger and flatter.
>>
>
FWIW, the extras thing looks a bit odd to me. It kind of reads a bit like
"proton.bolt-ons", or "proton.stuff-we-couldn't-bother-to-name-properly". I
also don't think the "disown" thing is particularly meaningful (especially
not to end users). Contrib certainly isn't about disowning anything, it's
about layering. The stuff in contrib isn't allowed to depend on proton
internals, only on public APIs. Historically whether something is
"disowned" has nothing to do with what it is named, and much more to do
with whether or not it is maintained. We've deleted way more stuff in core
that got old and crufty and was unmaintained than we've deleted from
contrib. The contrib stuff, assuming it includes a reasonable test suite,
actually has a much better chance of just continuing to work with minimal
maintenance precisely because of the layering. That layering means that
when something breaks in the contrib tests it's because we introduced a bug
in a core API and so there is good incentive for us to fix it and make sure
the contrib stuff still works.
> To amplify a little, the point was that the two things currently in the
> utils module are ways of adapting the reactive, non-blocking, event-driven
> style to some other style (messenger is in my view a similar sort of
> thing). Though it is certainly more narrow, I think its also more helpful.
>
> The other aspect to these is that they aren't yet considered as
> fully-baked as the reactive core. I certainly don't object to them being in
> an extras namespace to denote that, until we are more comfortable we have
> the interfaces right. You could also however indicate that through
> documentation or some 'experimental' annotation.
I agree we don't necessarily need to signal bakedness in the package name,
I think it's good to think of it as an orthogonal dimension. I'm not quite
sure I follow the reasoning for proton.adapters though. That sounds to me
like somewhere we'd put stuff for integrations, e.g. maybe the tornado
stuff.
--Rafael
