rabbitmq / rabbitmq-objc-client

RabbitMQ client for Objective-C and Swift
https://rabbitmq.com
Other
241 stars 84 forks source link

Make it possible for apps to provide a custom connection name #156

Closed michaelklishin closed 5 years ago

michaelklishin commented 5 years ago

Closes #155, references #154.

@grsteffens it took a couple of attempts, I like this API better than what I had in mind at first (a simpler way to instantiate RMQConnectionConfig which is not really meant to be used by 99% of users).

michaelklishin commented 5 years ago

It does work (you can see user-provided connection name in server logs and Wireshark), the question is whether

let conn = RMQConnection(
  uri: IntegrationHelper.defaultEndpoint,
  userProvidedConnectionName: "testUserProvidedConnectionName.1",
  delegate: RMQConnectionDelegateLogger())
conn.start()

is an API we want to expose. Compared to my first attempt it is definitely more straightforward and not too different from other clients. So let's keep it. More selectors that include userProvidedConnectionName can be added as needed.

Thanks for the review.

grsteffens commented 5 years ago

Agreed. It also syncs up pretty well with how the Java client works (where the name is passed into the factory), which is nice for parity's sake. My apologies before when I said "looks like this will work", I saw that it was functionally working, I just meant work from an architectural standpoint which it does as well.

Please let me know if there is anything I can contribute to in this library as I like the collaboration. Maybe I could become a collaborator. Thanks again, Michael!

michaelklishin commented 5 years ago

@grsteffens I'm happy to add you as a collaborator. We have a number of issues still open and I will file a couple of things I've noticed while working on this client in the last few months. I'd like to begin addressing things such as #24 and #31 after we produce a new release (#148) but there's an annoying issue with a couple of test cases hanging and failing for a reason I could not so far track down. I'll file a new issue about that now.

michaelklishin commented 5 years ago

@grsteffens check collaborator invitations, you should have one ;) In case you plan to look into an issue, just leave a comment.

I'd suggest setting up the test environment with Docker (see ./docker) and then manually with x509 certificate authentication enabled on the RabbitMQ node would be a good first step. CONTRIBUTING.md should be up-to-date.

grsteffens commented 5 years ago

Awesome, thanks @michaelklishin. I will work on setting up my test environment and then keep my eye out for new issues that you open. After I set up my environment, I can also offer a second set of eyes on those failing and hanging test cases to help expedite the release.

michaelklishin commented 5 years ago

The release doesn't have to wait for #157. This isn't something new and FWIW I don't recall any similar reports from actual users.

grsteffens commented 5 years ago

Ah, understood