refresh ethernet interfaces immediately after write actions

[nicoschmdt]

fix #3076

Summary by Sourcery

Refresh ethernet interfaces immediately after configuration changes and improve UI loading behavior for ethernet management.

Bug Fixes:

• Ensure ethernet interfaces are re-fetched after creating, deleting, or modifying addresses, DHCP servers, or triggering dynamic IP to keep the UI in sync with backend state.
• Prevent the ethernet interfaces list from getting stuck in an updating state by centralizing updating flag handling in the store and updater component.

Enhancements:

• Introduce a dedicated action to refresh available ethernet interfaces and reuse it across ethernet-related actions.
• Adjust ethernet manager UI conditions to show loading indicators while interfaces are updating and only show the 'no interfaces' message when none are available.
• Change the ethernet updater to perform an immediate initial fetch before starting the periodic refresh task and disable automatic start of the polling task.

Chores:

• Remove temporary caching on the backend endpoint that retrieves ethernet interfaces to always return current interface information.

[sourcery-ai]

Hey - I've left some high level feedback:

• There’s a lot of repeated setUpdatingInterfaces(true) / refreshInterfaces / setUpdatingInterfaces(false) logic across actions; consider extracting a small helper or wrapper action to centralize this pattern and reduce duplication.
• Now that setInterfaces no longer toggles updating_interfaces, it would be good to verify every path that triggers an interface refresh explicitly sets and clears updating_interfaces in the corresponding actions to avoid inconsistent spinner behavior.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- There’s a lot of repeated `setUpdatingInterfaces(true)` / `refreshInterfaces` / `setUpdatingInterfaces(false)` logic across actions; consider extracting a small helper or wrapper action to centralize this pattern and reduce duplication.
- Now that `setInterfaces` no longer toggles `updating_interfaces`, it would be good to verify every path that triggers an interface refresh explicitly sets and clears `updating_interfaces` in the corresponding actions to avoid inconsistent spinner behavior.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[github-actions]

Automated PR Review

0. Summary

• Verdict: DOES NOT SAIL 🪨
• Critical items to address: 1.1

Removes the 10 s cache on the cable_guy /ethernet GET, centralizes the ethernet "updating" flag inside the store, and dispatches a refreshInterfaces in the .finally of every write action so the UI reflects backend state immediately. EthernetManager.vue switches to v-show so the expansion panels keep their state across refreshes.

1. Correctness & Implementation Bugs

• 1.1 [major] core/frontend/src/components/ethernet/EthernetUpdater.vue:15 — adding autostart: false to the OneMoreTime options breaks the periodic 5 s background poll. OneMoreTime.setAction ends with softStart(), which only starts when (this.options.autostart ?? true) is truthy (one-more-time.ts:105-109, 117-121). With autostart: false, the call to setAction(this.fetchAvailableEthernetInterfaces) in mounted() registers the action but never schedules it — the task stays idle forever. Net effect: the only refreshes that ever happen are the one in mounted and the ones triggered by write actions; external state changes (cable plug/unplug, settings changed by another client) will no longer be picked up. Fix one of:

  • drop autostart: false (the original constructor without an action would not have started immediately anyway, since softStart short-circuits when this.action is undefined), or
  • explicitly call this.fetch_available_interfaces_task.start() after setAction(...), mirroring the pattern used in core/frontend/src/views/ExtensionManagerView.vue:718 and core/frontend/src/store/video.ts:33 / :310.

  If periodic polling is intentionally being removed, delete the OneMoreTime and the setAction call entirely instead of leaving dead code.

• 1.2 [minor] core/frontend/src/store/ethernet.ts:185-189 — getAvailableEthernetInterfaces's .catch still commits setInterfaces([]), but setInterfaces no longer flips updating_interfaces to false. After this PR, a transient failure during the post-write refreshInterfaces will wipe the cached interface list, then the calling action's .finally immediately commits setUpdatingInterfaces(false) — so the user sees the panels disappear and "No ethernet interfaces available" flash up after a successful-but-slow write whose follow-up GET happens to time out. Consider having refreshInterfaces swallow the error without overwriting available_interfaces, or stop clearing the list inside getAvailableEthernetInterfaces's catch.

4. Performance

• 4.1 [minor] core/services/cable_guy/main.py:39 — dropping @temporary_cache(timeout_seconds=10) is fine in principle, but manager.get_ethernet_interfaces() ultimately calls get_interfaces(...) which iterates the pyroute2 NDB. Once finding 1.1 is fixed, this endpoint will be called every 5 s by the poll plus once per write action, and is no longer coalesced with the other internal call sites of get_ethernet_interfaces that previously benefited from the same cache (api/manager.py:395, :886). Worth a quick check that NDB dump latency is acceptable on a Pi under load — if not, push the cache down to manager.get_ethernet_interfaces instead of removing it.

6. Code Quality & Style

• 6.1 [nit] core/frontend/src/store/ethernet.ts:32-35 — removing the this.updating_interfaces = false side effect from the setInterfaces mutation is fine and matches the new "the action owns the loading flag" model, but the mutation now does exactly what its name says and nothing more, so the previous behavior is no longer surprising. Consider a one-line note in the action that refreshInterfaces deliberately does not clear updating_interfaces so callers must clear it themselves.

7. Tests

• 7.1 [nit] No frontend tests added/changed. Not a blocker (the project doesn't gate frontend coverage), but the behavior of refreshInterfaces and the new .finally chains would be straightforward to unit test against a mocked axios.

Generated by PR Review Bot. This is advisory, a human reviewer must still approve.

[nicoschmdt]

1. Correctness & Implementation Bugs

• 1.2 [minor] core/frontend/src/store/ethernet.ts:185-189 — getAvailableEthernetInterfaces's .catch still commits setInterfaces([]), but setInterfaces no longer flips updating_interfaces to false. After this PR, a transient failure during the post-write refreshInterfaces will wipe the cached interface list, then the calling action's .finally immediately commits setUpdatingInterfaces(false) — so the user sees the panels disappear and "No ethernet interfaces available" flash up after a successful-but-slow write whose follow-up GET happens to time out. Consider having refreshInterfaces swallow the error without overwriting available_interfaces, or stop clearing the list inside getAvailableEthernetInterfaces's catch.

This behavior was already present in the code so this seems a little out of scope to me