Skip to content

fix(network): Make forget() and forget_vpn() idempotent#200

Merged
cachebag merged 2 commits intocachebag:masterfrom
ruthwik-01:master
Jan 21, 2026
Merged

fix(network): Make forget() and forget_vpn() idempotent#200
cachebag merged 2 commits intocachebag:masterfrom
ruthwik-01:master

Conversation

@ruthwik-01
Copy link
Contributor

@ruthwik-01 ruthwik-01 commented Jan 20, 2026

Summary

Closes #186


This PR makes the forget() and forget_vpn() methods idempotent, just like how disconnect_vpn() behaves.

Previously, calling these methods on a non-existing network or VPN would return a NoSavedConnection error. Now, they return Ok(()) even if the target connection is not present, making them more user-friendly.


Changes

  • Updated forget() to not throw error if no matching Wi-Fi connections exist.
  • Updated forget_vpn() to not fail if the VPN is already disconnected or missing.
  • Updated docstrings that follows this new idempotent behavior.

Copy link
Owner

@cachebag cachebag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you take a look at the conflict? Make sure you rebase off of master.

@neonbit101
For reference, we should include the code for checking that device_type is bluetooth, but add your changes for returning Ok() to match this PR's intent.

This LGTM otherwise, thank you!

@cachebag cachebag added nmrs Changes to nmrs refactor Change or improve code labels Jan 20, 2026
@ruthwik-01 ruthwik-01 force-pushed the master branch 3 times, most recently from d881a8a to 7769deb Compare January 21, 2026 08:50
@ruthwik-01
Copy link
Contributor Author

Hey @cachebag, I have made the changes regarding the merge conflicts.
Please have a look and let me know if anything else is needed.
Thanks!

@cachebag cachebag self-requested a review January 21, 2026 10:59
@cachebag cachebag merged commit be9d0ed into cachebag:master Jan 21, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

nmrs Changes to nmrs refactor Change or improve code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] nmrs: inconsistent error handling for "Not Found" cases

2 participants