Summary

Add support for checking broker connections using new settings.

Review Request #15402 — Created Oct. 16, 2026 and updated Oct. 29, 2026, 1:37 a.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-9.x

Bugs

Depends On

15231

Blocks

15407

Reviewers

Groups

reviewboard

People

Description

This introduces a check_connection() method on broker backends, which
takes saved or pending settings and checks that the broker can be
reached with them.

This will be used for the upcoming configuration form, and may be used
for server health checks later.

The Local Broker checks the writability of either the specified local
queue path or the common directory within the sitedir's data directory.
Due to the nature of local brokers, it's entirely possible that this
could succeed on the server performing the check but fail on a
separate misconfigured server. Still, best attempt.

Celery-based brokers will set up a temporary Celery instance using the
settings and then make a quick connection attempt to see if it succeeds.

This does make one path change for the Local Broker. Previously, queues
were stored in <datadir>/<hostname>/local-queue. Now it's
<datadir>/local-queue/<hostname>. This provides a better path to check
(especially for shared site directories) and also gives a stable symlink
point.

Testing Done

Unit tests pass.

Tested local broker validation with the upcoming admin form changes.

Commits

Summary	ID
Add support for checking broker connections using new settings. This introduces a `check_connection()` method on broker backends, which takes saved or pending settings and checks that the broker can be reached with them. This will be used for the upcoming configuration form, and may be used for server health checks later. The Local Broker checks the writability of either the specified local queue path or the common directory within the sitedir's data directory. Due to the nature of local brokers, it's entirely possible that this could succeed on the server performing the check but fail on a separate misconfigured server. Still, best attempt. Celery-based brokers will set up a temporary Celery instance using the settings and then make a quick connection attempt to see if it succeeds. This does make one path change for the Local Broker. Previously, queues were stored in `<datadir>/<hostname>/local-queue`. Now it's `<datadir>/local-queue/<hostname>`. This provides a better path to check (especially for shared site directories) and also gives a stable symlink point.	869993562f9b4910e1b28a5a5702cf588b925dd0

Summary

Add support for checking broker connections using new settings.

This introduces a `check_connection()` method on broker backends, which takes saved or pending settings and checks that the broker can be reached with them. This will be used for the upcoming configuration form, and may be used for server health checks later. The Local Broker checks the writability of either the specified local queue path or the common directory within the sitedir's data directory. Due to the nature of local brokers, it's entirely possible that this could succeed on the server performing the check but fail on a separate misconfigured server. Still, best attempt. Celery-based brokers will set up a temporary Celery instance using the settings and then make a quick connection attempt to see if it succeeds. This does make one path change for the Local Broker. Previously, queues were stored in `<datadir>/<hostname>/local-queue`. Now it's `<datadir>/local-queue/<hostname>`. This provides a better path to check (especially for shared site directories) and also gives a stable symlink point.

869993562f9b4910e1b28a5a5702cf588b925dd0

Issues

Description	From	Last Updated
If something in the typed path is a +x file, this loop will find it and say we're finished, but …	david	Oct. 29, 2026, 1:29 a.m.
If connection_for_write raises, the celery app could leak. How about: app = self._build_celery(broker_settings=broker_settings) try: conn = app.connection_for_write(connect_timeout=timeout_secs) try: conn.ensure_connection(max_retries=0, timeout=timeout_secs) …	david	Oct. 29, 2026, 1:31 a.m.
This (and the other unwritable tests) will fail if we're running tests in a docker environment. Can we skip if …	david	Oct. 29, 2026, 1:36 a.m.

flake8 passed.

JSHint passed.

reviewboard/broker/backends/local.py (Diff revision 1)

The issue has been resolved. Show all issues

If something in the typed path is a +x file, this loop will find it and say we're finished, but then makedirs will fail. Can we do another if not os.path.isdir(check_path): raise ValueError after the loop?

reviewboard/broker/base/backends.py (Diff revision 1)

The issue has been resolved. Show all issues

If connection_for_write raises, the celery app could leak. How about:

app = self._build_celery(broker_settings=broker_settings)

try:
    conn = app.connection_for_write(connect_timeout=timeout_secs)

    try:
        conn.ensure_connection(max_retries=0, timeout=timeout_secs)
    finally:
        conn.release()
finally:
    app.close()

reviewboard/broker/tests/test_local_backend.py (Diff revision 1)

The issue has been resolved. Show all issues

This (and the other unwritable tests) will fail if we're running tests in a docker environment. Can we skip if os.geteuid() == 0?

Change Summary:


Added protection against paths where a component is a file.
Added handling for connection_for_write() raising an exception.
Unit tests that need permission checks are skipped if run as root.

Commits:

	Summary	ID
	Add support for checking broker connections using new settings. This introduces a `check_connection()` method on broker backends, which takes saved or pending settings and checks that the broker can be reached with them. This will be used for the upcoming configuration form, and may be used for server health checks later. The Local Broker checks the writability of either the specified local queue path or the common directory within the sitedir's data directory. Due to the nature of local brokers, it's entirely possible that this could succeed on the server performing the check but fail on a separate misconfigured server. Still, best attempt. Celery-based brokers will set up a temporary Celery instance using the settings and then make a quick connection attempt to see if it succeeds. This does make one path change for the Local Broker. Previously, queues were stored in `<datadir>/<hostname>/local-queue`. Now it's `<datadir>/local-queue/<hostname>`. This provides a better path to check (especially for shared site directories) and also gives a stable symlink point.	c6d049b102a3275755f87f99664c157155c3ca36
	Add support for checking broker connections using new settings. This introduces a `check_connection()` method on broker backends, which takes saved or pending settings and checks that the broker can be reached with them. This will be used for the upcoming configuration form, and may be used for server health checks later. The Local Broker checks the writability of either the specified local queue path or the common directory within the sitedir's data directory. Due to the nature of local brokers, it's entirely possible that this could succeed on the server performing the check but fail on a separate misconfigured server. Still, best attempt. Celery-based brokers will set up a temporary Celery instance using the settings and then make a quick connection attempt to see if it succeeds. This does make one path change for the Local Broker. Previously, queues were stored in `<datadir>/<hostname>/local-queue`. Now it's `<datadir>/local-queue/<hostname>`. This provides a better path to check (especially for shared site directories) and also gives a stable symlink point.	869993562f9b4910e1b28a5a5702cf588b925dd0

Diff:

Revision 2 (+912 -10)

Show changes

	reviewboard/broker/backends/local.py
	reviewboard/broker/base/backends.py
	reviewboard/broker/tests/test_base_broker_backend.py
	reviewboard/broker/tests/test_base_celery_broker_backend.py
	reviewboard/broker/tests/test_local_backend.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.