Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted.

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Nikhil Mohite <nikhil(dot)mohite(at)enterprisedb(dot)com>
Cc: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted.
Date: 2021-01-21 10:36:54
Message-ID: CA+OCxozsMp+p62THjyi=UX9j-4ZmsDz75OLYQPsPFWzQ6cEXQA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi

On Thu, Jan 21, 2021 at 10:33 AM Nikhil Mohite <
nikhil(dot)mohite(at)enterprisedb(dot)com> wrote:

> Hi Dave,
>
> On Thu, Jan 21, 2021 at 3:24 PM Akshay Joshi <
> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>> Reverted the commit.
>>
>> On Thu, Jan 21, 2021 at 3:13 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>> This seems like a very bad idea. What if the user that has left was the
>>> user that setup 50 connections used by everyone else?
>>>
>>> Deleting those shared entries is (I would guess) most likely *not* what
>>> the majority of users would want, and the current behaviour is definitely
>>> safest.
>>>
>> In the current implementation when the admin user gets deleted all "*Server
> groups*" created by that user are getting deleted, so if that admin has
> created any *Shared server* other users are not able to access it as its
> server group is not present in the database.
>

That seems bad. I would suggest we only delete the group if there are no
shared servers left in it that would become orphaned. Of course, in that
case we'll also need to reassign ownership of the group.

>
>>> We should make this optional; i.e. ask the use if they want shared
>>> servers created by the user to be deleted. If they say no, they should be
>>> reassigned to another user; either the admin that's deleting the user, or
>>> their choice of user (a little more complex of course, but more flexible).
>>>
>> In the shared server table, we are creating entries per user, for
> deletion of non-admin user we can delete the shared server tables entries
> as it will not affect any other users. (because only admin users can mark
> the server as shared.)
> In case of admin user deletion, will add an extra check as suggested.
>

Sounds good - thanks!

>
>>>
>>
>>> Please revert this, until the deletion is made optional.
>>>
>>> On Thu, Jan 21, 2021 at 9:23 AM Akshay Joshi <
>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Thanks, patch applied.
>>>>
>>>> On Thu, Jan 21, 2021 at 12:18 PM Nikhil Mohite <
>>>> nikhil(dot)mohite(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi Team,
>>>>>
>>>>> Please find the attached patch for RM-6143
>>>>> <https://redmine.postgresql.org/issues/6143>: Shared server entries
>>>>> not getting deleted.
>>>>> Added code to delete shared server entries if the admin deletes the
>>>>> user from user management.
>>>>>
>>>>>
>>>>> --
>>>>> *Thanks & Regards,*
>>>>> *Nikhil Mohite*
>>>>> *Software Engineer.*
>>>>> *EDB Postgres* <https://www.enterprisedb.com/>
>>>>> *Mob.No: +91-7798364578.*
>>>>>
>>>>
>>>>
>>>> --
>>>> *Thanks & Regards*
>>>> *Akshay Joshi*
>>>> *pgAdmin Hacker | Principal Software Architect*
>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>
>>>> *Mobile: +91 976-788-8246*
>>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EDB: http://www.enterprisedb.com
>>>
>>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>> *pgAdmin Hacker | Principal Software Architect*
>> *EDB Postgres <http://edbpostgres.com>*
>>
>> *Mobile: +91 976-788-8246*
>>
>
> Regards,
> Nikhil Mohite.
>

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

EDB: http://www.enterprisedb.com

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2021-01-21 10:59:14 pgAdmin 4 commit: Updated Windows installer script for Kerberos support
Previous Message Nikhil Mohite 2021-01-21 10:33:11 Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted.