You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Gordon Sim <gs...@redhat.com> on 2015/03/03 15:39:27 UTC
Review Request 31681: suggestion: rename proton.utils to proton.sync?
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31681/
-----------------------------------------------------------
Review request for qpid, Alan Conway, Justin Ross, and Rafael Schloming.
Repository: qpid-proton-git
Description
-------
In order to convey a little bit more about what the contents do. Just a suggestion.
Diffs
-----
examples/python/helloworld_blocking.py d9a24a9
examples/python/sync_client.py 82cd85f
proton-c/bindings/python/CMakeLists.txt 7f54033
proton-c/bindings/python/proton/sync.py PRE-CREATION
proton-c/bindings/python/proton/utils.py d5e2e0a
tests/python/proton_tests/sync.py PRE-CREATION
tests/python/proton_tests/utils.py 727b586
Diff: https://reviews.apache.org/r/31681/diff/
Testing
-------
Thanks,
Gordon Sim
Re: Review Request 31681: suggestion: rename proton.utils to
proton.sync?
Posted by Alan Conway <ac...@redhat.com>.
On Tue, 2015-03-10 at 11:20 +0000, Gordon Sim wrote:
> On 03/10/2015 10:51 AM, Rafael Schloming wrote:
> > My tour of the code started with an attempt to figure out what the term
> > "sync" was intended to mean since "synchronous" is commonly used to refer
> > to both a blocking programming style and a request/response messaging
> > pattern, but the two aren't necessarily correlated. (Blocking APIs can do
> > asynchronous messaging, and non-blocking APIs can do synchronous
> > messaging.)
>
> Yes, that is a good point.
>
> (Personally I don't think it necessarily rules out having both under the
> same namespace, as long as the detailed semantics are clear. The name
> would be a hint for either and indeed both meanings are currently
> embodied in the code in that package).
>
> > As it stands this code could be
> > anything from the very early start of a general purpose blocking API for
> > proton, to a simple convenience API for one particular scenario. Where it
> > falls on this spectrum would significantly influence both its name and
> > maturity level.
>
> Yes, I agree very much with this. The code that is there is at present
> both immature and limited. It provides a very simple blocking,
> sequential 'adapter' over the reactive core, and additionally a simple
> rpc mechanism on top of that. How it evolves is very much open to
> feedback from users and other developers.
I think the approach is good, I found it useful for doing RPC. It can
probably be generalized and parameterized a bit more. I see an important
role for this in providing an on-ramp to proton for developers that are
used to "command-style" (I won't say blocking or synchronous) APIs
rather than a reactive API (e.g. messenger, messaging) and for simple
RPC-like tools (e.g. dispatch's command-line management tools.)
Obviously not a replacement for the more powerful reactive API but a
useful complement to make proton more approachable to a wider developer
audience.
Re: Review Request 31681: suggestion: rename proton.utils to proton.sync?
Posted by Gordon Sim <gs...@redhat.com>.
On 03/10/2015 10:51 AM, Rafael Schloming wrote:
> My tour of the code started with an attempt to figure out what the term
> "sync" was intended to mean since "synchronous" is commonly used to refer
> to both a blocking programming style and a request/response messaging
> pattern, but the two aren't necessarily correlated. (Blocking APIs can do
> asynchronous messaging, and non-blocking APIs can do synchronous
> messaging.)
Yes, that is a good point.
(Personally I don't think it necessarily rules out having both under the
same namespace, as long as the detailed semantics are clear. The name
would be a hint for either and indeed both meanings are currently
embodied in the code in that package).
> As it stands this code could be
> anything from the very early start of a general purpose blocking API for
> proton, to a simple convenience API for one particular scenario. Where it
> falls on this spectrum would significantly influence both its name and
> maturity level.
Yes, I agree very much with this. The code that is there is at present
both immature and limited. It provides a very simple blocking,
sequential 'adapter' over the reactive core, and additionally a simple
rpc mechanism on top of that. How it evolves is very much open to
feedback from users and other developers.
Re: Review Request 31681: suggestion: rename proton.utils to
proton.sync?
Posted by Alan Conway <ac...@redhat.com>.
On Tue, 2015-03-10 at 23:51 +1300, Rafael Schloming wrote:
> Looking at the SyncRequestResponse part, it's also possible I have the
> wrong end of the stick and that all the various Blocking* classes are
> mostly implementation details and the real point of the thing is to be an
> rpc tool.
The history here is that Gordon wrote the BlockingConnection parts as a
toolkit and I used them to do RPC because I needed that for management.
So I think this is intended to be a more general toolkit, RPC is just
one of the early applications.
Re: Review Request 31681: suggestion: rename proton.utils to proton.sync?
Posted by Rafael Schloming <rh...@alum.mit.edu>.
Sorry to follow up on this late. I'm currently traveling and have spotty
connectivity.
To be honest I haven't been able to spare the time to looked very closely
at the code in question until recently, so most of my comments are less
about the name and more a rather late set of questions around the
intentions behind the code itself.
My tour of the code started with an attempt to figure out what the term
"sync" was intended to mean since "synchronous" is commonly used to refer
to both a blocking programming style and a request/response messaging
pattern, but the two aren't necessarily correlated. (Blocking APIs can do
asynchronous messaging, and non-blocking APIs can do synchronous
messaging.) I expected a closer look at the code to clear up the ambiguity
for me, but it actually doesn't. The code uses the term Blocking throughout
suggesting that it refers to the programming style, yet it also seems
(possibly intentionally) limited to purely synchronous scenarios, e.g.
there is a window on the receive side, but send is hardcoded to have a
window of 1 and just from looking at the code it is hard to tell whether
this is an intentional design choice or whether the initial use case simply
doesn't happen to include send windows larger than 1.
Looking at the SyncRequestResponse part, it's also possible I have the
wrong end of the stick and that all the various Blocking* classes are
mostly implementation details and the real point of the thing is to be an
rpc tool.
I think some description of the intended scope of the API would be helpful
in providing a less provisional name. As it stands this code could be
anything from the very early start of a general purpose blocking API for
proton, to a simple convenience API for one particular scenario. Where it
falls on this spectrum would significantly influence both its name and
maturity level.
--Rafael
On Fri, Mar 6, 2015 at 8:56 AM, Justin Ross <jr...@apache.org> wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31681/
>
> Ship it!
>
> I like the name.
>
>
> - Justin Ross
>
> On March 3rd, 2015, 2:39 p.m. UTC, Gordon Sim wrote:
> Review request for qpid, Alan Conway, Justin Ross, and Rafael Schloming.
> By Gordon Sim.
>
> *Updated March 3, 2015, 2:39 p.m.*
> *Repository: * qpid-proton-git
> Description
>
> In order to convey a little bit more about what the contents do. Just a suggestion.
>
> Diffs
>
> - examples/python/helloworld_blocking.py (d9a24a9)
> - examples/python/sync_client.py (82cd85f)
> - proton-c/bindings/python/CMakeLists.txt (7f54033)
> - proton-c/bindings/python/proton/sync.py (PRE-CREATION)
> - proton-c/bindings/python/proton/utils.py (d5e2e0a)
> - tests/python/proton_tests/sync.py (PRE-CREATION)
> - tests/python/proton_tests/utils.py (727b586)
>
> View Diff <https://reviews.apache.org/r/31681/diff/>
>
Re: Review Request 31681: suggestion: rename proton.utils to
proton.sync?
Posted by Justin Ross <jr...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31681/#review75380
-----------------------------------------------------------
Ship it!
I like the name.
- Justin Ross
On March 3, 2015, 2:39 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31681/
> -----------------------------------------------------------
>
> (Updated March 3, 2015, 2:39 p.m.)
>
>
> Review request for qpid, Alan Conway, Justin Ross, and Rafael Schloming.
>
>
> Repository: qpid-proton-git
>
>
> Description
> -------
>
> In order to convey a little bit more about what the contents do. Just a suggestion.
>
>
> Diffs
> -----
>
> examples/python/helloworld_blocking.py d9a24a9
> examples/python/sync_client.py 82cd85f
> proton-c/bindings/python/CMakeLists.txt 7f54033
> proton-c/bindings/python/proton/sync.py PRE-CREATION
> proton-c/bindings/python/proton/utils.py d5e2e0a
> tests/python/proton_tests/sync.py PRE-CREATION
> tests/python/proton_tests/utils.py 727b586
>
> Diff: https://reviews.apache.org/r/31681/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 31681: suggestion: rename proton.utils to
proton.sync?
Posted by Alan Conway <ac...@redhat.com>.
> On March 3, 2015, 11:01 p.m., Alan Conway wrote:
> > I agree with the name change but please don't break dispatch at this late date. Please add a utils.py with `from sync import *` for backwards compatibility, put a comment in there to say don't use it, deprecated, internal use only, beware of dragons etc. Maybe build dispatch against it just to be sure ;)
>
> Gordon Sim wrote:
> Good point/reminder re Dispatch. However *if* we went ahead with this change before 0.9, I would want to change Dispatch at the same time. AT that point the utils module would never have been released so introducing a legacy version just for the one internal use would be a bad choice.
I'm fine with updating dispatch but we already have dispatch users with a proton pre-release and we have already broken their code with incompatible proton changes. I fully understand that we make no compatibility guarantees before release but all I'm asking for is a simple alias, undocumented, unsupported and with plenty of "don't use this!" signs on it. Let's not punish the early adopters too much for being early.
- Alan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31681/#review75072
-----------------------------------------------------------
On March 3, 2015, 2:39 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31681/
> -----------------------------------------------------------
>
> (Updated March 3, 2015, 2:39 p.m.)
>
>
> Review request for qpid, Alan Conway, Justin Ross, and Rafael Schloming.
>
>
> Repository: qpid-proton-git
>
>
> Description
> -------
>
> In order to convey a little bit more about what the contents do. Just a suggestion.
>
>
> Diffs
> -----
>
> examples/python/helloworld_blocking.py d9a24a9
> examples/python/sync_client.py 82cd85f
> proton-c/bindings/python/CMakeLists.txt 7f54033
> proton-c/bindings/python/proton/sync.py PRE-CREATION
> proton-c/bindings/python/proton/utils.py d5e2e0a
> tests/python/proton_tests/sync.py PRE-CREATION
> tests/python/proton_tests/utils.py 727b586
>
> Diff: https://reviews.apache.org/r/31681/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 31681: suggestion: rename proton.utils to
proton.sync?
Posted by Gordon Sim <gs...@redhat.com>.
> On March 3, 2015, 11:01 p.m., Alan Conway wrote:
> > I agree with the name change but please don't break dispatch at this late date. Please add a utils.py with `from sync import *` for backwards compatibility, put a comment in there to say don't use it, deprecated, internal use only, beware of dragons etc. Maybe build dispatch against it just to be sure ;)
Good point/reminder re Dispatch. However *if* we went ahead with this change before 0.9, I would want to change Dispatch at the same time. AT that point the utils module would never have been released so introducing a legacy version just for the one internal use would be a bad choice.
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31681/#review75072
-----------------------------------------------------------
On March 3, 2015, 2:39 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31681/
> -----------------------------------------------------------
>
> (Updated March 3, 2015, 2:39 p.m.)
>
>
> Review request for qpid, Alan Conway, Justin Ross, and Rafael Schloming.
>
>
> Repository: qpid-proton-git
>
>
> Description
> -------
>
> In order to convey a little bit more about what the contents do. Just a suggestion.
>
>
> Diffs
> -----
>
> examples/python/helloworld_blocking.py d9a24a9
> examples/python/sync_client.py 82cd85f
> proton-c/bindings/python/CMakeLists.txt 7f54033
> proton-c/bindings/python/proton/sync.py PRE-CREATION
> proton-c/bindings/python/proton/utils.py d5e2e0a
> tests/python/proton_tests/sync.py PRE-CREATION
> tests/python/proton_tests/utils.py 727b586
>
> Diff: https://reviews.apache.org/r/31681/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 31681: suggestion: rename proton.utils to
proton.sync?
Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31681/#review75072
-----------------------------------------------------------
I agree with the name change but please don't break dispatch at this late date. Please add a utils.py with `from sync import *` for backwards compatibility, put a comment in there to say don't use it, deprecated, internal use only, beware of dragons etc. Maybe build dispatch against it just to be sure ;)
- Alan Conway
On March 3, 2015, 2:39 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31681/
> -----------------------------------------------------------
>
> (Updated March 3, 2015, 2:39 p.m.)
>
>
> Review request for qpid, Alan Conway, Justin Ross, and Rafael Schloming.
>
>
> Repository: qpid-proton-git
>
>
> Description
> -------
>
> In order to convey a little bit more about what the contents do. Just a suggestion.
>
>
> Diffs
> -----
>
> examples/python/helloworld_blocking.py d9a24a9
> examples/python/sync_client.py 82cd85f
> proton-c/bindings/python/CMakeLists.txt 7f54033
> proton-c/bindings/python/proton/sync.py PRE-CREATION
> proton-c/bindings/python/proton/utils.py d5e2e0a
> tests/python/proton_tests/sync.py PRE-CREATION
> tests/python/proton_tests/utils.py 727b586
>
> Diff: https://reviews.apache.org/r/31681/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>