Skip to content

Fix ChainerBackend.delete_password to try all backends#740

Open
veeceey wants to merge 5 commits intojaraco:mainfrom
veeceey:fix/issue-697-chainer-delete
Open

Fix ChainerBackend.delete_password to try all backends#740
veeceey wants to merge 5 commits intojaraco:mainfrom
veeceey:fix/issue-697-chainer-delete

Conversation

@veeceey
Copy link

@veeceey veeceey commented Feb 13, 2026

Summary

Fixes #697 where ChainerBackend.delete_password() was only trying the first backend instead of iterating through all backends in the chain.

The issue occurred when a password existed in a lower priority backend but not in the highest priority one. The delete_password method would try only the first (highest priority) backend, get a PasswordDeleteError, and fail immediately.

Changes

  • Updated ChainerBackend.delete_password() to iterate through all backends, similar to how get_password() works
  • Now catches PasswordDeleteError exceptions and continues trying other backends
  • If all backends fail to delete the password, raises the last PasswordDeleteError encountered
  • Added comprehensive test coverage for the new behavior

Testing

Added two new test cases:

  1. test_delete_password_tries_all_backends - Verifies that deletion succeeds when password exists only in a lower priority backend
  2. test_delete_password_raises_if_all_backends_fail - Verifies that appropriate error is raised when password doesn't exist in any backend

All existing tests continue to pass.

The delete_password method was only trying the first backend in the
chain and returning immediately. This caused it to fail when a password
existed in a lower priority backend but not in the highest priority one.

Now delete_password iterates through all backends, catching
PasswordDeleteError exceptions and continuing to try other backends,
similar to how it already handles NotImplementedError. If all backends
fail to delete the password, it raises the last PasswordDeleteError.

This matches the behavior of get_password which also iterates through
all backends to find a password.

Fixes jaraco#697
Use 'if keyring == self.backends[-1]: raise' instead of
collecting errors in a list - cleaner and avoids the unnecessary
list allocation.
Simplify HighPriorityKeyring to always return None / raise,
remove unused storage logic, and exercise set_password stubs
so all new lines in the diff are covered.
Remove type annotation from storage class variable to avoid
triggering mypy's annotation-unchecked note on Windows CI.
@veeceey
Copy link
Author

veeceey commented Feb 16, 2026

Pushed a fix for the Windows CI mypy failure — the storage: dict = {} class-level type annotation was triggering mypy's annotation-unchecked note. Removed the type annotation since the __init__ method sets self.storage anyway.

The other two CI failures (Windows 3.9 pip install failure and pypy3.10 timeout/cancellation) appear to be transient infrastructure issues unrelated to these changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ChainerBackend not deleting password from lower priority backend

2 participants

Comments