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

Add support for ssl connection #10

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

marcelloceschia
Copy link

No description provided.

@keithf4
Copy link
Contributor

keithf4 commented Mar 15, 2016

Looking for anyone that would be able to work on implementing this with the current version and bundle it as an extension update.

@marcelloceschia
Copy link
Author

I can try to merge the ssl branch with the current master. But are there any plans to integrate this into master/develop?

@marcelloceschia
Copy link
Author

updated ssl branch

@keithf4
Copy link
Contributor

keithf4 commented Mar 30, 2016

I do plan on doing my best to try and get this pulled into the master branch if possible. C is not my strong suit at the moment, so may take a while.

@marcelloceschia
Copy link
Author

@keithf4 If you have any questions, feel free to contact me.
I have the latest version from ssl branch running in one productive environment that requires ssl connections.

@keithf4
Copy link
Contributor

keithf4 commented Mar 30, 2016

So I think I'd like to see about getting the updated librabbitmq library in place before trying to get this pulled in.

#18

Seems that would be better long term to make sure the base libraries are up to date and working before pulling in a new major feature like this. I'm going to be testing out that pull request to the best of my ability to make sure all the basic stuff is working and will then push out a new version. Could use any additional assistance you can provide testing that then.

@marcelloceschia
Copy link
Author

That's a good idea. If you create a feature branch with the latest library version, I will reimplement the ssl support against this branch and you can publish a new version 0.5.0 with latest library version and ssl support

@postwait
Copy link
Member

It appears that @[email protected] changed signature and the old one was removed? Am I reading that diff correctly? If so, this would break existing clients of the extension. Can you confirm the old function signature is left in place and "does the right thing?"

@marcelloceschia
Copy link
Author

@postwait yes @[email protected] signature changed. But if you read this carefully, you will see that this only extends the old signature:

old:

CREATE FUNCTION @[email protected](broker_id integer, exchange varchar, routing_key varchar, message varchar, delivery_mode integer default null, content_type varchar default null, reply_to varchar default null, correlation_id varchar default null)

new:

CREATE FUNCTION @[email protected](broker_id integer, exchange varchar, routing_key varchar, message varchar, delivery_mode integer default null, content_type varchar default null, reply_to varchar default null, correlation_id varchar default null, headers varchar[][2] default null, properties varchar[][2] default null)

to allow header routing as well.
But I will do this PR for a newer version of the amqp library as discussed with @keithf4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants