fix: persist media server port and wait for waku connection status#16
Open
friofry wants to merge 2 commits into
Open
fix: persist media server port and wait for waku connection status#16friofry wants to merge 2 commits into
friofry wants to merge 2 commits into
Conversation
friofry
requested review from
Copilot and
nickninov
and removed request for
nickninov
There was a problem hiding this comment.
Pull request overview
This PR updates the SDK and CI to keep the media server on a fixed port (8081) across logout/re-initialization cycles, and adjusts messenger startup to wait on an updated Waku connection signal.
Changes:
- Add an end-to-end pytest that verifies the media server port (8081) persists across
Logout -> InitializeApplication -> LoginAccount. - Persist media server bind/advertise settings in
InitializeApplicationpayload generation. - Add a GitHub Actions workflow job that boots
status-backendvia Docker Compose and runs the new test.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_media_server_port.py | Adds an integration test using HTTP + websocket signals to validate media port persistence across logout/re-init. |
| requirements.txt | Adds pytest dependency to support running the new test in CI. |
| docker-compose.yaml | Exposes port 8081 and improves backend healthcheck robustness to support the CI test. |
| bot/account.py | Adds a fixed media_port to initialization payload and updates messenger startup to wait for a new Waku signal. |
| .github/workflows/ci.yml | Adds a CI job that builds/runs the backend and executes the new pytest. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1336
to
+1339
| self.logger.info("Starting messaging") | ||
| self.__call_rpc("messaging", "startMessenger") | ||
| self.__signal.get("wakuv2.peerstats") | ||
| with self.__signal.expect("waku.connection.status.change", timeout=60): | ||
| pass |
| - `port` - the port to connect to Status Backend. Verify the port in the Docker files. | ||
| - `is_secure` - if `http` or `https` should be used | ||
| - `backup_folder` - where backup files will be created and stored | ||
| - `media_port` - fixed media server port (advertized in localUrl as domain:media_port) |
Comment on lines
+119
to
+126
| def observed_port(signals: SignalBuffer, since: int) -> int | None: | ||
| signal_port = signals.latest_port("mediaserver.started", since) | ||
| if signal_port is not None: | ||
| return signal_port | ||
| if media_server_health_ok(MEDIA_PORT): | ||
| return MEDIA_PORT | ||
| return None | ||
|
|
Comment on lines
+74
to
+76
| def close(self): | ||
| self._stop = True | ||
|
|
friofry
force-pushed
the
fix/media-server-port
branch
3 times, most recently
from
86769e3 to
f4214ed
Compare
Pass media server bind/advertize options in InitializeApplication and replace the removed wakuv2.peerstats signal with waku.connection.status.change. Publish port 8081 in docker-compose and add a pytest + GitHub Actions check that the media server port survives logout/re-init.
friofry
force-pushed
the
fix/media-server-port
branch
from
f4214ed to
81e4252
Compare
Comment on lines
+1336
to
+1339
| self.logger.info("Starting messaging") | ||
| self.__call_rpc("messaging", "startMessenger") | ||
| self.__signal.get("wakuv2.peerstats") | ||
| with self.__signal.expect("waku.connection.status.change", timeout=60): | ||
| pass |
| - `port` - the port to connect to Status Backend. Verify the port in the Docker files. | ||
| - `is_secure` - if `http` or `https` should be used | ||
| - `backup_folder` - where backup files will be created and stored | ||
| - `media_port` - fixed media server port (advertized in localUrl as domain:media_port) |
Comment on lines
+121
to
+126
| def logout(signals: SignalBuffer | None = None) -> None: | ||
| since = signals.count("node.stopped") if signals is not None else 0 | ||
| try: | ||
| post_json("Logout") | ||
| except requests.RequestException: | ||
| return |
Comment on lines
+211
to
+214
| key_uid = ensure_account(signals, init_data) | ||
| login(key_uid, signals) | ||
| port1 = observed_port(signals, since) | ||
| assert port1 == MEDIA_PORT, f"first login: expected {MEDIA_PORT}, got {port1}" |
Comment on lines
+218
to
+221
| initialize() | ||
| login(key_uid, signals) | ||
| port2 = observed_port(signals, since) | ||
| assert port2 == MEDIA_PORT, f"after re-init: expected {MEDIA_PORT}, got {port2}" |
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
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.
Configure media server bind/advertize options in InitializeApplication, replace removed wakuv2.peerstats with waku.connection.status.change, and add a pytest + GitHub Actions check that port 8081 survives logout/re-init.
refs status-im/status-go#7611