Thanks - committed!
On Fri, Oct 5, 2018 at 5:24 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:
> Hi Dave,
>
> I found one issue with the selecting previous selected node which I was
> fixed and attached the updated patch.
> Please let me know if you find any specific scenario which is not covered.
>
> While testing the patch, please clear your sqlite entry from the setting
> table as we have changed the structure.
>
> Thanks,
> Khushboo
>
> On Thu, Oct 4, 2018 at 8:07 PM Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>>
>>
>> On Thu, Oct 4, 2018 at 7:52 PM Dave Page <dp...@pgadmin.org> wrote:
>>
>>> Hi
>>>
>>> On Thu, Oct 4, 2018 at 1:12 PM Khushboo Vashi <
>>> khushboo.va...@enterprisedb.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.
>>>
>>>
>> I have checked the previous state and the current state and if both are
>> same then I skip the save call.
>> Is this the case or even if you change the tree view state it is not
>> saving?
>>
>>
>>
>>>
>>>> Thanks,
>>>> Khushboo
>>>>
>>>> On Mon, Oct 1, 2018 at 7:14 PM Dave Page <dp...@pgadmin.org> wrote:
>>>>
>>>>> Hi
>>>>>
>>>>> On Mon, Oct 1, 2018 at 1:53 PM Khushboo Vashi <
>>>>> khushboo.va...@enterprisedb.com> wrote:
>>>>>
>>>>>> Hi Dave
>>>>>>
>>>>>>
>>>>>> On Mon, Oct 1, 2018 at 4:02 PM Dave Page <dp...@pgadmin.org> wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Mon, Oct 1, 2018 at 11:06 AM Khushboo Vashi <
>>>>>>> khushboo.va...@enterprisedb.com> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Sep 25, 2018 at 6:26 PM Khushboo Vashi <
>>>>>>>> khushboo.va...@enterprisedb.com> wrote:
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Tue, Sep 25, 2018 at 6:17 PM Dave Page <dp...@pgadmin.org>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Tue, Sep 25, 2018 at 3:00 AM Khushboo Vashi <
>>>>>>>>>> khushboo.va...@enterprisedb.com> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi Dave,
>>>>>>>>>>>
>>>>>>>>>>> On Tue, Sep 25, 2018 at 12:15 AM Dave Page <dp...@pgadmin.org>
>>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi
>>>>>>>>>>>> On Mon, Sep 24, 2018 at 2:05 AM Khushboo Vashi <
>>>>>>>>>>>> khushboo.va...@enterprisedb.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
>>>
>>
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
