galaxyproject / galaxyproject/pulsar
Extract the resilience harness and recorder into an installable package
- Dominant language
- Python
- Stars
- 46
- Forks
- 62
- Avg merge
- 3d 15h
- Merged PRs (30d)
- 14
Description
*This issue was drafted and posted by Claude (AI assistant) on jmchilton's behalf.*
## Summary
The resilience test framework's `harness` and `recorder` modules are importable only
because of an accident of `__init__.py` placement. #489 had to name them in
`known_first_party` so isort would stop filing them as third-party libraries. That
config entry works, but it is papering over the layout. Making them a real package
removes it, and unlocks reuse that is not possible today.
## How the imports actually resolve
`test/resilience/scenarios/` has an `__init__.py`; `test/resilience/` does not. So pytest
walks up from a scenario file, stops at the first directory without `__init__.py`, and
puts **`test/resilience`** on `sys.path`:
```
test/resilience/scenarios/test_broker_outage.py
sys.path gets: /test/resilience
imported as: scenarios.test_broker_outage
```
That `sys.path` entry is the entire reason bare `from harness.assertions import ...`
works, across `conftest.py` and 7 scenario files. Adding `test/resilience/__init__.py` —
the intuitive tidy-up — *breaks* all of them, because the `sys.path` entry moves to the
repo root and `harness` stops being top-level.
`recorder` is a second, unrelated mechanism: `test/resilience/mock_galaxy/` has no
`__init__.py`, and `mock_galaxy/Dockerfile` copies the two files flat into the image:
```dockerfile
COPY mock_galaxy/app.py /app/app.py
COPY mock_galaxy/recorder.py /app/recorder.py
CMD ["python", "/app/app.py"]
```
so `app.py` does `from recorder import StatusRecorder # type: ignore`. The
`# type: ignore` is there because mypy cannot see the module from the repo root either.
## Why this is a small job
The two modules worth extracting are the featherweight half of the framework:
| module | dependencies |
|---|---|
| `harness/*.py` | `requests` + stdlib |
| `recorder.py` | **pure stdlib** (`threading`, `time`, `collections`) |
| `mock_galaxy/app.py` | fastapi, uvicorn, kombu, a2wsgi, simple-job-files |
`harness` never imports `recorder` — they talk over HTTP (`/_recorder/events`,
`/_recorder/clear`). Only `app.py` carries the heavy dependencies, and it runs solely
inside the container, so it can sit behind an extra.
## There is already a pattern for this in the repo
`pulsar/client/test/` is test-support code shipped inside the installed distribution —
it is in `setup.py`'s `packages` list and backs the `pulsar-check` console script. Its
`test_common.py` opens with:
```python
""" For code shared between test_utils.py and check.py. Not sure this is the
best place - but check needs to not depend on test_utils so run_client_tests
can execute without webob dependency.
"""
```
So "where does shared test-support code live" was already asked and left unresolved. The
resilience framework hit the same question later and answered it with a `sys.path`
asymmetry instead.
## Proposal
Move `harness` and `recorder` into a real package:
```
/
harness/ assertions, broker_control, job_factory, pulsar_control # requests
recorder.py # stdlib
server/app.py # [mock-server] extra
```
and register `test/resilience/conftest.py`'s fixtures (`compose_up`, the mode
parametrization, the recorder reset) as a **pytest plugin via the `pytest11` entry
point**, rather than leaving them reachable only at a magic conftest path.
## Payoffs
- `known_first_party` in `.isort.cfg` goes away entirely.
- `# type: ignore` on the recorder import goes away — mypy can see an installed package.
- The `sys.path` / `__init__.py` asymmetry stops being load-bearing, so
`test/resilience/__init__.py` becomes safe to add.
- The Dockerfile's flat `COPY` becomes a package install or a package-directory copy.
- **Galaxy could consume it.** A fake Galaxy plus ordering assertions for Pulsar's
job-state delivery is exactly what Galaxy wants on its side of the same wire; today it
is trapped in Pulsar's test tree.
## Open questions
1. Subpackage (`pulsar.testing.*`) or separate distribution? Turns on whether Galaxy
consuming this is a real goal — Galaxy should not have to install the Pulsar server to
get a test harness. Subpackage is less machinery and matches the `pulsar/client/test/`
precedent.
2. Name. "replay" would be misleading — nothing here replays recorded traffic; the
recorder accumulates status events in memory and the harness asserts on their
ordering. `pulsar_mock_galaxy` reuses the name already in the tree and in the compose
file.
3. Does `app.py` move too, or stay container-local? It is the only piece with heavy
dependencies, but it imports `recorder`, so leaving it behind means keeping the flat
`COPY`.
Contributor guide
Research direction
Start by reading setup.py, test/resilience/conftest.py, the harness modules, recorder.py, and mock_galaxy/Dockerfile. Run the resilience scenarios before changing the package layout. Done means the shared harness and recorder have an agreed installable location, imports no longer depend on pytest's sys.path behavior, and the existing resilience tests and container still work.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- docker, fastapi, python
- Domain
- build-system, testing-qa, tooling
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100