After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 754508 - [review] refactor agent-manager and asynchronous functions [th/secret-requests-bgo754508]
[review] refactor agent-manager and asynchronous functions [th/secret-request...
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2015-09-03 10:54 UTC by Thomas Haller
Modified: 2015-09-18 12:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Description Thomas Haller 2015-09-03 10:54:19 UTC 
For lr/applied-connection-bgo724041 (bug 724041), we will have to extend requesting secrets to handle a combination of applied-connection and settings-connection.

While looking into that, I had problems understanding NMAgentManager, so here is a refactoring of that class.

I also move parts from lr/applied-connection-bgo724041 onto this branch.

Especially, I also refactor the handling of cancelling asynchronous functions.
I really think the proper pattern for many use-cases is to always invoke the callback -- even for cancellation.



Please review
Comment 1 Lubomir Rintel 2015-09-09 17:50:03 UTC 
LGTM

(Tested for two days; no issues noted).
Comment 2 Dan Williams 2015-09-11 14:44:19 UTC 
LGTM; we should double-check that secrets requests for PPPoE and WWAN still work though, just to make sure.
Comment 3 Thomas Haller 2015-09-18 12:31:01 UTC 
(In reply to Dan Williams from comment #2)
> LGTM; we should double-check that secrets requests for PPPoE and WWAN still
> work though, just to make sure.

I have some problems with testing this due to missing test setup.

From code review it seems that the change probably doesn't cause any issue.
I'd go ahead and merge it.

I'd like to run our RH internal tests, but master fails there currently already. So, I'd rather merge this first, and fix the tests (or any fallout) later.