From: | Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com> |
---|---|
To: | Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com> |
Cc: | pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org> |
Subject: | Re: Refactor: Registry Classes |
Date: | 2021-06-23 14:24:32 |
Message-ID: | CAG7mmow6uPDAhfELmyXjBziHti0dE6aYCM7ufJZS3VXYZoLT-A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
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*
>
Attachment | Content-Type | Size |
---|---|---|
refactor_registry_class_v3.patch | application/octet-stream | 19.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Akshay Joshi | 2021-06-24 06:00:22 | pgAdmin 4 commit: Refactor the registry class logic and remove duplicat |
Previous Message | Libor M. | 2021-06-23 13:50:10 | pgAdmin 4 - add missing gettexts and text space improvements |