[pgadmin-hackers] PATCH: Enhancement to backform controls [pgAdmin4]

[Murtuza Zabuawala]

Hi,

PFA patch to add functionalities as below,

1) Unique collection control: we have added 'Edit' button in grid, earlier
it was only delete button.
2) Sql field control: We have added control to disabled & visible
functionality which were not available.



--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


updated_sqlField_UniqueCollection_controls.patch
Description: Binary data

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

Re: [pgadmin-hackers] PATCH: Preferences/Options dialog

[Dave Page]

On Thu, Mar 3, 2016 at 3:40 PM, Ashesh Vashi 
wrote:

> On Thu, Mar 3, 2016 at 7:09 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Mon, Feb 29, 2016 at 6:22 PM, Ashesh Vashi <
>> ashesh.va...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> Please find the patch adding support for the preferences/options dialog.
>>> Initial patch was shared by Khushboo with me, but - I made some design
>>> changes.
>>>
>>> With this patch:
>>> * Each preferences can be saved/retrieved per module basis.
>>> * An object is created which represent that module in the preferences by
>>> default.
>>> * Module needs to override the register_preferences(...) method to
>>> register certain preference.
>>> * You can access other modules preference using static methods of the
>>> preferences.
>>> * A collection module (PGChildModule) will register
>>> show_node_, and also refers the 'show_system_objects' in it,
>>> which can be accessed using self.pref_show_system_objects, or can use class
>>> property 'show_system_obejcts', also there is one more property
>>> 'show_node', which uses the 'pref_show_node' object of that module.
>>>
>>
>> I've done an initial review:
>>
> Thanks.
>
>>
>> - The patch has bit-rotted, and needs to be rebased.
>>
> Done.
>
>>
>> - web/pgadmin/preferences/__init__.py has an out of date copyright notice
>> with no blank line after it, and no pydoc comment to introduce the file.
>>
> Done.
>
>>
>> I don't see anything else that seems horrendously wrong at the moment,
>> but will look more closely once the rebase has been done.
>>
> Please find the updated patch.
>
>
Thanks. Is it dependent on any other patches? I'm getting the attached
message when expanding the server group node (I have the schema/catalog
patch applied).

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

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

Re: [pgadmin-hackers] [pgAdmin IV] PATCH: Add support for the notices/messages from the backend server.

[Dave Page]

Thanks, committed.

On Thu, Mar 3, 2016 at 6:00 PM, Ashesh Vashi 
wrote:

> Hi,
>
> This patch implements the supports in the connection driver to fetch the
> notices/messages returned by the database server. This will be used by
> tools like debugger, sql-editor, etc.
>
> --
>
> Thanks & Regards,
>
> Ashesh Vashi
> EnterpriseDB INDIA: Enterprise PostgreSQL Company
> 
>
>
> *http://www.linkedin.com/in/asheshvashi*
> 
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>
>


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

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

[pgadmin-hackers] pgAdmin 4 commit: Allow the connection driver to return notices/message

[Dave Page]

Allow the connection driver to return notices/messages from the server.

Branch
--
master

Details
---
http://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=0b434431514dd898086ef69079d76ed3eb758ec8
Author: Ashesh Vashi 

Modified Files
--
web/pgadmin/utils/driver/abstract.py  | 14 ++
web/pgadmin/utils/driver/psycopg2/__init__.py | 25 +
2 files changed, 31 insertions(+), 8 deletions(-)


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

Re: [pgadmin-hackers] PATCH: Preferences/Options dialog

[Dave Page]

And here's the attachment.

On Fri, Mar 4, 2016 at 10:27 AM, Dave Page  wrote:
>
>
> On Thu, Mar 3, 2016 at 3:40 PM, Ashesh Vashi 
> wrote:
>>
>> On Thu, Mar 3, 2016 at 7:09 PM, Dave Page  wrote:
>>>
>>> Hi
>>>
>>> On Mon, Feb 29, 2016 at 6:22 PM, Ashesh Vashi
>>>  wrote:

 Hi Dave,

 Please find the patch adding support for the preferences/options dialog.
 Initial patch was shared by Khushboo with me, but - I made some design
 changes.

 With this patch:
 * Each preferences can be saved/retrieved per module basis.
 * An object is created which represent that module in the preferences by
 default.
 * Module needs to override the register_preferences(...) method to
 register certain preference.
 * You can access other modules preference using static methods of the
 preferences.
 * A collection module (PGChildModule) will register
 show_node_, and also refers the 'show_system_objects' in it,
 which can be accessed using self.pref_show_system_objects, or can use class
 property 'show_system_obejcts', also there is one more property 
 'show_node',
 which uses the 'pref_show_node' object of that module.
>>>
>>>
>>> I've done an initial review:
>>
>> Thanks.
>>>
>>>
>>> - The patch has bit-rotted, and needs to be rebased.
>>
>> Done.
>>>
>>>
>>> - web/pgadmin/preferences/__init__.py has an out of date copyright notice
>>> with no blank line after it, and no pydoc comment to introduce the file.
>>
>> Done.
>>>
>>>
>>> I don't see anything else that seems horrendously wrong at the moment,
>>> but will look more closely once the rebase has been done.
>>
>> Please find the updated patch.
>>
>
> Thanks. Is it dependent on any other patches? I'm getting the attached
> message when expanding the server group node (I have the schema/catalog
> patch applied).
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company



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

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

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

Re: [pgadmin-hackers] Numeric control optionally allow null values [pgadmin4]

[Dave Page]

Hi

On Fri, Mar 4, 2016 at 6:28 AM, Harshal Dhumal <
harshal.dhu...@enterprisedb.com> wrote:

> Hi,
>
> Please find attached patch for numeric and integer control with optionally
> null value support.
>
> Usage:
>
> 1] Integer
>
> id: 'fillfactor', label: '{{ _('Fill factor') }}', deps: ['index'],
> type: 'int', group: '{{ _('Definition') }}', allowNull: true,
>
>
> 2] Numeric
>
> id: 'fillfactor', label: '{{ _('Fill factor') }}', deps: ['index'],
> type: 'numeric', group: '{{ _('Definition') }}', allowNull: true,
>
>
You've used regexps to validate numbers in this patch, however they assume
that the only value number formats are (ignoring a leading -):

1234567 (integer)
1234567.89 (numeric)

However, this is only the standard in some countries - for example, a
French user might write a numeric as:

1 234 567,89

Even in India that might be written as:

12,34,567.89

In Germany:

1.234.567,89

The numeric inputs should be validated based on the client's locale.

Thanks.

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

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

Re: [pgadmin-hackers] PATCH: Enhancement to backform controls [pgAdmin4]

[Dave Page]

Thanks - committed.

On Fri, Mar 4, 2016 at 8:53 AM, Murtuza Zabuawala
 wrote:
> Hi,
>
> PFA patch to add functionalities as below,
>
> 1) Unique collection control: we have added 'Edit' button in grid, earlier
> it was only delete button.
> 2) Sql field control: We have added control to disabled & visible
> functionality which were not available.
>
>
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>



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

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


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

[pgadmin-hackers] pgAdmin 4 commit: Backform control enhancements:

[Dave Page]

Backform control enhancements:

1) Unique collection control: we have added 'Edit' button in grid, earlier it 
was only delete button.
2) Sql field control: We have added control to disabled & visible functionality 
which were not available.

Branch
--
master

Details
---
http://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=6ed5e7183eb7c97070bff0f12da8705156a7941f
Author: Murtuza Zabuawala 

Modified Files
--
web/pgadmin/static/js/backform.pgadmin.js | 39 ++-
1 file changed, 38 insertions(+), 1 deletion(-)


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

[pgadmin-hackers] pgAdmin 4 commit: Fix an error retrieving dependents and dependencies f

[Dave Page]

Fix an error retrieving dependents and dependencies for casts.

Branch
--
master

Details
---
http://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=a4e819c68cb5a6d52e97a5f20a60216af0a24eb2
Author: Sanket Mehta 

Modified Files
--
.../server_groups/servers/databases/casts/__init__.py| 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)


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

Re: [pgadmin-hackers] patch for cast module

[Dave Page]

Thanks, patch applied.

On Tue, Mar 1, 2016 at 7:20 AM, Sanket Mehta
 wrote:
> Hi,
>
> There was an error in cast module while fetching its dependency and
> dependent.
> Below is the patch which resolves this issue.
> Please review and commit it.
>
>
>
> Regards,
> Sanket Mehta
> Sr Software engineer
> Enterprisedb
>
> On Wed, Feb 24, 2016 at 10:17 PM, Dave Page  wrote:
>>
>> Thanks - committed.
>>
>> On Tue, Feb 23, 2016 at 1:34 PM, Sanket Mehta
>>  wrote:
>>>
>>> Hi,
>>>
>>> PFA the revised patch as per your comments.
>>> Please review it and let me know the feedback.
>>>
>>> Regards,
>>> Sanket Mehta
>>> Sr Software engineer
>>> Enterprisedb
>>>
>>> On Tue, Feb 23, 2016 at 4:10 PM, Dave Page  wrote:

 Hi

 I've attached an update to this patch, in which I've done some
 word-smithing on various comments, and adjusted the SQL templates to 
 improve
 the formatting.

 However, it looks like it's bit-rotted, as the dependents/dependencies
 display is throwing Python errors. Please fix and then I think it's just
 about ready to commit.

 Thanks.


 On Fri, Feb 19, 2016 at 11:03 AM, Sanket Mehta
  wrote:
>
> Hi Dave,
>
> PFA the revise patch.
>
> It includes changes according to your review comments as well as
> dependency/dependent part also.
>
> Let me know in case anything is missing.
>
> Regards,
> Sanket Mehta
> Sr Software engineer
> Enterprisedb
>
> On Mon, Feb 15, 2016 at 10:25 PM, Dave Page  wrote:
>>
>> And this time with the attachment...
>>
>> On Mon, Feb 15, 2016 at 4:53 PM, Dave Page  wrote:
>>>
>>> That's much better. Just a couple of comments now, partly based on an
>>> email I wrote earlier:
>>>
>>> - There is still inconsistency in comment style. Please see the
>>> attachment for an example. Note that there is *always* a space between 
>>> the
>>> comment marker and text.
>>>
>>> - If I try to edit a cast, I can change the description - but no SQL
>>> is shown on the SQL tab, despite the comment being correctly applied 
>>> when I
>>> hit save. The properties pane of the main window is also not updated.
>>>
>>> Otherwise, it looks fine.
>>>
>>> Thanks.
>>>
>>> On Mon, Feb 15, 2016 at 1:28 PM, Sanket Mehta
>>>  wrote:

 Hi,

 PFA the revised patch with all the required comments.



 Regards,
 Sanket Mehta
 Sr Software engineer
 Enterprisedb

 On Mon, Feb 15, 2016 at 4:18 PM, Dave Page 
 wrote:
>
>
>
> On Mon, Feb 15, 2016 at 8:10 AM, Sanket Mehta
>  wrote:
>>
>> Hi Dave,
>>
>> Regarding your suggestion of putting some comments in javascript,
>> I think I have already put some comments regarding model data and 
>> their
>> controls if any extended.
>>
>> Can you please let me know where exactly you think more comments
>> are required?
>
>
> Hi
>
> The issue for me is that jQuery code isn't the easiest to read at
> the best of times, with nested/anonymous functions and inline JSON 
> etc. As I
> look through the code for the various nodes in isolation, it's 
> extremely
> difficult to get a sense of what exactly each part of the code is 
> doing. In
> this example, what I see by reading the code is:
>
> - Define the required libraries (require.js stuff)
> - Extend the collection class
> - Extend the node class
>   - Define an init function inline
>   - Add the menu options
>
> That part is fairly easy to figure out (easier because there are
> blank lines between the logical sections). From there though, it 
> becomes
> much harder;
>
> - There are no blank lines to separate logical code sections at all
> between line 48 and 235 (there is one blank line, but it doesn't 
> separate
> code sections).
> - There are 4 comments that I can see. The first two are identical,
> and appear to have identical code blocks following them for reasons 
> that are
> not even remotely obvious.
> - As a newcomer to this code, I'm wondering if it's purpose is to
> define the backform model. If so, why is it not broken up into 
> sections with
> a comment to tell me what field each block handles, and any other 
> useful
> information I may need to know? If it's not, then what is it for?
>
> So... I'm not going to tell you exactly where to put comments,
> because the point is that without spending a couple of hours 
> understanding

Re: [pgadmin-hackers] PATCH: Enhancement to backform controls [pgAdmin4]

[Murtuza Zabuawala]

Hi,

PFA patch to disable edit button functionality conditionally in backgrid.


Regards,
Murtuza

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Mar 4, 2016 at 4:30 PM, Dave Page  wrote:

> Thanks - committed.
>
> On Fri, Mar 4, 2016 at 8:53 AM, Murtuza Zabuawala
>  wrote:
> > Hi,
> >
> > PFA patch to add functionalities as below,
> >
> > 1) Unique collection control: we have added 'Edit' button in grid,
> earlier
> > it was only delete button.
> > 2) Sql field control: We have added control to disabled & visible
> > functionality which were not available.
> >
> >
> >
> > --
> > Regards,
> > Murtuza Zabuawala
> > EnterpriseDB: http://www.enterprisedb.com
> > The Enterprise PostgreSQL Company
> >
> >
> > --
> > Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> > To make changes to your subscription:
> > http://www.postgresql.org/mailpref/pgadmin-hackers
> >
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


update_backform_control_v1.patch
Description: Binary data

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

[pgadmin-hackers] pgAdmin 4 commit: Enable/disable edit button functionality conditionall

[Dave Page]

Enable/disable edit button functionality conditionally in backgrid.

Branch
--
master

Details
---
http://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=fa97b5635e55b3bc65412bed51013c0715698e65
Author: Murtuza Zabuawala 

Modified Files
--
web/pgadmin/static/js/backform.pgadmin.js | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)


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

Re: [pgadmin-hackers] PATCH: Enhancement to backform controls [pgAdmin4]

[Dave Page]

Thanks, applied.

On Fri, Mar 4, 2016 at 11:40 AM, Murtuza Zabuawala
 wrote:
> Hi,
>
> PFA patch to disable edit button functionality conditionally in backgrid.
>
>
> Regards,
> Murtuza
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Fri, Mar 4, 2016 at 4:30 PM, Dave Page  wrote:
>>
>> Thanks - committed.
>>
>> On Fri, Mar 4, 2016 at 8:53 AM, Murtuza Zabuawala
>>  wrote:
>> > Hi,
>> >
>> > PFA patch to add functionalities as below,
>> >
>> > 1) Unique collection control: we have added 'Edit' button in grid,
>> > earlier
>> > it was only delete button.
>> > 2) Sql field control: We have added control to disabled & visible
>> > functionality which were not available.
>> >
>> >
>> >
>> > --
>> > Regards,
>> > Murtuza Zabuawala
>> > EnterpriseDB: http://www.enterprisedb.com
>> > The Enterprise PostgreSQL Company
>> >
>> >
>> > --
>> > Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>> > To make changes to your subscription:
>> > http://www.postgresql.org/mailpref/pgadmin-hackers
>> >
>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>
>



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

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


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

[pgadmin-hackers] pgAdmin 4 commit: Removed the wrong copyright information in the javasc

[Ashesh Vashi]

Removed the wrong copyright information in the javascript file, which is
owned by pgAdmin IV.

Branch
--
master

Details
---
http://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=ee71cab51e1996f4eb66d828a303dd68584c9ea9

Modified Files
--
web/pgadmin/static/js/backform.pgadmin.js | 8 
1 file changed, 8 deletions(-)


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

[pgadmin-hackers] pgAdmin 4 commit: Add some code review notes/hints to the docs.

[Dave Page]

Add some code review notes/hints to the docs.

Branch
--
master

Details
---
http://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=8f6c8fdd6efd6d564ed7781b7ac3892dc1e47f21

Modified Files
--
docs/en_US/code-snippet.rst   | 113 --
docs/en_US/index.rst  |   5 +-
docs/en_US/submitting-patches.rst |   4 +-
3 files changed, 91 insertions(+), 31 deletions(-)


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

[pgadmin-hackers] pgAdmin 4 commit: Add file missing from previous commit.

[Dave Page]

Add file missing from previous commit.

Branch
--
master

Details
---
http://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=5bf505e822242912e547cb9511796b1bef19067e

Modified Files
--
docs/en_US/code-review.rst | 55 ++
1 file changed, 55 insertions(+)


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

Re: [pgadmin-hackers] [pgAdmin4] [Patch]: Grant Wizard

[Dave Page]

Hi

On Thu, Mar 3, 2016 at 12:49 PM, Surinder Kumar
 wrote:
> Hi,
>
> PFA patch for Grant Wizard.
>
> Before applying grant wizard patch:
>
> 1. Apply patch for "wizard JS file" which Khushboo had shared with
> Ashesh personally.
> I am using that patch with few changes in that. Ashesh will
> review
> and commit that patch.
>
> 2. Apply patches of "Sequence" and "Functions" macros.
>
>
> Please review the patch and Let me know for any comments.

Initial feedback:

- Why does this add specific knowledge of the Grant Wizard to the
Browser module? It should be a plugin that loads itself at runtime.
Any changes to the browser to support that should be entirely generic
and usable by any module.

- The comment above also applies to the core templates. CSS should be
advertised by the plugin, and the browser can add it into the rendered
output when the module is dynamically loaded.

- +""" Implements Grant Wizard""" - why the leading space? Please
review all comments and code for such small details.

- The Python code to detect and alias various Python 2/3 classes
should be centralised, as I've seen it in at least one other module.

- In other module names, we've separated multiple words with a hypen.
e.g. server-groups. s/grantwizard/grant-wizard/g

- The published routes for this module are all under

- "GET /browser/static/js/wizard.js HTTP/1.1" 404 -

- For consistency, when naming CSS/JS files that are core to a module,
please use the module name, e.g.
/web/pgadmin/tools/grant-wizard/static/css/grant-wizard.css

Thanks.

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

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


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

Re: [pgadmin-hackers] Event trigges node patch [pgadmin4]

[Dave Page]

On Fri, Feb 12, 2016 at 6:32 AM, Harshal Dhumal <
harshal.dhu...@enterprisedb.com> wrote:

> Hi,
>
> PFA patch event triggers node.
>
>
Hi

It looks like this has bit-rotted. Please update and re-submit - currently
it doesn't list triggers in the tree if they're already present, and if you
try to create a new one it fails to find any trigger functions. I see this
in the console:

2016-03-04 14:24:01,759: INFO werkzeug: 127.0.0.1 - - [04/Mar/2016
14:24:01] "GET /browser/event_trigger/nodes/1/1/12403/ HTTP/1.1" 500 -
Traceback (most recent call last):
  File
"/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1836, in __call__
return self.wsgi_app(environ, start_response)
  File
"/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1820, in wsgi_app
response = self.make_response(self.handle_exception(e))
  File
"/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1403, in handle_exception
reraise(exc_type, exc_value, tb)
  File
"/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1817, in wsgi_app
response = self.full_dispatch_request()
  File
"/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1477, in full_dispatch_request
rv = self.handle_user_exception(e)
  File
"/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1381, in handle_user_exception
reraise(exc_type, exc_value, tb)
  File
"/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1475, in full_dispatch_request
rv = self.dispatch_request()
  File
"/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1461, in dispatch_request
return self.view_functions[rule.endpoint](**req.view_args)
  File
"/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/views.py",
line 84, in view
return self.dispatch_request(*args, **kwargs)
  File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/utils.py", line 248,
in dispatch_request
return method(*args, **kwargs)
  File
"/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/event_triggers/__init__.py",
line 118, in wrap
return f(*args, **kwargs)
  File
"/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/event_triggers/__init__.py",
line 142, in nodes
res.append(
AttributeError: 'dict' object has no attribute 'append'

NOTE: Please also check the code conforms to the checklist I added to the
docs earlier:
http://git.postgresql.org/gitweb/?p=pgadmin4.git;a=blob;f=docs/en_US/code-review.rst

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

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

Re: [pgadmin-hackers] [pgAdmin4] [Patch]: Grant Wizard

[Dave Page]

A couple of corrections below 

On Fri, Mar 4, 2016 at 1:39 PM, Dave Page  wrote:
> Hi
>
> On Thu, Mar 3, 2016 at 12:49 PM, Surinder Kumar
>  wrote:
>> Hi,
>>
>> PFA patch for Grant Wizard.
>>
>> Before applying grant wizard patch:
>>
>> 1. Apply patch for "wizard JS file" which Khushboo had shared with
>> Ashesh personally.
>> I am using that patch with few changes in that. Ashesh will
>> review
>> and commit that patch.
>>
>> 2. Apply patches of "Sequence" and "Functions" macros.
>>
>>
>> Please review the patch and Let me know for any comments.
>
> Initial feedback:
>
> - Why does this add specific knowledge of the Grant Wizard to the
> Browser module? It should be a plugin that loads itself at runtime.
> Any changes to the browser to support that should be entirely generic
> and usable by any module.
>
> - The comment above also applies to the core templates. CSS should be
> advertised by the plugin, and the browser can add it into the rendered
> output when the module is dynamically loaded.
>
> - +""" Implements Grant Wizard""" - why the leading space? Please
> review all comments and code for such small details.
>
> - The Python code to detect and alias various Python 2/3 classes
> should be centralised, as I've seen it in at least one other module.
>
> - In other module names, we've separated multiple words with a hypen.
> e.g. server-groups. s/grantwizard/grant-wizard/g

That should be an underscore, not a hyphen:

s/grantwizard/grant_wizard/g

>
> - The published routes for this module are all under
>
> - "GET /browser/static/js/wizard.js HTTP/1.1" 404 -
>
> - For consistency, when naming CSS/JS files that are core to a module,
> please use the module name, e.g.
> /web/pgadmin/tools/grant-wizard/static/css/grant-wizard.css

/web/pgadmin/tools/grant_wizard/static/css/grant_wizard.css

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

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


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

Re: [pgadmin-hackers] [pgAdmin4][Patch]: Foreign Data Wrapper

[Dave Page]

Hi

On Tue, Feb 23, 2016 at 5:24 PM, Neel Patel 
wrote:

> Hi,
>
> Please find attached patch file for the following three nodes.
>
>- Foreign Data Wrappers
>- Foreign Servers
>- User Mapping
>
> With this patch, we have implemented "Dependencies", "Dependent" tab and
> proper comments has been added.
>
> Do review it and let us know for any comments.
>

This seems to be nearly ready now. Some feedback below - note that a couple
of the issues may be caused by infrastructure code, in which case please do
fix them, but feel free to put them in a different patch:

- When adding a User Mapping, you cannot specify an empty option, e.g. a
blank password.

- When granting USAGE to PUBLIC on a foreign server, the WITH GRANT OPTION
is set, despite not being selected. e.g. adding "U" permissions for
"public" results in:

GRANT ALL ON FOREIGN SERVER redis_server TO public;
GRANT ALL ON FOREIGN SERVER redis_server TO public WITH GRANT OPTION;

- Why all the extra blank lines in this SQL? I would expect to see only 2,
between the ALTER/COMMENT/GRANT sections.


ALTER SERVER redis_server
VERSION 'Fooo';

COMMENT ON SERVER redis_server
IS 'Redis Server x';






GRANT ALL ON FOREIGN SERVER redis_server TO public;
GRANT ALL ON FOREIGN SERVER redis_server TO public WITH GRANT OPTION;




- Error messages from the server are not displayed properly - e.g:

invalid option "server"
HINT:  Valid options in this context are: 

Is displayed as:

invalid option "server" HINT: Valid options in this context are:

(Notice the lack of "")

- A foreign server object is not listed as being dependent upon the FDW
it's defined as using (but, pgAdmin 3 gets this wrong too).

- The "Options" tabs have a hint message of "Please enter some value!",
whether the focus is in the Option or Value field. Please fix to display:

Please enter an option name.

or

Please enter a value.

Based on the current focus. Note also that there should not be an
exclamation mark.

Thanks!

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

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