Added support for amqps protocol #1473
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for the amqps (secure AMQP over TLS/SSL) protocol to Flower's broker monitoring functionality. The changes ensure that when using amqps:// broker URLs, Flower correctly uses HTTPS for the RabbitMQ Management API with the appropriate default port (15671 instead of 15672).
Key changes:
- Extended transport protocol checks to include both 'amqp' and 'amqps' in broker and task API views
- Added scheme detection to automatically select the correct HTTP scheme (https for amqps, http for amqp) and default management API port (15671 for amqps, 15672 for amqp)
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| flower/views/broker.py | Updated broker transport check to support 'amqps' alongside 'amqp' |
| flower/utils/broker.py | Added scheme attribute to BrokerBase; implemented conditional logic for default port (15671 vs 15672) and HTTP scheme (https vs http) based on broker protocol |
| flower/api/tasks.py | Updated transport check in queue length API to support 'amqps' alongside 'amqp' |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| http_scheme = 'https' if self.scheme == 'amqps' else 'http' | ||
| http_api = f"{http_scheme}://{self.username}:{self.password}@{self.host}:{self.port}/api/{self.vhost}" |
There was a problem hiding this comment.
The new logic for determining the http_scheme (https for amqps, http for amqp) lacks test coverage. Consider adding tests that verify the generated http_api URL uses the correct scheme: https://... for amqps:// broker URLs and http://... for amqp:// broker URLs.
|
|
||
| self.host = self.host or 'localhost' | ||
| self.port = self.port or 15672 | ||
| self.port = self.port or (15671 if self.scheme == 'amqps' else 15672) |
There was a problem hiding this comment.
The default port logic for amqps (15671) differs from amqp (15672), but the existing test in tests/unit/utils/test_broker.py at line 35-42 (test_url_defaults_rabbitmq) expects both amqp:// and amqps:// to use port 15672. This test will fail with these changes and should be updated to verify that amqps:// defaults to port 15671 while amqp:// defaults to 15672.
1 participant
No description provided.