Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

qamqp-ng #42

Open
mbroadst opened this issue Jun 17, 2014 · 8 comments
Open

qamqp-ng #42

mbroadst opened this issue Jun 17, 2014 · 8 comments

Comments

@mbroadst
Copy link

Hey,
Not sure how to contact you other than here (no email provided, github doesn't allow messages, etc). I've started a fork of this project over here, which includes a ton of refactoring, auto tests, converting qamqp into a proper shared library, automatic CI with travis CI, amongst many other improvements. It made more sense to work on it as a fork since the changes were so dramatic, but I hope that we can share the effort in the future. I've found a number of bugs in the existing codebase (mostly related to incorrect enums being used for "Ok" frames coming from the server). I have not actually built it on windows (which seems to be your main platform?), but feel that it is back to feature parity with qamqp at this point otherwise.

Thanks for the hard work!

@photex
Copy link

photex commented Jul 10, 2014

+1 for combining efforts. I was hoping to start making use of this and it would be nice to have a primary repo.

Has anyone thought about starting the process of moving this work into qt-project? AMQP for Qt seems like a natural optional module.

@mbroadst
Copy link
Author

@photex I'm all about moving it into qt-project, but it's going to need like.. actual documentation before it makes it in there. I also think there are a number of issues with the API in general that I couldn't anticipate up front and have only presented themselves with actual use (setting MessageProperties is actually really cumbersome, when you have to do like properties.insert(Frame::Content::Something::whateverFlag, "some data")).

Ideally, people would start using this version of the library. I'm actively using it now, and will be quite responsive with issues/PRs. There is also a lot to be done imho in terms of wire performance. Please play around with it, the repo on my github isn't going anyway anytime soon.

@fuCtor
Copy link
Owner

fuCtor commented Jul 19, 2014

Hi, it will be great.
When I started working on this solution it was a proof of concept for our internal project.
At primary it was port of C version + implementation of spec as is.

What about performance. I read some articles where people write server on pure Qt. QtNetwork was bottleneck as result. All suggests to use ASIO for high performance.

@mbroadst
Copy link
Author

@fuCtor cool, are you interested in merging the work into mainline? I don't see any issues with qt network bottlenecks since this is a client library (not implementing an AMQP server). Having said that, I do fully intend to write some benchmarks around the serializing/deserializing code in particular

@photex
Copy link

photex commented Jul 19, 2014

I submitted an RFE to digia for amqp support and provided links to the work you guys have done. Hopefully this starts the discussion.

@mbroadst
Copy link
Author

@photex - unfortunately that's unlikely to get the work done, the proper path is for us to submit it to the qt project for review and inclusion as an add-on. before that happens, the API has to be pretty clean, as the review is likely to be rigorous. one of the initial problems, as I pointed out above, will be that there is little to no documentation. so in order to get this added we'll at least need to start there.

Cheers

@photex
Copy link

photex commented Jul 19, 2014

Certainly. I didn't mean suggest I expected them to pick it up. But registering an rfe is important to get them to know that a customer intends to depend and make use of amqp in their application.

@mbroadst
Copy link
Author

The project at this point has officially moved to https://github.com/mbroadst/qamqp

@mbroadst mbroadst reopened this Sep 11, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants