Thread: Refactor: Registry Classes

Refactor: Registry Classes

From
Ashesh Vashi
Date:
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. 

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company


http://www.linkedin.com/in/asheshvashi

Attachment

Re: Refactor: Registry Classes

From
Ashesh Vashi
Date:
On Sat, Jun 19, 2021 at 1:39 AM Ashesh Vashi <ashesh.vashi@enterprisedb.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.linkedin.com/in/asheshvashi

Attachment

Re: Refactor: Registry Classes

From
Akshay Joshi
Date:
Hi Ashesh

Following are the review comments:
  • Fixed PEP8 issues.
  • 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.
  • Remove unused imports from "driver/registry.py"
  • Fixed sonarqube issues in "dynamic_registry/tests/registry/__init__.py"

On Sat, Jun 19, 2021 at 11:27 AM Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Sat, Jun 19, 2021 at 1:39 AM Ashesh Vashi <ashesh.vashi@enterprisedb.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.linkedin.com/in/asheshvashi



--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246

Re: Refactor: Registry Classes

From
Ashesh Vashi
Date:
On Wed, Jun 23, 2021 at 1:22 PM Akshay Joshi <akshay.joshi@enterprisedb.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.vashi@enterprisedb.com> wrote:
On Sat, Jun 19, 2021 at 1:39 AM Ashesh Vashi <ashesh.vashi@enterprisedb.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.linkedin.com/in/asheshvashi



--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246

Attachment

Re: Refactor: Registry Classes

From
Akshay Joshi
Date:
Thanks, the patch applied.

On Wed, Jun 23, 2021 at 7:54 PM Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Wed, Jun 23, 2021 at 1:22 PM Akshay Joshi <akshay.joshi@enterprisedb.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.vashi@enterprisedb.com> wrote:
On Sat, Jun 19, 2021 at 1:39 AM Ashesh Vashi <ashesh.vashi@enterprisedb.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.linkedin.com/in/asheshvashi



--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246

Re: Refactor: Registry Classes

From
Khushboo Vashi
Date:
Hi,

This patch introduced the server mode api test case failure, please find the attached patch to fix those as well as some of the old issues in the server mode. 
Patch by: Ashesh Vashi

Thanks,
Khushboo


On Thu, Jun 24, 2021 at 11:31 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Thanks, the patch applied.

On Wed, Jun 23, 2021 at 7:54 PM Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Wed, Jun 23, 2021 at 1:22 PM Akshay Joshi <akshay.joshi@enterprisedb.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.vashi@enterprisedb.com> wrote:
On Sat, Jun 19, 2021 at 1:39 AM Ashesh Vashi <ashesh.vashi@enterprisedb.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.linkedin.com/in/asheshvashi



--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246

Attachment

Re: Refactor: Registry Classes

From
Akshay Joshi
Date:
Thanks, the patch applied.

On Thu, Jun 24, 2021 at 3:51 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
Hi,

This patch introduced the server mode api test case failure, please find the attached patch to fix those as well as some of the old issues in the server mode. 
Patch by: Ashesh Vashi

Thanks,
Khushboo


On Thu, Jun 24, 2021 at 11:31 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Thanks, the patch applied.

On Wed, Jun 23, 2021 at 7:54 PM Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Wed, Jun 23, 2021 at 1:22 PM Akshay Joshi <akshay.joshi@enterprisedb.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.vashi@enterprisedb.com> wrote:
On Sat, Jun 19, 2021 at 1:39 AM Ashesh Vashi <ashesh.vashi@enterprisedb.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.linkedin.com/in/asheshvashi



--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246