Re: Reducing the visibility of proton-j constructors
On 23 January 2013 17:36, Phil Harvey p...@philharveyonline.com wrote:
As part of the Proton JNI work, I would like to remove all calls to
proton-j implementation constructors from client code. I intend that
factories will be used instead [1], thereby abstracting away whether the
implementation is pure Java or proton-c-via-JNI.
I'd like to check that folks are happy with me making this change, and to
mention a couple of problems I've had.
In this context, client code is anything outside the current
sub-component, where our sub-components are Engine, Codec, Driver, Message
and Messenger, plus each of the contrib modules, and of course third party
code.
To enforce this abstraction, I am planning to make the constructors of the
affected classes package-private where possible. I believe that, although
third party projects might already be calling these constructors, it is
acceptable for us to change its public API in this manner while Proton is
such a young project.
+1 to all of the above
Please shout if you disagree with any of the above.
Now, onto my problem. I started off with the org.apache.qpid.proton.engine.
impl package, and found that o.a.q.p.hawtdispatch.impl.AmqpTransport calls
various methods on ConnectionImpl and TransportImpl, so simply using a
Connection and Transport will not work. I don't know what to do about
this, and would welcome people's opinions.
So, the simplest change would be to change the factories to use
covariant return types
e.g. EngingFactoryImpl becomes:
@Override
public ConnectionImpl createConnection()
{
return new ConnectionImpl();
}
@Override
public TransportImpl createTransport()
{
return new TransportImpl();
}
... etc
Code that requires the extended functionality offered by the pure Java
implementation can thus instantiate the desired Factory directly.
A second refinement might be to actually separate out the interface
and implementation within the pure Java implementation so that we have
a well defined extended Java API. This interface could then be the
return type of the factory.
-- Rob
Thanks
Phil
[1] for example, these work-in-progress classes:
https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-j/proton-api/src/main/java/org/apache/qpid/proton/ProtonFactoryLoader.java
https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-j/proton-api/src/main/java/org/apache/qpid/proton/engine/EngineFactory.java
https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-j/proton/src/main/java/org/apache/qpid/proton/engine/impl/EngineFactoryImpl.java
https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-c/bindings/java/jni/src/main/java/org/apache/qpid/proton/engine/jni/JNIEngineFactory.java
Re: Question about proton-c message id API
On 23 January 2013 15:40, Rafael Schloming r...@alum.mit.edu wrote:
Yeah, it appears to be a bug. I checked in a potential fix on trunk. Give
it a shot and see if it's still an issue.
--Rafael
Thanks, that has indeed addressed the issue. The Message system tests
which previously failed now run cleanly against the Proton-JNI (on the
jni-branch).
It does leave the question regarding the differences in usage of the
Proton-API. The Python/C binding makes calls to pn_message,
pn_message_id, and pn_message_correlation_id when creating its Message
facade object. The Ruby/C and Java/C bindings call only pn_message
during construction of their facade object.
Python/C binding:
def __init__(self):
self._msg = pn_message()
self._id = Data(pn_message_id(self._msg))
self._correlation_id = Data(pn_message_correlation_id(self._msg))
Ruby/C binding:
def initialize
@impl = Cproton.pn_message
ObjectSpace.define_finalizer(self, self.class.finalize!(@impl))
end
Java/C binding (jni-branch only)
JNIMessage()
{
_impl = Proton.pn_message();
}
Is there any reason why Python/C works in a different manner?
Kind regards, Keith.
Re: Changing the Proton build system to accommodate jni bindings
On 24 January 2013 14:43, Rafael Schloming r...@alum.mit.edu wrote: On Wed, Jan 23, 2013 at 6:10 PM, Rob Godfrey rob.j.godf...@gmail.comwrote: On 23 January 2013 19:09, Rafael Schloming r...@alum.mit.edu wrote: I've added another wiki page that documents the proton release steps as best I can remember. I'll updated it more during the 0.4 release: https://cwiki.apache.org/confluence/display/qpid/Proton+Release+Steps I think it's important to understand the overall release and testing process as it is a significant and perhaps underrepresented factor against which to measure any proposals. I believe the build system requirements documented below are inherently incomplete as they don't recognize the fact that the C build system is not just a developer productivity tool, it is also the installer for our end users. And before anyone says our end users will just use yum or equivalents, all those packaging tools *also* depend on our build system both directly, and because we can't even supply a release for packagers to consume without a reasonable amount of direct install testing. To a good extent a standard looking C source tarball is pretty much the equivalent of a jar or jar + pom file in the Java world, it's really the only platform independent means of distribution we have. It would be helpful if you could enumerate requirements which you believe to be missing and add the to the existing wiki page. I don't think anyone is suggesting that the make install step should be broken in the source tarball, so it's a little unclear to me the problem you are trying to highlight above. I believe it was suggested at one point that we not have a C source tarball but just export the entire tree as a single source tarball. This strictly speaking would not break the make install step, however it would have a serious impact on our ability to leverage others to test the C impl. Anyone downloading this would need to understand a great deal about the dual nature of proton and how it is structured just in order to know that they can ignore half the tree. Compare that with a standard C source tarball where I can hand it off to someone who knows nothing about proton and simply tell them to do a make install and then run one test script. Given the latter structure to our release artifacts there are *significantly* more resources we have access to in order to perform the testing necessary to do a quality release. I'm not sure requiring the ability to read a README file is really going to have a huge impact on our ability to leverage others. I'm not sure how widespread CMake use is (it's certainly less familiar to me than autotools) - certainly I expect people unfamiliar with cmake will have to read the README anyway. I'll take a stab at distilling some requirements out of the above scenario and sticking them onto the wiki page, but I actually think the scenario itself is more important than the requirements. There's no disagreement that it would be nice to have a very standard looking C source tarball with minimal dependencies and so forth that can be used in the above manner, it's simply the relative priority of the requirement when it conflicts with developer convenience that is a source of contention. It's also probably worth noting that perhaps the biggest issue with system tests in Java is not so much imposing maven on proton-c developers, but the and nottest against Java code running elsewhere. fact that Java may not be available on all the platforms that proton-c needs to be tested on. My primary concern here would be iOS. I'm not an expert, but my brief googling seems to suggest there would be significant issues. So, I think we probably need to consider what sort of tests are required, and which languages it is appropriate to write any particular type of test in. For me tests in Java have some advantages over Python tests. Firstly they allow interop tests between the two implementations within the same process Can you elaborate on the benefits of this? It seems to me when it comes to interop testing that, to the extent you can get away with it, over the wire tests would be preferred. For example you could run proton-c on iOS via Java tests running on another system, to say nothing of testing against non proton implementations which would necessarily need to be over-the-wire. The same arguments could be made about our existing tests. However being able to run in process makes thing much easier to automate - no need to randomly assign ports, communicate randomly assigned ports to the other process, etc. and secondly they will also be able to be used against any future pure JavaScript Proton implementation (something we have planned to do but not yet embarked upon). This is also true of python tests. In fact the whole point of a python test suite is that you can run python, java, and javascript all within
Re: Changing the Proton build system to accommodate jni bindings
On Wed, Jan 23, 2013 at 6:44 PM, Rob Godfrey rob.j.godf...@gmail.comwrote: Firstly I think it would be helpful if you made clear the requirements you consider to be essential, nice to have, unimportant and/or detrimental. On 23 January 2013 20:17, Rafael Schloming r...@alum.mit.edu wrote: On Wed, Jan 23, 2013 at 8:01 AM, Keith W keith.w...@gmail.com wrote: Essential 3. To change proton-api, all that is required is to edit a Java file. - Developer productivity This seems to be kind of a leading requirement so to speak, or at least it's phrased a little bit oddly. That said I would never argue with it for most of the Java files, however in the case of the API files I don't see how you're ever going to be able to stop after just editing the API. Because we have two implementations, we're fundamentally stuck with manually syncing the implementations themselves whenever a change to the interface occurs. By comparison the highly automatable task of syncing the API files themselves seems quite small. I'm imagining most changes would go something like this, say we want to add a getter to the Message interface, we would need to: I think it's worth considering two different cases 1) The API change is purely on the Java side... there is no corresponding change to the C API. This may be to add some sort of convenience method, or simply a refactoring. In this case the developer making the change needs only to work in Java, there will be two implementations of the interface to change (in two different source locations) but is all rather trivial. Is this actually possible? Wouldn't you at least need to build/run the C so that you know there is actually no impact on the C impl? Even if you're just calling it differently it could tickle a bug. 2) The API change affects both C and Java. In this case either a single developer has to commit to making the change in both the C and the Java, or the API change has to have been discussed before work commences and Java and C developers will need to work together. If there is a single developer or developers working very closely together then I would suggest that the steps would in fact be: 1. edit the Message interface / edit the message.h file 2. write and/or modify a test (and Python binding if necessary) 3. edit the JNI binding to use the SWIG generated API 4. edit the C / Pure Java 5. run the tests against the C / Java (6. modify other bindings if necessary) repeat steps 4 and 5 until they pass. In the case where the C and Java developers are separated by time/distance then the build / tests on one side will be broken until the implementation catches up. For the sake of politeness it is probably better to ensure that at all points the checked in code compiles even if the tests do not pass. For cases where the changes to the API are additions then it should be relatively easy to make the changes in such a way as to simply have any tests relating to the new API be skipped. For cases where the C leads the Java, the Java implementation can simply throw UnsupportedOperationException or some such. Where the Java leads the C we can throw said exception from the JNI binding code and leave the .h file unchanged until the C developer is ready to do the work. Only for cases where there is modification to existing APIs does it seem that there may be occaisins where we could not have a consistent build across components, and I would strongly recommend that any change where the Java and C are being worked on in such a fashion should take place on a branch, with a merge to trunk only occurring when all tests are passing against all implementations. 1. edit the Message interface 2. write and/or possibly modify a test 3. edit the java Message implementation 4. run the tests against java, if they don't pass go to step 2 5. now that the java impl passes the tests, run the tests against the C impl 6. if the sync check fails on the C build, run the sync script 7. edit the message.h file 8. edit the message.c implementation 9. edit the adapter layer between the C API and the Java interfaces 10. run the tests against the C, if they don't pass go to step 8 11. run the tests against both, just to be sure 12. check in Given the above workflow, it seems like even with a relatively small change like adding a getter, the scripted portion of the syncing effort is going to be vanishingly small compared to the manual process of syncing the implementations. Perhaps I'm just envisioning a different workflow than you, or maybe I'm missing some important scenarios. Could you describe what workflow(s) you envision and how the sync process would impacting your productivity? I differ strongly in my opinion here. Every time I need to drop out of my development environment to run some ad-hoc script then there is
Re: Reducing the visibility of proton-j constructors
You not actually going to prohibit folks for using the old constructors are you? I'd say adding factories is a good thing, and you should encourage folks to use the factories instead of the constructors, but please don't stop folks from using the constructors directly. On Wed, Jan 23, 2013 at 11:36 AM, Phil Harvey p...@philharveyonline.comwrote: As part of the Proton JNI work, I would like to remove all calls to proton-j implementation constructors from client code. I intend that factories will be used instead [1], thereby abstracting away whether the implementation is pure Java or proton-c-via-JNI. I'd like to check that folks are happy with me making this change, and to mention a couple of problems I've had. In this context, client code is anything outside the current sub-component, where our sub-components are Engine, Codec, Driver, Message and Messenger, plus each of the contrib modules, and of course third party code. To enforce this abstraction, I am planning to make the constructors of the affected classes package-private where possible. I believe that, although third party projects might already be calling these constructors, it is acceptable for us to change its public API in this manner while Proton is such a young project. Please shout if you disagree with any of the above. Now, onto my problem. I started off with the org.apache.qpid.proton.engine. impl package, and found that o.a.q.p.hawtdispatch.impl.AmqpTransport calls various methods on ConnectionImpl and TransportImpl, so simply using a Connection and Transport will not work. I don't know what to do about this, and would welcome people's opinions. Thanks Phil [1] for example, these work-in-progress classes: https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-j/proton-api/src/main/java/org/apache/qpid/proton/ProtonFactoryLoader.java https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-j/proton-api/src/main/java/org/apache/qpid/proton/engine/EngineFactory.java https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-j/proton/src/main/java/org/apache/qpid/proton/engine/impl/EngineFactoryImpl.java https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-c/bindings/java/jni/src/main/java/org/apache/qpid/proton/engine/jni/JNIEngineFactory.java -- ** *Hiram Chirino* *Engineering | Red Hat, Inc.* *hchir...@redhat.com hchir...@redhat.com | fusesource.com | redhat.com* *skype: hiramchirino | twitter: @hiramchirinohttp://twitter.com/hiramchirino * *blog: Hiram Chirino's Bit Mojo http://hiramchirino.com/blog/*
Re: Question about proton-c message id API
On Thu, Jan 24, 2013 at 7:38 AM, Keith W keith.w...@gmail.com wrote:
On 23 January 2013 15:40, Rafael Schloming r...@alum.mit.edu wrote:
Yeah, it appears to be a bug. I checked in a potential fix on trunk. Give
it a shot and see if it's still an issue.
--Rafael
Thanks, that has indeed addressed the issue. The Message system tests
which previously failed now run cleanly against the Proton-JNI (on the
jni-branch).
It does leave the question regarding the differences in usage of the
Proton-API. The Python/C binding makes calls to pn_message,
pn_message_id, and pn_message_correlation_id when creating its Message
facade object. The Ruby/C and Java/C bindings call only pn_message
during construction of their facade object.
Python/C binding:
def __init__(self):
self._msg = pn_message()
self._id = Data(pn_message_id(self._msg))
self._correlation_id = Data(pn_message_correlation_id(self._msg))
Ruby/C binding:
def initialize
@impl = Cproton.pn_message
ObjectSpace.define_finalizer(self, self.class.finalize!(@impl))
end
Java/C binding (jni-branch only)
JNIMessage()
{
_impl = Proton.pn_message();
}
Is there any reason why Python/C works in a different manner?
It's just to avoid code duplication between the id property's getter and
setter:
def _get_id(self):
return self._id.get_object()
def _set_id(self, value):
if type(value) in (int, long):
value = ulong(value)
self._id.rewind()
self._id.put_object(value)
id = property(_get_id, _set_id,
doc=
The id of the message.
)
I don't think this particular difference should have an impact on behaviour
one way or another. I think the reason the python binding wasn't hitting
the same bug you've observed is that it extracts the id from the data
object by calling Data.get_object, and that will first query the type of
the current node and then call a specific accessor based on that type,
whereas you appear to be calling pn_data_get_atom which will return the
type and the value in a discriminated union. It's possible we could
simplify the swig binding process by always using the former style as then
we might be able to avoid having to add typemaps for pn_atom_t.
--Rafael
[jira] [Created] (PROTON-201) Provide a C++ Messenger and Message class
Darryl L. Pierce created PROTON-201: --- Summary: Provide a C++ Messenger and Message class Key: PROTON-201 URL: https://issues.apache.org/jira/browse/PROTON-201 Project: Qpid Proton Issue Type: Bug Components: proton-c Reporter: Darryl L. Pierce Assignee: Darryl L. Pierce Provide wrapper classes for these two types similar to what's available in Perl and Ruby. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
Re: Reducing the visibility of proton-j constructors
On Thu, Jan 24, 2013 at 5:06 AM, Rob Godfrey rob.j.godf...@gmail.comwrote:
On 23 January 2013 17:36, Phil Harvey p...@philharveyonline.com wrote:
As part of the Proton JNI work, I would like to remove all calls to
proton-j implementation constructors from client code. I intend that
factories will be used instead [1], thereby abstracting away whether the
implementation is pure Java or proton-c-via-JNI.
I'd like to check that folks are happy with me making this change, and to
mention a couple of problems I've had.
In this context, client code is anything outside the current
sub-component, where our sub-components are Engine, Codec, Driver,
Message
and Messenger, plus each of the contrib modules, and of course third
party
code.
To enforce this abstraction, I am planning to make the constructors of
the
affected classes package-private where possible. I believe that,
although
third party projects might already be calling these constructors, it is
acceptable for us to change its public API in this manner while Proton is
such a young project.
+1 to all of the above
Please shout if you disagree with any of the above.
Now, onto my problem. I started off with the
org.apache.qpid.proton.engine.
impl package, and found that o.a.q.p.hawtdispatch.impl.AmqpTransport
calls
various methods on ConnectionImpl and TransportImpl, so simply using a
Connection and Transport will not work. I don't know what to do about
this, and would welcome people's opinions.
So, the simplest change would be to change the factories to use
covariant return types
e.g. EngingFactoryImpl becomes:
@Override
public ConnectionImpl createConnection()
{
return new ConnectionImpl();
}
@Override
public TransportImpl createTransport()
{
return new TransportImpl();
}
... etc
Code that requires the extended functionality offered by the pure Java
implementation can thus instantiate the desired Factory directly.
What's the point of going through the factory in this scenario rather than
directly instantiating the classes as Hiram suggests? Is there some class
of thing the factory would/could do that the constructor can't/shouldn't?
A second refinement might be to actually separate out the interface
and implementation within the pure Java implementation so that we have
a well defined extended Java API. This interface could then be the
return type of the factory.
Maybe I'm misunderstanding, but what's the point of using an interface here
if you're still locked into the pure Java impl? Are you expecting to swap
out that impl under some circumstances?
--Rafael
Re: Reducing the visibility of proton-j constructors
When I asked the original question I had been assuming that the contrib
modules were intended to be using the proton-api interfaces, but had to
resort to concrete types for tactical reasons pending a more complete API.
If that assumption were true, then using factory interfaces rather than
constructors would clearly be necessary.
But if this assumption is false then I personally have no problem with
concrete classes being instantiated and used directly. However, at the
risk of putting words into Rob's mouth, I guess he may be viewing the use
of concrete classes across module boundaries to be A Bad Thing that tends
to lead to leaky abstractions. Whether we apply that rule on Proton is an
interesting question, and is one of those areas where I care more about us
being consistent than about being right.
Phil
On 24 January 2013 19:03, Rafael Schloming r...@alum.mit.edu wrote:
On Thu, Jan 24, 2013 at 5:06 AM, Rob Godfrey rob.j.godf...@gmail.com
wrote:
On 23 January 2013 17:36, Phil Harvey p...@philharveyonline.com wrote:
As part of the Proton JNI work, I would like to remove all calls to
proton-j implementation constructors from client code. I intend that
factories will be used instead [1], thereby abstracting away whether
the
implementation is pure Java or proton-c-via-JNI.
I'd like to check that folks are happy with me making this change, and
to
mention a couple of problems I've had.
In this context, client code is anything outside the current
sub-component, where our sub-components are Engine, Codec, Driver,
Message
and Messenger, plus each of the contrib modules, and of course third
party
code.
To enforce this abstraction, I am planning to make the constructors of
the
affected classes package-private where possible. I believe that,
although
third party projects might already be calling these constructors, it is
acceptable for us to change its public API in this manner while Proton
is
such a young project.
+1 to all of the above
Please shout if you disagree with any of the above.
Now, onto my problem. I started off with the
org.apache.qpid.proton.engine.
impl package, and found that o.a.q.p.hawtdispatch.impl.AmqpTransport
calls
various methods on ConnectionImpl and TransportImpl, so simply using a
Connection and Transport will not work. I don't know what to do about
this, and would welcome people's opinions.
So, the simplest change would be to change the factories to use
covariant return types
e.g. EngingFactoryImpl becomes:
@Override
public ConnectionImpl createConnection()
{
return new ConnectionImpl();
}
@Override
public TransportImpl createTransport()
{
return new TransportImpl();
}
... etc
Code that requires the extended functionality offered by the pure Java
implementation can thus instantiate the desired Factory directly.
What's the point of going through the factory in this scenario rather than
directly instantiating the classes as Hiram suggests? Is there some class
of thing the factory would/could do that the constructor can't/shouldn't?
A second refinement might be to actually separate out the interface
and implementation within the pure Java implementation so that we have
a well defined extended Java API. This interface could then be the
return type of the factory.
Maybe I'm misunderstanding, but what's the point of using an interface here
if you're still locked into the pure Java impl? Are you expecting to swap
out that impl under some circumstances?
--Rafael
