fix: stomp-py 6.x callback arg-shape + DRY consolidation across gridappsd-python#210
Merged
Merged
Conversation
CI run 29548332271 traced to a dict object has no attribute headers error: stomp-py 6.x calls raw listener callbacks with two positional arguments (headers, body), but code at several call sites in goss.py assumed the stomp-py 8.x single Frame argument shape (frame.headers, frame.body). The bare except at goss.py:407 in CallbackRouter.run_callbacks was not implicated: it wraps callback dispatch, not argument unpacking. Extract _unpack_stomp_args to accept both shapes (two positional args for stomp-py pre-8, or a single object exposing .headers/.body for stomp-py 8.x, with a tuple fallback), and _serialize_message to centralize the dict/list to JSON string conversion. Apply both at all 6 former _STOMP_V8 sites so the module is correct against stomp-py 6.x (the version actually pinned in pyproject.toml) and against stomp-py 8.x without duplicated branching per site.
Parametrized coverage of _unpack_stomp_args against the two-positional- argument shape (stomp-py 6.x, the version pinned in pyproject.toml) and a fake Frame object exposing .headers/.body (stomp-py 8.x), plus _serialize_message. Assertions check the exact headers and body values returned, including dict body pass-through by identity, not just that the call does not raise.
…helper pause, stop, resume, and resume_pause_at each built and sent the same command dict shape independently. Extract _send_simulation_command so the wire format (command name, optional nested input dict) is defined once, and each wrapper becomes a one-line call. Add tests asserting the exact destination topic and JSON payload sent for the bare commands and for resumePauseAt's nested pauseIn input, and that each wrapper sets _running_or_paused.
set_application_status and set_service_status both wrote directly to
the shared _process_status field with the same try/except ValueError
handling for an invalid status string. Extract _set_status(status, kind)
so the conversion and the warning message are defined once; kind
labels the warning ("application" or "service") for the reader.
Add tests asserting that both setters mutate the same shared state
(visible through either getter), that the process starts in STARTING,
and that an invalid status string is ignored with the old value
retained rather than raised. The invalid-status tests set
GRIDAPPSD_APPLICATION_ID in the environment because the warning path
reads it via get_logger().warning() to build the log record; this
mirrors the precondition app_registration.py's registration callback
sets in production before application code runs.
ge, le, eq, and between each appended a differently-shaped filter dict to queryFilter using the same current-key-must-be-set precondition. Extract _add_filter(operator, value) so the dict shape and the where_key precondition check are defined once. Add tests asserting the exact queryFilter list contents and ordering produced by each filter method, that between appends ge then le in that order, that chained filters on different keys each keep their own entries, that calling a filter method before where_key raises ValueError, and that filter methods return self for chaining.
The GRIDAPPSD_APPLICATION_STATUS environment variable name and the os.environ write to it were duplicated at each of the 4 places Job.run and ApplicationController transitioned status. Extract _set_application_status(status) so the environment variable name and write are defined once, keyed by an ApplicationStatusEnum member instead of a raw string. Add tests asserting the exact string value written to the environment for each status, that a later write overwrites an earlier one, and that Job.run drives the environment variable through RUNNING then STOPPED for a command that exits cleanly, or RUNNING then ERROR for one that fails to launch.
craigpnnl
added a commit
that referenced
this pull request
Jul 17, 2026
Loosens the stomp-py dependency from ==6.0.0 to >=6.0.0 in gridappsd-python-lib/pyproject.toml, making it consistent with pixi.toml (already >=6.0.0). Justified by the GADP-044 fix (PR #210): _unpack_stomp_args now handles both the stomp-py 6.x two-arg callback shape and the 8.x/9.x Frame shape, so pinning to exactly 6.0.0 is artificially strict and produced a dependency-conflict warning in the platform build (gridappsd-python requires stomp-py==6.0.0 but the platform force-upgrades to 9.0.0 via test-stomp-topics). Verified under BOTH stomp-py 6.0.0 (floor) and 9.0.0 (latest): callback extracts (header, message) correctly using the real stomp.utils.Frame, full suite green, ruff/format/mypy clean. Note: gridappsd-field-bus-lib/info/requirements.txt still hard-pins stomp-py==6.0.0 but is a stale pip-freeze export not referenced by any build; left untouched, flagged separately.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause: stomp-py 6.0.0 (the pinned/installed version) invokes raw stomp listener callbacks with two positional args (headers, body), but several goss.py call sites assumed the stomp-py 8.x single Frame object shape (frame.headers / frame.body). At argument unpack time this called .headers on the headers dict, raising AttributeError: 'dict' object has no attribute 'headers'. This broke the simulation integration tests, surfacing downstream as a 30 second query_model_info TimeoutError, and was masked in CI by continue-on-error on the sim step.
Fix: a single _unpack_stomp_args() helper now handles both the stomp 6 two-arg shape and the stomp 8 Frame shape, applied at all 6 former call sites in goss.py. A companion _serialize_message() helper centralizes dict/list to JSON serialization.
Additional DRY consolidations included in this PR:
Test results: 51 pytest tests passed, ruff clean. Two pre-existing mypy errors remain in simulation.py (SimulationArgs.publish_period and interval typed int but defaulted None); these are unrelated to this change and are tracked in #208.
Closes #207
Closes #208.
Related follow-up: #209 tracks sharing the _unpack_stomp_args helper with gridappsd-field-bus-lib, which has its own duplicate of the same stomp arg-shape branch. That work is out of scope for this PR.
Related: #208 (pre-existing mypy type issue in simulation.py, not fixed by this PR).