Thread: [pgAdmin4] [Patch]: Language Module

Hi Akshay,

Please find the review comments below for Language node.

- On postgres9.1 language node failed with below error when clicked on properties or sql tab.
   function expression in FROM cannot refer to other relations of same query level LINE 15: aclexplode(lanacl) d ^

- Error when tried to update language name, owner, comment.

File "/home/edb/PRJ/pgadmin4/web/pgadmin/browser/utils.py", line 277, in dispatch_request
    return method(*args, **kwargs)
  File "/home/edb/PRJ/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/languages/__init__.py", line 139, in wrap
    return f(*args, **kwargs)
  File "/home/edb/PRJ/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/languages/__init__.py", line 224, in update
    sql = self.get_sql(gid, sid, data, did, lid)
TypeError: get_sql() takes from 2 to 3 positional arguments but 6 were given

 

- In pgadmin3 it shows drop down for language name while in pgadmin4 in edit mode it shows input text.

Need some clarification whether user is allowed to enter any language name or can only select from predefined language names.

Regards,
~Harshal

-- 

Harshal Dhumal

Software Engineer 

EenterpriseDB

On Mon, Feb 8, 2016 at 6:17 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Hi All

Attached is the patch for the Language Module. Please review it and if it looks good then please commit it. 

--

Akshay Joshi

Principal Software Engineer 

Phone: +91 20-3058-9517
Mobile: +91 976-788-8246

--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Hi,

New language patch is working fine.

Ashesh - Can you please review it ?

If it looks good then can you please commit ?

Thank you,

Harshal

Hi All

Attached is the new patch after fixing review comments given by Dave.

On Fri, Feb 12, 2016 at 10:04 PM, Dave Page <dpage@pgadmin.org> wrote:

On Fri, Feb 12, 2016 at 11:21 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:

Hi,

New language patch is working fine.

Ashesh - Can you please review it ?

If it looks good then can you please commit ?

Thank you,

Harshal

A few comments:

- We omitted CREATE LANGUAGE, as most are now extensions. Shouldn't we also therefore omit DROP LANGUAGE?

    Fixed. 

- Please leave a blank line between the file header and python code in python files.

    Fixed. 

- The pydoc module description is missing from __init__.py

    Fixed. 

- What about the Dependents and Dependencies tabs?

     Infrastructure is not ready, so not implemented yet. 

 

- There are few, if any comments in the Javascript. There should be at least minimal comments there to note the purpose of each logical code block.

    Fixed. 

--

Akshay Joshi

Principal Software Engineer 

Phone: +91 20-3058-9517
Mobile: +91 976-788-8246

On Mon, Feb 15, 2016 at 7:29 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Hi All

Attached is the new patch after fixing review comments given by Dave.

On Fri, Feb 12, 2016 at 10:04 PM, Dave Page <dpage@pgadmin.org> wrote:

On Fri, Feb 12, 2016 at 11:21 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:

Hi,

New language patch is working fine.

Ashesh - Can you please review it ?

If it looks good then can you please commit ?

Thank you,

Harshal

A few comments:

- We omitted CREATE LANGUAGE, as most are now extensions. Shouldn't we also therefore omit DROP LANGUAGE?

    Fixed. 

- Please leave a blank line between the file header and python code in python files.

    Fixed. 

- The pydoc module description is missing from __init__.py

    Fixed. 

- What about the Dependents and Dependencies tabs?

     Infrastructure is not ready, so not implemented yet. 

The first person that needs it should be working on it! I don't want to be committing partially completed features, otherwise we'll have to come back and revisit them all when the infrastructure is done.

Having done it before for pgAdmin 3, I don't imagine it's hard. It's really just the same as the Statistics view. Please take a look at it (assuming noone else is already).

Thanks.

 

 

- There are few, if any comments in the Javascript. There should be at least minimal comments there to note the purpose of each logical code block.

    Fixed. 

--

Akshay Joshi

Principal Software Engineer 

Phone: +91 20-3058-9517
Mobile: +91 976-788-8246

--

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi All

On Mon, Feb 15, 2016 at 3:47 PM, Dave Page <dpage@pgadmin.org> wrote:

On Mon, Feb 15, 2016 at 7:29 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Hi All

Attached is the new patch after fixing review comments given by Dave.

On Fri, Feb 12, 2016 at 10:04 PM, Dave Page <dpage@pgadmin.org> wrote:

On Fri, Feb 12, 2016 at 11:21 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:

Hi,

New language patch is working fine.

Ashesh - Can you please review it ?

If it looks good then can you please commit ?

Thank you,

Harshal

A few comments:

- We omitted CREATE LANGUAGE, as most are now extensions. Shouldn't we also therefore omit DROP LANGUAGE?

    Fixed. 

- Please leave a blank line between the file header and python code in python files.

    Fixed. 

- The pydoc module description is missing from __init__.py

    Fixed. 

- What about the Dependents and Dependencies tabs?

     Infrastructure is not ready, so not implemented yet. 

The first person that needs it should be working on it! I don't want to be committing partially completed features, otherwise we'll have to come back and revisit them all when the infrastructure is done.

Having done it before for pgAdmin 3, I don't imagine it's hard. It's really just the same as the Statistics view. Please take a look at it (assuming noone else is already).

   Fixed. Attached is the modified patch with support of Dependents and Dependencies tabs. Please review it.

On Fri, Feb 19, 2016 at 10:48 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Hi All

On Mon, Feb 15, 2016 at 3:47 PM, Dave Page <dpage@pgadmin.org> wrote:

On Mon, Feb 15, 2016 at 7:29 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Hi All

Attached is the new patch after fixing review comments given by Dave.

On Fri, Feb 12, 2016 at 10:04 PM, Dave Page <dpage@pgadmin.org> wrote:

On Fri, Feb 12, 2016 at 11:21 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:

Hi,

New language patch is working fine.

Ashesh - Can you please review it ?

If it looks good then can you please commit ?

Thank you,

Harshal

A few comments:

- We omitted CREATE LANGUAGE, as most are now extensions. Shouldn't we also therefore omit DROP LANGUAGE?

    Fixed. 

- Please leave a blank line between the file header and python code in python files.

    Fixed. 

- The pydoc module description is missing from __init__.py

    Fixed. 

- What about the Dependents and Dependencies tabs?

     Infrastructure is not ready, so not implemented yet. 

The first person that needs it should be working on it! I don't want to be committing partially completed features, otherwise we'll have to come back and revisit them all when the infrastructure is done.

Having done it before for pgAdmin 3, I don't imagine it's hard. It's really just the same as the Statistics view. Please take a look at it (assuming noone else is already).

   Fixed. Attached is the modified patch with support of Dependents and Dependencies tabs. Please review it.

Committed, with a couple of additional minor changes:

- s/resourcegroups/resource_groups (for consistency with server_groups)

- Added a PyDoc module description to __init__.py