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 setting client properties #132

Merged

Conversation

jeromegamez
Copy link
Contributor

@jeromegamez jeromegamez commented Jan 27, 2023

Hey there 👋!

Thank you for providing this library, I'm currently trying it out by porting an existing worker to it and enjoying the experience so far!

One feature I was missing was setting client properties and capabilities, for example, to provide a connection name to more easily identify connections to a server.

Enabling Bunny to set client properties is what I'd like to propose with this PR.

$connection = [
    'host'              => 'HOSTNAME',
    'vhost'             => 'VHOST',    // The default vhost is /
    'user'              => 'USERNAME', // The default user is guest
    'password'          => 'PASSWORD', // The default password is guest
    'client_properties' => [
        'connection_name' => 'It works!',
    ],
];
Without connection name With connection name
without_connection_name with_connection_name

I looked if there was a meaningful way to add a test for this, but I'm honestly not sure how to prove that the connection name really is set - all I have are the screenshots above and my word 😅.

I'd appreciate your feedback and am happy to make the needed adjustments to make a merge possible.

:octocat:

@SimonSerrano
Copy link

Hello,
we actually need that feature. This is quite a small change, @jakubkulhan can you take a look, merge and publish a new version ?

@jeromegamez jeromegamez force-pushed the client-properties-support branch from 4aa5266 to 10b26cc Compare September 5, 2023 12:39
@jeromegamez
Copy link
Contributor Author

I rebased the feature branch with the latest main branch to make sure the changes can still be applied 💪🏻

@jeromegamez jeromegamez force-pushed the client-properties-support branch from 10b26cc to 5a0e944 Compare May 10, 2024 22:50
@jeromegamez
Copy link
Contributor Author

Gentle ping @WyriHaximus 🙏🏻, I just rebased the branch again.

I also looked at the 0.6 branch, but I'd first have to find my way to see how I could add this there, if it's even needed 🤔

@WyriHaximus
Copy link
Collaborator

@jeromegamez Thanks for the PR and nudge. I'll have a look at it tomorrow. Been using similar features to add information like pod name to consumers, so I fully welcome and support these features added to Bunny. As for 0.6, lets get it in 0.5 first before adding it to 0.6. 0.6 is a massive internal change from 0.6, but I love that you mention it.

@WyriHaximus WyriHaximus modified the milestones: v0.5.5, v0.5.6 May 10, 2024
Copy link
Collaborator

@WyriHaximus WyriHaximus left a comment

Choose a reason for hiding this comment

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

Thank you for filing this PR adding the neat little feature 👍 . I'll tag a release in the next few days. Would you mind filing this for 0.6 as well, should be fairly simular?

@WyriHaximus WyriHaximus merged commit 6cdf0ed into jakubkulhan:master May 19, 2024
128 of 146 checks passed
@jeromegamez
Copy link
Contributor Author

jeromegamez commented May 19, 2024

I'll definitely try, but I have to admit, 0.6 isn't easy for me to grasp 🙈. But don't tell me!

@jeromegamez jeromegamez deleted the client-properties-support branch May 19, 2024 22:01
@jeromegamez
Copy link
Contributor Author

jeromegamez commented May 19, 2024

Should I target master or 0.6.x?

Update: Nevermind, sorry!

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.

3 participants