Re: pgAdmin4 PATCH: Domain Module

From: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: pgAdmin4 PATCH: Domain Module
Date: 2016-03-16 09:18:10
Message-ID: CAFOhELfzRV1WoA7A87=g5Nb9uvU3rLL2jPUCEid66pqzFdrfRQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi,

Please find the updated patch for the Domain Module.

To test this, the Data-type Reader patch needs to be applied first.

Also, please find in-line comments below.

Thanks,
Khushboo

On Thu, Feb 25, 2016 at 7:00 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Per discussion with Khushboo, the patch at
> http://www.postgresql.org/message-id/attachment/41939/schemas_macros_10_Feb_2.patch
> is a pre-req for this. Updated comments below...
>
> On Thu, Feb 25, 2016 at 12:13 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> Hi
>>
>> On Wed, Feb 24, 2016 at 9:24 AM, Khushboo Vashi <
>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi,
>>>
>>> I have updated the Domain module as below:
>>>
>>> - Used 'NodeByListControl' to get schemas, in domains.js file as
>>> suggested by Ashesh to avoid code redundancy.
>>>
>>> - Applied *'Security Label Macro'* Patch (Implemented by Harshal) and
>>> removed same changes from the Domain Patch.
>>> To test Domain patch, 'Security Label Macro' patch must be applied
>>> first as that is not committed yet.
>>>
>>> Please find attached Domain Module Patch.
>>>
>>
>> Initial feedback:
>>
>> - Owner and schema should be allowed to be left blank (and then default
>> to the current user/schema)
>>
> Done

> - Length and Precision fields should only be enabled if appropriate for
>> the data type.
>>
> Done

> The above still apply.
>
>
>>
>> - SQL generation for new Domains doesn't work:
>>
>
> This now works.
>
>
>>
>> - When adding constraints, I should be able to type directly into the
>> grid. Expanding the row should be optional.
>>
> I have made the grid non-editable explicitly as the Check constraint
control is multi-line control and right now there is no support in the grid
for the multi-line control.

> - The comment column on the constraints grid expands when the text reaches
>> ~50% of the width. It should be a fixed size (and use 100% of the space
>> available, less appropriate margins)
>>
> I have applied the size for the each header of the grid, but if the given
input will be without space in the grid then it will expand. For this, we
can make table layout fixed. So, please suggest, should I do that or not?

> - Backend support checks should not special-case Slony schemas.
>>
>> Done

> - 4 character indentation not used consistently in SQL templates.
>>
> Done

> These still apply.
>
>
>>
>> - Error seen when saving a domain: "macros/schemas/security.macros"
>>
>> 016-02-25 11:55:10,728: INFO werkzeug: 127.0.0.1 - - [25/Feb/2016
>> 11:55:10] "GET
>> /browser/domain/msql/1/1/24587/2200/?name=email&owner=postgres&basensp=public&description=This+is+an+email+data+type&basetype=text&typlen=&precision=&typdefault=&typnotnull=true&collname=&constraints=%5B%5D&seclabels=%5B%5D&_=1456401124386
>> 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/schemas/domains/__init__.py",
>> line 277, in wrap
>> return f(*args, **kwargs)
>> File
>> "/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/domains/__init__.py",
>> line 232, in wrap
>> return f(self, **kwargs)
>> File
>> "/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/domains/__init__.py",
>> line 700, in msql
>> status=200
>> File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/ajax.py", line 41, in
>> make_json_response
>> response=json.dumps(doc, cls=DataTypeJSONEncoder),
>> File
>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/simplejson/__init__.py",
>> line 386, in dumps
>> **kw).encode(obj)
>> File
>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/simplejson/encoder.py",
>> line 269, in encode
>> chunks = self.iterencode(o, _one_shot=True)
>> File
>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/simplejson/encoder.py",
>> line 348, in iterencode
>> return _iterencode(o, 0)
>> File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/ajax.py", line 26, in
>> default
>> return json.JSONEncoder.default(self, obj)
>> File
>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/simplejson/encoder.py",
>> line 246, in default
>> raise TypeError(repr(o) + " is not JSON serializable")
>> TypeError: TemplateNotFound() is not JSON serializable
>>
>
> This issue is resolved.
>
> Additional issues:
>
> - We can add a comment to constraints (and view them), however they are
> not saved.
>
> Done

> - The domain is not created as a single SQL statement, but by creating a
> domain over the base type, then adding constraints. Can this be done in one
> query?
>
Done

> - Reverse engineered SQL doesn't include the normal header and
> commented-out drop statement.
>
Done

> Thanks.
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Attachment Content-Type Size
domains_ver_4.patch text/x-patch 99.2 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2016-03-16 09:25:15 Re: pgAdmin4 PATCH: Domain Module
Previous Message Khushboo Vashi 2016-03-16 09:05:25 [pgAdmin4][Patch]: Data-type Reader