SecretService: Add timeout option to all methods

[mitya57]

Previously, when the Secret Service daemon is improperly configured, or the user cannot see the prompt dialog for some reason, all operations that involved collection.unlock() hung indefinitely. One example of this is pypa/twine#1276.

In mitya57/secretstorage#33, I added a timeout argument to the unlock() method in SecretStorage. In this PR, my goal is to provide end applications with a way to pass timeout to the underlying SecretStorage backend. For now, I implemented it in a naive way: just added a timeout argument to get_preferred_collection() and all methods that call it.

But I don’t much like the fact that one backend has this argument but the others don’t. So I am marking this PR as draft to discuss possible alternatives. Maybe it should be a class-level attribute, that this backend will support and the others will ignore? Or I should make the functions in keyring.core accept this argument, forward to the SecretService backend and silently ignore for other backends?

@jaraco Your opinion would be the most important here.

@woodruffw @sigmavirus24 Since it affects twine, twine will likely be a user of the new argument, so your feedback would be welcome too.

P.S. SecretStorage 3.5 requires Python ≥ 3.10. Are there any objections if I make it required in Keyring too? Currently we support Python 3.9 but the tests failed for it.

[woodruffw]

Since it affects twine, twine will likely be a user of the new argument, so your feedback would be welcome too.

No objection from me on the twine side -- adding a timeout makes sense to me, and would unblock the upstream behavior on twine.

P.S. SecretStorage 3.5 requires Python ≥ 3.10. Are there any objections if I make it required in Keyring too? Currently we support Python 3.9 but the tests failed for it.

Noting for twine: I suspect we'll want to drop Python 3.9 support soon anyways (since it's EOL), so this probably own't be a concern for us.

[michaelk83]

Without going into the details of the linked threads yet, note that the underlying DBus calls should intrinsically time out after ~30 sec or so. No explicit timeout parameters should be necessary anywhere. Instead, whatever layer interfaces directly to DBus, should be bubbling the timeout error back to the clients (i.e. aborting the call).

[mitya57]

@michaelk83 In this case, it’s not a D-Bus method call that times out. We are waiting for a signal that the user accepted the prompt or dismissed it, and not getting one.

It only happens in rare situations when it’s not possible to show the dialog for some reason, for example on headless systems.

[michaelk83]

Ah, I see, a timeout on waiting for the user, effectively. That makes more sense.