[Kde-pim] Review Request 116826: add a new method setTemporaryOffline() to temporarily halt an agent to restart after some timeout

[Dan Vrátil]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116826/#review53162
-----------------------------------------------------------



akonadi/agentbase.h
<https://git.reviewboard.kde.org/r/116826/#comment37438>

    Maybe we could have a reasonable default, like 5 minutes?



akonadi/agentbase.cpp
<https://git.reviewboard.kde.org/r/116826/#comment37439>

    I think this should revert to online state that the resource was in before setTemporaryOnline is called.
    
    Imagine following case:
    1) Resource sends a network request
    2) Network goes down, AgentBase sends network to offline (setOfflineInternal(false))
    3) Network request fails, resource implementation calls setTemporaryOffline()
    4) slotTemporaryOfflineTimeout() is called, resource is set online
    
    This is obviously wrong, because you introduce transition Offline -> TemporaryOffline -> Online, which IMO makes no sense.
    
    
    I think we should only have transitions Offline -> TemporaryOffline -> Offline (but see the other comment below), and Online -> TemporaryOffline -> Online, in other words the initial and final states should be identical.



akonadi/agentbase.cpp
<https://git.reviewboard.kde.org/r/116826/#comment37440>

    When the resource is already offline, I think this method should do nothing and return right away. With the change I described above, we would have transitions Online -> TemporaryOffline -> Online, which is perfectly fine, and Offline -> TemporaryOffline -> Offline, which is more or less a no-op, with the same result as 
    
    if ( !d->mOnline) {
      return;
    }
    
    here.
    


- Dan Vrátil


On March 16, 2014, 1:29 p.m., Martin Koller wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116826/
> -----------------------------------------------------------
> 
> (Updated March 16, 2014, 1:29 p.m.)
> 
> 
> Review request for KDEPIM-Libraries, Dan Vrátil and Kevin Krammer.
> 
> 
> Repository: kdepimlibs
> 
> 
> Description
> -------
> 
> as described in the review request 115817 a resource which detects that it can currently not continue to work correctly
> shall set itself temporarily to offline and shall retry to continue with the pending actions after some timeout.
> 
> This patch adds a new method setTemporaryOffline() to agentbase for this functionality.
> 
> 
> Diffs
> -----
> 
>   akonadi/agentbase.h 4671c80 
>   akonadi/agentbase.cpp 4e51824 
>   akonadi/agentbase_p.h 3ead634 
> 
> Diff: https://git.reviewboard.kde.org/r/116826/diff/
> 
> 
> Testing
> -------
> 
> with the DAV resource against an owncloud 6 Server
> 
> 
> Thanks,
> 
> Martin Koller
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/