Re: Refactor: Registry Classes

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: Refactor: Registry Classes
Date: 2021-06-24 06:00:59
Message-ID: CANxoLDcQtnsEt-koh8dFLznyQMkaVhkF-rerW+NtzvdqPEoB=Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Thanks, the patch applied.

On Wed, Jun 23, 2021 at 7:54 PM Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
wrote:

> On Wed, Jun 23, 2021 at 1:22 PM Akshay Joshi <
> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>> Hi Ashesh
>>
>> Following are the review comments:
>>
>> - Fixed PEP8 issues.
>>
>> Done.
>
>>
>> - In "dynamic_registry/__init__.py" decorator @classmethod used for "
>> *_get*" and "*_load_modules*" methods which are actually outside of
>> the class. Even constructor also outside of the class.
>>
>> 'create_registry_metaclass' is not a class, but a method to create the
> dynamic classes.
> If I move these methods in 'create_registry_metaclass' method, SonarQube
> raises issues about complexity of the functions, hence - they're best kept
> outside of that method.
>
>>
>> - Remove unused imports from "driver/registry.py"
>>
>> Done
>
>>
>> -
>> - Fixed sonarqube issues in
>> "dynamic_registry/tests/registry/__init__.py"
>>
>> Done
> As discussed, SonarQube is not able to understand that the result object
> is a class, and not an object, hence - showing linter issues.
> I've disabled them in those lines by adding the comment '# NOSNAR' at the
> end.
>
> -- Thanks, Ashesh
>
>>
>> On Sat, Jun 19, 2021 at 11:27 AM Ashesh Vashi <
>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>> On Sat, Jun 19, 2021 at 1:39 AM Ashesh Vashi <
>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Akshay,
>>>>
>>>> We do have a couple of classes, which does automatic registration of
>>>> the base classes,
>>>> and which creates single-ton objects for these base classes, when
>>>> needed.
>>>>
>>>> I would be working on a patch sooner, which will be using similar
>>>> functionality for loading
>>>> the multi-factor authentication.
>>>>
>>>> I realized - it will be a duplicate code at three places for the same
>>>> functionalities.
>>>> Hence - I worked on refactoring this registry class.
>>>>
>>>> Please find the patch for the same.
>>>>
>>> Found issues - some test files were using the old function
>>> 'Driver.load_drivers(...)'.
>>> They're fixed now.
>>>
>>> -- Thanks, Ashesh
>>>
>>>>
>>>> --
>>>>
>>>> Thanks & Regards,
>>>>
>>>> Ashesh Vashi
>>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>>>> <http://www.enterprisedb.com>
>>>>
>>>>
>>>> *http://www.linkedin.com/in/asheshvashi*
>>>> <http://www.linkedin.com/in/asheshvashi>
>>>>
>>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>> *pgAdmin Hacker | Principal Software Architect*
>> *EDB Postgres <http://edbpostgres.com>*
>>
>> *Mobile: +91 976-788-8246*
>>
>

--
*Thanks & Regards*
*Akshay Joshi*
*pgAdmin Hacker | Principal Software Architect*
*EDB Postgres <http://edbpostgres.com>*

*Mobile: +91 976-788-8246*

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2021-06-24 06:01:14 Re: pgAdmin 4 - add missing gettexts and text space improvements
Previous Message Akshay Joshi 2021-06-24 06:00:22 pgAdmin 4 commit: Added missing gettexts and text space improvements.