Skip to content

Commit 238fb10

Browse files
Sam-BollingSam-Bolling
committed
fix(node): default _mqtt_client/_basic_auth/_client_session to None
Upstream's MQTT/event-bus refactor (406bd26) interacts badly with the dataclass-generated __eq__ on Node: three private fields are declared without defaults but only conditionally assigned in __init__, so comparing two Node instances raises AttributeError on any field that was never assigned. Adding = None defaults preserves the existing lazy-init pattern (rest of code already uses getattr(self, '_mqtt_client', None) defensively) and lets __eq__ work on every construction path. Also adds docs/research/Upstream_Sync_Merge_Report_2026-05-09.md documenting the merge, the conflict resolution, and this regression.
1 parent 0facebc commit 238fb10

2 files changed

Lines changed: 200 additions & 3 deletions

File tree

‎docs/research/Upstream_Sync_Merge_Report_2026-05-09.md‎

Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
# Upstream Sync Merge Report
2+
3+
**Date:** 2026-05-09
4+
**Branch:** `integrate/upstream-merge-2026-05-09`
5+
**Upstream:** `Botts-Innovative-Research/OSHConnect-Python` @ `main`
6+
**Fork:** `OS4CSAPI/OSHConnect-Python` @ `main` (commit `d8c4058`)
7+
8+
---
9+
10+
## 0. TL;DR
11+
12+
We merged **21 commits / 51 files / +5 048 / −506** of upstream work into our fork without losing any of our 132 ahead-commits. One textual conflict (`README.md`) was resolved by keeping our fork-focused content and appending upstream's MkDocs section. A single real regression — an `AttributeError` on `Node._mqtt_client` triggered by upstream's MQTT/event-bus refactor interacting with `@dataclass`'s auto-generated `__eq__` — was fixed in-place. Test suite: **144 passed, 2 skipped, 4 deselected** (the 4 are pre-existing network-dependent tests requiring a local OSH at `:8282`; they fail identically on `main` without the merge). All 9 publisher modules import cleanly. Branch ready to fast-forward into `main`.
13+
14+
---
15+
16+
## 1. Why we synced
17+
18+
Three drivers:
19+
20+
1. **Silent SensorML field loss bug** — Documented in `Silent_SensorML_Field_Loss_Engineering_Report_2026-05-06.md`. Upstream commit [`7e6ac05`](https://github.com/Botts-Innovative-Research/OSHConnect-Python/commit/7e6ac05c5d024a634a79e38d5865726fbd9095b8) (*"Prefer `AnyComponent` type over `SerializeAsAny` to prevent loss of data"*) is a direct fix in upstream code for the same root cause we observed locally.
21+
2. **Strict-parsing migration on `/csapi-go-v2`** — While debugging publisher rejections on the new Go server (see `Strict_Parsing_Migration_Spec_Grounded_Reanalysis_2026-05-09.md`), it became clear we were patching client-side issues that upstream had already fixed three weeks earlier in their data-model layer.
22+
3. **Drift cost** — We were 21 commits behind / 132 ahead. Each week of further drift makes the merge surface-area larger and the conflict resolution risk higher.
23+
24+
---
25+
26+
## 2. What we pulled in
27+
28+
### Library / data-model fixes (high relevance)
29+
30+
| Commit | Subject |
31+
|---|---|
32+
| [`ea73e22`](https://github.com/Botts-Innovative-Research/OSHConnect-Python/commit/ea73e228615f01b130b185af712a9fcfff0cef66) | Fix longstanding time comparison issue + a deserialization problem |
33+
| [`5541ccd`](https://github.com/Botts-Innovative-Research/OSHConnect-Python/commit/5541ccd492718f752a74422ea62b74446ce97080) | Fix incorrect serialization alias on `VectorSchema` |
34+
| [`7e6ac05`](https://github.com/Botts-Innovative-Research/OSHConnect-Python/commit/7e6ac05c5d024a634a79e38d5865726fbd9095b8) | **Prefer `AnyComponent` over `SerializeAsAny` to prevent data loss** |
35+
| [`5a4a970`](https://github.com/Botts-Innovative-Research/OSHConnect-Python/commit/5a4a97092aea512eb655fe9cce66d100a8eaf92c) | SWE components use `Literal[type_name]` for `type` field |
36+
| [`97cd5e2`](https://github.com/Botts-Innovative-Research/OSHConnect-Python/commit/97cd5e255e98e1a3e3c5a000cc3bdb7e785af00c) | Enforce SWE Common 3 SoftNamedProperty rule |
37+
| [`0d6ef64`](https://github.com/Botts-Innovative-Research/OSHConnect-Python/commit/0d6ef64c140010722735adbba8de8c57b6f1974f) | Bring data models in line with SWECommon 3.0 + validation |
38+
| [`04bee27`](https://github.com/Botts-Innovative-Research/OSHConnect-Python/commit/04bee27fd582cf698c01a2489dd6558e575c4bb2) | Discovery: more reliably attach subresources to parents |
39+
40+
### Feature additions (large, opinionated — accepted with the merge)
41+
42+
| Commit | Subject |
43+
|---|---|
44+
| [`98e63c3`](https://github.com/Botts-Innovative-Research/OSHConnect-Python/commit/98e63c32427a8f5ea34469f072ca61200b910451) | Event-bus framework (initial) |
45+
| [`95bdb6b`](https://github.com/Botts-Innovative-Research/OSHConnect-Python/commit/95bdb6ba932640eecb103153c616bf8efc70b025) | CSAPI Part 3 pub/sub topics, expanded public API |
46+
| [`d7e077e`](https://github.com/Botts-Innovative-Research/OSHConnect-Python/commit/d7e077e55c3a22c9de6c05cd83ee7abeb7547816) | Updates for Part 3 + datastores |
47+
| [`406bd26`](https://github.com/Botts-Innovative-Research/OSHConnect-Python/commit/406bd26e2bb454bebe1b6e44af6a0853b89c4de9) | Refactor event bus into `events/` sub-package, wire lifecycle/streaming |
48+
| [`2e15c29`](https://github.com/Botts-Innovative-Research/OSHConnect-Python/commit/2e15c2973c55fd3d7261133f9ea4470040ec0f08) | SQLite-backed `DataStore` |
49+
50+
### DevX / docs / CI
51+
52+
| Commit | Subject |
53+
|---|---|
54+
| [`de14a46`](https://github.com/Botts-Innovative-Research/OSHConnect-Python/commit/de14a46549c2b1aace8cb3c8583fc52ea5662db5) | Docker + local publish DevX |
55+
| [`853a028`](https://github.com/Botts-Innovative-Research/OSHConnect-Python/commit/853a028883700f6f1289b2cffd2be990bc57dd6c) | Version bump, docs, remove deprecated `to_lower_camel` |
56+
| `1070198`, `02ca719`, `981e5a4`, `3f4b595`, `fe8ae1d`, `5bd72d7`, `c194e2b` | MkDocs migration, GH-Pages workflow, test cleanup |
57+
58+
---
59+
60+
## 3. Conflict resolution
61+
62+
**Single conflict:** `README.md`.
63+
64+
- Our `HEAD` had a fork-focused README (publisher fleet quick-start, fleet summary table, 9 publisher cadence breakdown).
65+
- `upstream/main` had added a "Generating the Docs" MkDocs section.
66+
67+
**Resolution:** Kept our entire fork-focused content verbatim. Appended upstream's MkDocs section after our `## License` heading because that section documents real new files arriving with the merge (`mkdocs.yml`, `docs/markdown/*.md`, `.github/workflows/docs_pages.yaml` updates) and removing it would leave those files undocumented.
68+
69+
No other files conflicted. The 50 other modified/added files merged cleanly because our 132 ahead-commits are concentrated in `publishers/`, `docs/research/`, `scripts/`, `scenarios/` while upstream's 21 are concentrated in `src/oshconnect/` plus new top-level `docs/markdown/`, `mkdocs.yml`, and new tests.
70+
71+
---
72+
73+
## 4. The one real regression: `Node._mqtt_client` AttributeError
74+
75+
### Symptom
76+
77+
```
78+
tests/test_oshconnect.py::TestOSHConnect::test_oshconnect_add_node FAILED
79+
AttributeError: 'Node' object has no attribute '_mqtt_client'.
80+
Did you mean: 'get_mqtt_client'?
81+
```
82+
83+
Triggered by the dataclass-generated `__eq__` on the assertion `assert app._nodes[0] == node`.
84+
85+
### Root cause
86+
87+
`Node` in [`src/oshconnect/streamableresource.py`](../../src/oshconnect/streamableresource.py) is a `@dataclass(kw_only=True)` with a custom `__init__`. The class declares three private fields without defaults:
88+
89+
```python
90+
_basic_auth: bytes
91+
_client_session: OSHClientSession
92+
_mqtt_client: MQTTCommClient
93+
```
94+
95+
Each is assigned conditionally inside `__init__`:
96+
97+
| Field | Assigned only when |
98+
|---|---|
99+
| `_basic_auth` | `username` and `password` are both supplied (via `add_basicauth`) |
100+
| `_client_session` | `session_manager` is passed and `register_with_session_manager` resolves |
101+
| `_mqtt_client` | `kwargs.get('enable_mqtt')` is truthy |
102+
103+
Before upstream's MQTT refactor, the auto-generated `__eq__` happened not to be exercised on uninitialised attributes because the test surface didn't compare `Node` instances. Upstream's event-bus / MQTT refactor wired new code paths (and the new `tests/test_mqtt_topics.py`) that *do* construct and compare `Node`-like objects; combined with our existing `test_oshconnect_add_node` exercising `__eq__`, the latent bug surfaces. The dataclass `__eq__` does `getattr(self, field) == getattr(other, field)` for every declared field; missing attributes raise `AttributeError`.
104+
105+
This is a **pre-existing latent bug in upstream that was unmasked by the merge**, not a bug we introduced. It would have hit upstream's own CI as soon as anyone exercised `Node.__eq__` after [`406bd26`](https://github.com/Botts-Innovative-Research/OSHConnect-Python/commit/406bd26e2bb454bebe1b6e44af6a0853b89c4de9).
106+
107+
### Fix applied
108+
109+
Minimal: add `= None` defaults on the three conditionally-set private fields. Preserves the existing lazy-init pattern (the rest of the codebase already uses `getattr(self, '_mqtt_client', None)` defensively, so `None` is the documented sentinel).
110+
111+
```python
112+
# src/oshconnect/streamableresource.py
113+
_basic_auth: bytes = None
114+
_client_session: OSHClientSession = None
115+
_mqtt_client: MQTTCommClient = None
116+
```
117+
118+
This makes the dataclass-generated `__eq__` see real attribute values (including `None`) on every instance, eliminating the `AttributeError`.
119+
120+
### Why we're not filing upstream
121+
122+
User decision (2026-05-09): documented here instead. Rationale: upstream is on a Part-3 / event-bus push and our patch is one-line; carrying it locally has near-zero maintenance cost. If we re-sync and they've fixed it differently, the merge driver will resolve cleanly. If they haven't, our patch survives. Recorded here for traceability.
123+
124+
---
125+
126+
## 5. Test results after merge + fix
127+
128+
```
129+
============================= test session starts =============================
130+
collected 150 items
131+
132+
tests\test_api_helper.py 1 passed
133+
tests\test_bootstrap_roundtrip.py 4 passed, 2 skipped
134+
tests\test_datastore.py 21 passed
135+
tests\test_imports.py 14 passed
136+
tests\test_mqtt_topics.py 27 passed
137+
tests\test_oshconnect.py 3 passed, 4 deselected*
138+
tests\test_resource_datamodels.py 1 passed
139+
tests\test_schema_equivalence.py 2 passed
140+
tests\test_serialization.py 1 passed
141+
tests\test_streamable_resources.py 0 passed, 1 deselected*
142+
tests\test_swe_name_validation.py 35 passed
143+
tests\test_swe_schema_validation.py 35 passed
144+
145+
================ 144 passed, 2 skipped, 4 deselected in 9.31s =================
146+
```
147+
148+
\* The 4 deselected tests (`test_find_systems`, `test_oshconnect_find_datastreams`, `test_obs_ws_stream`, `test_streamble_observations`) require a live OSH server at `localhost:8282`. They are not merge regressions — they fail identically on pre-merge `main`. They are integration tests masquerading as unit tests.
149+
150+
### Publisher-fleet smoke tests
151+
152+
All 9 publisher bootstrap modules import successfully:
153+
154+
```
155+
oshconnect OK
156+
publishers.nws.bootstrap_nws OK
157+
publishers.iss.bootstrap_iss OK
158+
publishers.ndbc.bootstrap_ndbc OK
159+
publishers.coops.bootstrap_coops OK
160+
publishers.aviation_wx.bootstrap_aviation_wx OK
161+
publishers.opensky.bootstrap_opensky OK
162+
publishers.usgs_water.bootstrap_usgs_water OK
163+
publishers.usgs_nims.bootstrap_usgs_nims OK
164+
publishers.usgs_eq.bootstrap_usgs_eq OK
165+
```
166+
167+
This validates that nothing in upstream's data-model refactor broke our publisher serialisation paths at the import / class-definition level.
168+
169+
---
170+
171+
## 6. What this merge does NOT validate
172+
173+
- **Live publishing against `/csapi-go-v2`.** The strict-parsing rejections documented in `Strict_Parsing_Migration_Spec_Grounded_Reanalysis_2026-05-09.md` are server-side; this merge does not address them. We still need to either (a) implement dual-content publication (POST GeoJSON, PUT SensorML) per OGC 23-001, or (b) wait for the Go server to fix its own conformance gaps.
174+
- **Event-bus / Part 3 functionality.** None of our publishers exercise the new `events/` package or Part 3 pub/sub. We accept the new code surface but make no claim it works end-to-end against a real CSAPI Part 3 server.
175+
- **SQLite DataStore.** New, untested in our environment, but isolated behind `datastores/sqlite_store.py` and not pulled into the publisher hot path.
176+
177+
---
178+
179+
## 7. Recommended merge sequence
180+
181+
1. ✅ Branch `integrate/upstream-merge-2026-05-09` pushed
182+
2. ✅ README conflict resolved
183+
3. ✅ `Node._mqtt_client` regression fixed
184+
4. ✅ Test suite green (144 passed)
185+
5. ✅ All 9 publisher imports verified
186+
6. ⏳ Fast-forward `main` to `integrate/upstream-merge-2026-05-09`
187+
7. ⏳ Run one publisher with `--dry-run` against `/csapi-go-v2` to confirm no payload regressions
188+
8. ⏳ Resume strict-parsing remediation work on the now-current main
189+
190+
---
191+
192+
## 8. Cross-references
193+
194+
- [`Silent_SensorML_Field_Loss_Engineering_Report_2026-05-06.md`](Silent_SensorML_Field_Loss_Engineering_Report_2026-05-06.md) — original observation that motivated this sync
195+
- [`Strict_Parsing_Migration_Findings_Publisher_Fleet_2026-05-09.md`](Strict_Parsing_Migration_Findings_Publisher_Fleet_2026-05-09.md) — initial publisher rejection inventory
196+
- [`Strict_Parsing_Migration_Spec_Grounded_Reanalysis_2026-05-09.md`](Strict_Parsing_Migration_Spec_Grounded_Reanalysis_2026-05-09.md) — corrected remediation plan (server-side, not addressed by this merge)
197+
- Upstream commit range: [`d8c4058...upstream/main`](https://github.com/OS4CSAPI/OSHConnect-Python/compare/main...Botts-Innovative-Research:OSHConnect-Python:main)

‎src/oshconnect/streamableresource.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,11 +117,11 @@ class Node:
117117
server_root: str = 'sensorhub'
118118
endpoints: Endpoints
119119
is_secure: bool
120-
_basic_auth: bytes
120+
_basic_auth: bytes = None
121121
_api_helper: APIHelper
122122
_systems: list[System] = field(default_factory=list)
123-
_client_session: OSHClientSession
124-
_mqtt_client: MQTTCommClient
123+
_client_session: OSHClientSession = None
124+
_mqtt_client: MQTTCommClient = None
125125
_mqtt_port: int = 1883
126126

127127
def __init__(self, protocol: str, address: str, port: int,

0 commit comments

Comments
 (0)