Re: [pgAdmin4][Patch]: RM 1253 - Store and reload current location in treeview

Hi

On Thu, Oct 4, 2018 at 1:12 PM Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

> Hi,
>
> Please find the attached updated patch.
>

This seems to behave quite strangely - only sporadically saving the
treeview state, and then not always (maybe ever?) restoring the selected
node. When I watch the Python console, even with the save interval set to 1
second, I very rarely see the state being saved. For example:

2018-10-04 15:14:01,341: INFO werkzeug: 127.0.0.1 - - [04/Oct/2018
15:14:01] "GET /browser/database/children/1/1/44243 HTTP/1.1" 200 -
2018-10-04 15:14:01,355: INFO werkzeug: 127.0.0.1 - - [04/Oct/2018
15:14:01] "POST /settings/save_tree_state/ HTTP/1.1" 200 -
2018-10-04 15:14:02,357: INFO werkzeug: 127.0.0.1 - - [04/Oct/2018
15:14:02] "POST /settings/save_tree_state/ HTTP/1.1" 200 -
2018-10-04 15:14:23,459: INFO werkzeug: 127.0.0.1 - - [04/Oct/2018
15:14:23] "GET /preferences/ HTTP/1.1" 200 -

You can see there it called it twice, then 21 seconds later I opened the
preferences panel to check what the interval was set to (it was still 1
second). Even as I type this some minutes later, it still hasn't called it
again. I think it's essential that works reliably, as it's so unreliable to
do it on Window close.

>
> Thanks,
> Khushboo
>
> On Mon, Oct 1, 2018 at 7:14 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> Hi
>>
>> On Mon, Oct 1, 2018 at 1:53 PM Khushboo Vashi <
>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Dave
>>>
>>>
>>> On Mon, Oct 1, 2018 at 4:02 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>>
>>>>
>>>> On Mon, Oct 1, 2018 at 11:06 AM Khushboo Vashi <
>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Tue, Sep 25, 2018 at 6:26 PM Khushboo Vashi <
>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Tue, Sep 25, 2018 at 6:17 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Tue, Sep 25, 2018 at 3:00 AM Khushboo Vashi <
>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Hi Dave,
>>>>>>>>
>>>>>>>> On Tue, Sep 25, 2018 at 12:15 AM Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hi
>>>>>>>>> On Mon, Sep 24, 2018 at 2:05 AM Khushboo Vashi <
>>>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> Please find the attached updated patch.
>>>>>>>>>>
>>>>>>>>>> Feature Details:
>>>>>>>>>> - The current tree state as well as the previous will be stored
>>>>>>>>>> in the sqlite database.
>>>>>>>>>> - The time interval to store the tree state is configurable via
>>>>>>>>>> preferences and the default is 30 secs.
>>>>>>>>>> -1 can be used to stop the tree saving functionality,
>>>>>>>>>> - Jasmine test cases are included.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Khushboo
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> This is looking good, however there are a couple of cases where I
>>>>>>>>> think it's not quite working as I would expect:
>>>>>>>>>
>>>>>>>>> 1) As soon as the user opens pgAdmin, the tree state should be
>>>>>>>>> restored.
>>>>>>>>>
>>>>>>>>> As per our initial discussion, we have decided that once the user
>>>>>>>> connects / expands the server, then we will restore that server state.
>>>>>>>>
>>>>>>>> *"If the user has switched network that may cause a whole bunch of
>>>>>>>> connection failures after some period of time. Perhaps we should only
>>>>>>>> restore when opening a particular server." *
>>>>>>>>
>>>>>>>
>>>>>>> Yeah, I remember that now. I guess it just seems less than ideal -
>>>>>>> but I'm not sure how we could open everything by default without risking
>>>>>>> connection failures. Let's stick with the original plan (ie. what you've
>>>>>>> implemented).
>>>>>>>
>>>>>> Okay.
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> 2) If a database or server is disconnected, I would expect it's
>>>>>>>>> state to be stored. When it is explicitly re-connected by the user, the
>>>>>>>>> state should be restored.
>>>>>>>>>
>>>>>>>>> This is already implemented and working. Am I missing any
>>>>>>>> particular scenario?
>>>>>>>>
>>>>>>>
>>>>>>> I tried disconnecting a server and database, and when I re-opened
>>>>>>> it, the state wasn't restored. Do we explicitly save the state on
>>>>>>> disconnect, or was it that I needed to wait up to 30 seconds (or should
>>>>>>> have waited 30 seconds before disconnecting)?
>>>>>>>
>>>>>> I have explicitly saved the state on the server disconnection. So, on
>>>>>> the reconnection of the server, the state should be restored.
>>>>>> But this logic is only applicable on server not on the databases. So,
>>>>>> once you disconnect the database the state will not be restored.
>>>>>> Should I include databases also ?
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> I think that covers the different cases I'm imagining.
>>>>>>>>>
>>>>>>>>
>>>>>>> I wrote that to try to convince myself I hadn't forgotten anything -
>>>>>>> I think I had though. When the state is restored on any given branch of the
>>>>>>> tree, the previously selected node should be re-selected. That doesn't seem
>>>>>>> to be happening at the moment.
>>>>>>>
>>>>>>> This means even if I have opened multiple servers/databases, the
>>>>> selected item will remain only one.
>>>>> Is this correct?
>>>>>
>>>>
>>>> The selected item will be the previously selected item from whatever
>>>> was just restored.
>>>>
>>>>
>>> Previously when the last selected node was not considered, I stored the
>>> data in the database as below:
>>>
>>> {
>>>
>>> 1:
>>>
>>> [
>>>
>>>
>>> “server_group/1,server/1,coll-database/1,database/16393,coll-schema/16393,schema/2200,coll-table/2200,table/16400”,
>>>
>>>
>>> "server_group/1,server/1,coll-database/1,database/16393,coll-schema/16393,schema/2200,coll-table/2200,table/16394,coll-column/16394”,
>>>
>>>
>>> "server_group/1,server/1,coll-database/1,database/12669,coll-schema/12669,schema/2200,coll-table/2200,table/16397,coll-column/16397”
>>>
>>> ]
>>>
>>> }
>>>
>>>
>>>
>>> Here key is the server_id i.e. 1 and all the paths are stored as above
>>>
>>>
>>> Now, we are adding previously selected item as well as the database
>>> restore option, so I am thinking of changing the data storage structure as
>>> below.
>>>
>>>
>>>
>>> {
>>>
>>> 1:
>>>
>>> {
>>>
>>> 'paths':
>>>
>>> [
>>>
>>>
>>> “server_group/1,server/1,coll-database/1,database/16393,coll-schema/16393,schema/2200,coll-table/2200,table/16400”,
>>>
>>>
>>> "server_group/1,server/1,coll-database/1,database/16393,coll-schema/16393,schema/2200,coll-table/2200,table/16394,coll-column/16394”,
>>>
>>>
>>> "server_group/1,server/1,coll-database/1,database/12669,coll-schema/12669,schema/2200,coll-table/2200,table/16397,coll-column/16397”
>>>
>>> ],
>>>
>>> 'selected' :
>>>
>>> {
>>>
>>> "server/1": "coll-database/1",
>>>
>>> "database/16393": "table/16400",
>>>
>>> "database/12669": "coll-column/16397"
>>>
>>> },
>>>
>>> 'connection_status':
>>>
>>> {
>>>
>>> "database/16393": 1,
>>>
>>> "database/12669": 0
>>>
>>> }
>>>
>>>
>>> }
>>>
>>> }
>>>
>>> Please review it and let me know your suggestion.
>>>
>>
>> Looks reasonable to me.
>>
>>
>>>
>>> Thanks,
>>> Khushboo
>>>
>>>
>>>
>>>> Will do.
>>>>>>
>>>>>>> Thanks!
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>> Thanks, Dave.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>> Khushboo
>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Dave Page
>>>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>
>>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Dave Page
>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>> Twitter: @pgsnake
>>>>>>>
>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>
>>>>>>
>>>>
>>>> --
>>>> Dave Page
>>>> Blog: http://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>>
>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>>
>>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company