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

Hi

On Thu, Oct 4, 2018 at 1:12 PM Khushboo Vashi <khushboo.vashi@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.

 

Thanks,

Khushboo

On Mon, Oct 1, 2018 at 7:14 PM Dave Page <dpage@pgadmin.org> wrote:

Hi

On Mon, Oct 1, 2018 at 1:53 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

Hi Dave

On Mon, Oct 1, 2018 at 4:02 PM Dave Page <dpage@pgadmin.org> wrote:

On Mon, Oct 1, 2018 at 11:06 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

On Tue, Sep 25, 2018 at 6:26 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

On Tue, Sep 25, 2018 at 6:17 PM Dave Page <dpage@pgadmin.org> wrote:

On Tue, Sep 25, 2018 at 3:00 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

Hi Dave,

On Tue, Sep 25, 2018 at 12:15 AM Dave Page <dpage@pgadmin.org> wrote:

Hi

On Mon, Sep 24, 2018 at 2:05 AM Khushboo Vashi <khushboo.vashi@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