Repository navigation
forecast pvnode: lowercase name - #4312
seaspotter wants to merge 2 commits into
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="packages/helpermodules/update_config.py" line_range="4225-4226" />
<code_context>
+ if topic == "openWB/optional/forecast/provider":
+ provider = decode_payload(payload)
+ if provider.get("type") == "pvnode" and provider.get("name") == "PVNode V2":
+ provider["name"] = "pvnode V2"
+ return {topic: provider}
+ return None
+
</code_context>
<issue_to_address>
**Newer provider settings are overwritten**
When the UI saves a provider after the updater reads its retained snapshot but before the migration publishes, `upgrade_datastore_159` publishes the provider from `all_received_topics`, replacing the newer settings with stale configuration and losing the UI edit.
Before publishing the rename, re-read and update the current retained provider or otherwise ensure the publish cannot overwrite a newer settings write.
</issue_to_address>
### Comment 2
<location path="packages/helpermodules/update_config.py" line_range="4230" />
<code_context>
+ return None
+
+ self._loop_all_received_topics(upgrade)
+ self._append_datastore_version(159)
</code_context>
<issue_to_address>
**Completed migration leaves old name**
When the provider publish is lost or rejected but the datastore-version publish succeeds, `_append_datastore_version(159)` records completion despite the failed provider publish, so the next startup skips the migration and the retained provider keeps its old name.
Append datastore version 159 only after the provider publish is confirmed successful, and leave the migration pending or retry when the publish fails.
</issue_to_address>
### Comment 3
<location path="packages/helpermodules/update_config.py" line_range="4224" />
<code_context>
+ def upgrade(topic: str, payload) -> Optional[dict]:
+ if topic == "openWB/optional/forecast/provider":
+ provider = decode_payload(payload)
+ if provider.get("type") == "pvnode" and provider.get("name") == "PVNode V2":
+ provider["name"] = "pvnode V2"
+ return {topic: provider}
</code_context>
<issue_to_address>
**String provider migration gets skipped**
When the stored forecast provider payload is a string or null, `provider` is a string or `None`, so `.get()` raises; `_loop_all_received_topics` catches the exception and `_append_datastore_version(159)` still marks the migration complete, leaving the stored name unchanged.
Check that the decoded provider is a dictionary before calling `.get()`, and only mark the migration complete after any required update succeeds.
</issue_to_address>
### Comment 4
<location path="packages/modules/forecast/pvnode/config.py" line_range="13" />
<code_context>
@dataclass
class PvNode:
- name: str = "PVNode V2"
+ name: str = "pvnode V2"
type: str = "pvnode"
official: bool = False
</code_context>
<issue_to_address>
**Forecast wiki remains outdated**
The code changes update the provider default and stored configurations, but this PR includes no change to the linked wiki page or indication that the requested handoff was made; readers of the forecast guide therefore do not get the requested lowercase branding.
Update the linked wiki page or record that the requested handoff to Andreas has been made.
</issue_to_address>| provider["name"] = "pvnode V2" | ||
| return {topic: provider} |
There was a problem hiding this comment.
🟠 High · Newer provider settings are overwritten
When the UI saves a provider after the updater reads its retained snapshot but before the migration publishes, upgrade_datastore_159 publishes the provider from all_received_topics, replacing the newer settings with stale configuration and losing the UI edit.
Before publishing the rename, re-read and update the current retained provider or otherwise ensure the publish cannot overwrite a newer settings write.
Prompt for AI agents
In `packages/helpermodules/update_config.py` at lines 4225-4226:
**Newer provider settings are overwritten**
When the UI saves a provider after the updater reads its retained snapshot but before the migration publishes, `upgrade_datastore_159` publishes the provider from `all_received_topics`, replacing the newer settings with stale configuration and losing the UI edit.
Before publishing the rename, re-read and update the current retained provider or otherwise ensure the publish cannot overwrite a newer settings write.| return None | ||
|
|
||
| self._loop_all_received_topics(upgrade) | ||
| self._append_datastore_version(159) |
There was a problem hiding this comment.
🟡 Medium · Completed migration leaves old name
When the provider publish is lost or rejected but the datastore-version publish succeeds, _append_datastore_version(159) records completion despite the failed provider publish, so the next startup skips the migration and the retained provider keeps its old name.
Append datastore version 159 only after the provider publish is confirmed successful, and leave the migration pending or retry when the publish fails.
Prompt for AI agents
In `packages/helpermodules/update_config.py` at line 4230:
**Completed migration leaves old name**
When the provider publish is lost or rejected but the datastore-version publish succeeds, `_append_datastore_version(159)` records completion despite the failed provider publish, so the next startup skips the migration and the retained provider keeps its old name.
Append datastore version 159 only after the provider publish is confirmed successful, and leave the migration pending or retry when the publish fails.| def upgrade(topic: str, payload) -> Optional[dict]: | ||
| if topic == "openWB/optional/forecast/provider": | ||
| provider = decode_payload(payload) | ||
| if provider.get("type") == "pvnode" and provider.get("name") == "PVNode V2": |
There was a problem hiding this comment.
🟡 Medium · String provider migration gets skipped
When the stored forecast provider payload is a string or null, provider is a string or None, so .get() raises; _loop_all_received_topics catches the exception and _append_datastore_version(159) still marks the migration complete, leaving the stored name unchanged.
Check that the decoded provider is a dictionary before calling .get(), and only mark the migration complete after any required update succeeds.
Prompt for AI agents
In `packages/helpermodules/update_config.py` at line 4224:
**String provider migration gets skipped**
When the stored forecast provider payload is a string or null, `provider` is a string or `None`, so `.get()` raises; `_loop_all_received_topics` catches the exception and `_append_datastore_version(159)` still marks the migration complete, leaving the stored name unchanged.
Check that the decoded provider is a dictionary before calling `.get()`, and only mark the migration complete after any required update succeeds.| @dataclass | ||
| class PvNode: | ||
| name: str = "PVNode V2" | ||
| name: str = "pvnode V2" |
There was a problem hiding this comment.
⚪ Low · Forecast wiki remains outdated
The code changes update the provider default and stored configurations, but this PR includes no change to the linked wiki page or indication that the requested handoff was made; readers of the forecast guide therefore do not get the requested lowercase branding.
Update the linked wiki page or record that the requested handoff to Andreas has been made.
Prompt for AI agents
In `packages/modules/forecast/pvnode/config.py` at line 13:
**Forecast wiki remains outdated**
The code changes update the provider default and stored configurations, but this PR includes no change to the linked wiki page or indication that the requested handoff was made; readers of the forecast guide therefore do not get the requested lowercase branding.
Update the linked wiki page or record that the requested handoff to Andreas has been made.
UI: openWB/openwb-ui-settings#1167
Vom pvnode-Team wurde der Wunsch an mich herangetragen, dass der Name komplett klein geschrieben wird, weil das ihr Eigenname dafür ist, kleingeschrieben.
Bitte auch im Wiki anpassen bzw. an Andreas weitergeben: https://wiki.openwb.de/doku.php?id=openwb:vc:2.3.x:anwendungen:pv-prognose
Dankeschön :)
Summary by Sourcery
Standardize the pvnode provider branding to lowercase across configuration, logging, and persisted datastore data.
Enhancements:
Chores: