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

AMQ: multiple hosts in AmqpDetailsConnectionProvider #1772

Merged
merged 4 commits into from
Jun 25, 2019

Conversation

EldarKZ16
Copy link
Contributor

@EldarKZ16 EldarKZ16 commented Jun 24, 2019

Purpose

Create AmqpDetailsConnectionProvider with multiple hosts and ports

References

References #xxxx

Changes

Background Context

@lightbend-cla-validator

Hi @EldarKZ16,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

http://www.lightbend.com/contribute/cla

@@ -203,6 +203,9 @@ object AmqpDetailsConnectionProvider {
def apply(host: String, port: Int): AmqpDetailsConnectionProvider =
new AmqpDetailsConnectionProvider(immutable.Seq(host -> port))

def apply(hostAndPorts: immutable.Seq[(String, Int)]): AmqpDetailsConnectionProvider =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add it to the Java API with j.u.Collection<akka.japi.Pair<String, Integer>>, as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to add some conversion logic...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I forgot, sorry about this. Fixed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, your change is adapting the wrong data.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm Junior Backend developer :)

Copy link
Member

@ennru ennru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@ennru ennru added this to the 1.0.3 milestone Jun 25, 2019
@ennru ennru changed the title Add additional AmqpDetailsConnectionProvider apply method AMQ: multiple hosts in AmqpDetailsConnectionProvider Jun 25, 2019
@ennru ennru merged commit 0772a75 into akka:master Jun 25, 2019
@ennru
Copy link
Member

ennru commented Jun 25, 2019

Thank you! Keep the improvements coming.

cheleb pushed a commit to cheleb/alpakka that referenced this pull request Jul 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants