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

Support headers in queue_declare function #174

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

V4UIDev
Copy link

@V4UIDev V4UIDev commented Jul 19, 2024

SUMMARY

Add headers when running the declare queue function.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

RabbitMQ Publish
basic_publish

ADDITIONAL INFORMATION

When running the following Ansible task:

    - name: Send reindex message to queue
      community.rabbitmq.rabbitmq_publish:
        url: "amqp://user:[email protected]:5672/%2F"
        queue: "example"
        body: '{ "type": "example" }'
        content_type: "application/json"
        durable: true
        headers: 
          x-max-priority: 15
        delegate_to: localhost

The task would fail adding the message to the queue, due to the headers not being passed in correctly. This pull request addresses that and adds the headers to the message.

Before:

fatal: [192.168.0.1 -> localhost]: FAILED! => changed=false 
  msg: 'Queue declare issue: (406, "PRECONDITION_FAILED - inequivalent arg ''x-max-priority'' for queue ''example'' in vhost ''/'': received none but current is the value ''15'' of type ''signedint''")'

After:

changed: [192.168.0.1-> localhost] => changed=true 
  result:
    content_type: application/json
    msg: Successfully published to queue example
    queue: example

It has also been tested without passing in headers, and the play still functions as before.

@V4UIDev
Copy link
Author

V4UIDev commented Aug 27, 2024

@Andersson007 @csmart could I get a review of this please?

@Andersson007
Copy link
Contributor

@V4UIDev hello, thanks for the PR!

@csmart
Copy link
Collaborator

csmart commented Aug 28, 2024

@V4UIDev thanks for the contribution and fixing up this missing component! I think some tests would be great, if you have the time. That will help us to keep on top of this in the future. Thanks!

@Andersson007
Copy link
Contributor

@V4UIDev if you have any questions, please let us know

@Andersson007
Copy link
Contributor

@csmart do you think if we can merge the PR w/o the requested integration tests as the author does not respond if i add the fragment in another PR? if you think it's dangerous, we could wait/close the PR, np

@V4UIDev
Copy link
Author

V4UIDev commented Oct 18, 2024

@Andersson007 @csmart Apologies for the delay. Changelog fragment and integration tests added, let me know if they need any further tweaking. Thanks!

@Andersson007
Copy link
Contributor

will be happy to review the PR again once the integration tests are green, thanks!

@V4UIDev
Copy link
Author

V4UIDev commented Oct 24, 2024

will be happy to review the PR again once the integration tests are green, thanks!

Yes, I think there is a rate limit that cloudsmith impose on rabbitmq so will put in a commit next month :)

@Andersson007
Copy link
Contributor

@V4UIDev @csmart are there maybe any other ways to securely install those packages? maybe something has changed on the packages provider site and now there are other URLs that are valid

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

Successfully merging this pull request may close these issues.

3 participants