Re: [pgAdmin4] [Patch]: Extension Module

From: Neel Patel <neel(dot)patel(at)enterprisedb(dot)com>
To: Surinder Kumar <surinder(dot)kumar(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4] [Patch]: Extension Module
Date: 2016-02-04 06:29:11
Message-ID: CACCA4P27KHmsiWAJzOy3rL+9pLyg1jr=c27kbK97g+N3nAZZyQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi,

New patch is working fine.
Ashesh - Can you please review it? If it looks good then we can commit the
extension module.

Thanks,
Neel Patel

On Thu, Feb 4, 2016 at 11:40 AM, Surinder Kumar <
surinder(dot)kumar(at)enterprisedb(dot)com> wrote:

> Hi Neel,
>
> Please find the patch with following changes:
> 1. Removed Whitespace from files.
> 2. Fixed an issue in which json object converted into [object object]
> string.
> 3. Fixed python3 issue of unicode type where code breaks at
> "isinstance(SQL, str)" in python3 if str is unicode,
> because python no longer support unicode type of string.
>
> Please review the patch and Let me know for any comments.
>
> Thanks,
> Surinder Kumar
>
> On Thu, Feb 4, 2016 at 10:16 AM, Neel Patel <neel(dot)patel(at)enterprisedb(dot)com>
> wrote:
>
>> Hi Surinder,
>>
>> While applying the patch, we are getting below warnings.
>>
>> extension_v3.patch:362: trailing whitespace.
>> This function will generate sql for sql panel
>> extension_v3.patch:646: trailing whitespace.
>> -- Extension: {{ conn|qtIdent(data.name) }}
>> warning: 2 lines add whitespace errors.
>>
>>
>> Can you please resend the patch file, after fixing above warnings ?
>>
>> Thanks,
>> Neel Patel
>>
>> On Fri, Jan 22, 2016 at 12:25 PM, Surinder Kumar <
>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>
>>> Hi
>>>
>>> Please find the updated patch with following fixes:
>>> 1. Missing `owner` column under properties for `extensions collection`.
>>> Add cell: 'string' property for owner fixed it
>>> 2. Schema object identifier should be wrapped with in function qtIdent .
>>> Using function `qtIdent` for schema in create.sql fixed it.
>>>
>>>
>>> Thanks
>>> Surinder Kumar
>>>
>>> On Thu, Jan 21, 2016 at 8:04 PM, Surinder Kumar <
>>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi,
>>>>
>>>> I've fixed the issues given in review comments.
>>>>
>>>> Please find the attached updated patch for extension module, review it
>>>> and let me know for any comments.
>>>>
>>>> On Mon, Jan 18, 2016 at 5:44 PM, Surinder Kumar <
>>>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Thanks Neel for reviewing. I'll send the patch with the fixes
>>>>> suggested.
>>>>>
>>>>> On Mon, Jan 18, 2016 at 4:52 PM, Neel Patel <
>>>>> neel(dot)patel(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi Surinder,
>>>>>>
>>>>>> We have applied/tested the patch and below are the review comments.
>>>>>>
>>>>>> 1. When we select the extension "plpython3u", "plperl", "plperu" etc.
>>>>>> then it gives 'TypeError' in Javascript.
>>>>>> TypeError: d.version is undefined
>>>>>> 'version': (!_.isNull(d.version[0]) ? d.version[0]: '')
>>>>>>
>>>>>> We are getting this error while selecting many extensions so please
>>>>>> test with all types of extensions, it should not give any error at client
>>>>>> side.
>>>>>>
>>>>> Fixed.
>>>>
>>>>>
>>>>>> 2. Use 2 space indentation instead of 4 space in javascript file.
>>>>>>
>>>>> Done
>>>>
>>>>>
>>>>>> 3. In "validate" function in "extension.js" file, validate only the
>>>>>> changed values not all, and "this.get('name') - should be called only one
>>>>>> time not multiple
>>>>>> time".
>>>>>>
>>>>> Fixed.
>>>>
>>>>>
>>>>>> 4. When we pass object identifier, use the function 'qtIdent', and
>>>>>> for the values, use function 'qtLiteral' in all the sql files.
>>>>>>
>>>>> Done.
>>>>
>>>>>
>>>>>> 5. By default, when we create the extension, "schema_name" and
>>>>>> "version" should not be be set with value. It should be set blank by
>>>>>> default.
>>>>>>
>>>>> Kept blank while creating extension.
>>>>
>>>>>
>>>>>> 6. When we create any extension like "citext" then we are not able to
>>>>>> create the same extension again after deleting the same extension. May be
>>>>>> issue
>>>>>> with caching mechanism.
>>>>>>
>>>>> It is an architecture change. we'll fix it later.
>>>>
>>>>>
>>>>>> 7. When we remove the schema_name during the "Edit" operation then
>>>>>> wrong SQL is getting generated.
>>>>>>
>>>>> Fixed, Now it generates right SQL.
>>>>
>>>>>
>>>>>>
>>>>> 8. Remove "Use Slony" option. As discussed with Ashesh, we will
>>>>>> implement it as separate module.
>>>>>>
>>>>> Removed.
>>>>
>>>>>
>>>>>> Please fix the above issues. Let us know if you want more information.
>>>>>>
>>>>>> Thanks,
>>>>>> Neel Patel
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Neel Patel
>>>>>>
>>>>>>
>>>>>> On Tue, Jan 12, 2016 at 1:15 PM, Surinder Kumar <
>>>>>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Please find the updated patch with following changes:
>>>>>>>
>>>>>>> 1. corrected copyright.
>>>>>>> 2. Added proper comment for script_module function in
>>>>>>> __init__.py file.
>>>>>>> 3. Renamed collection Node's label to Extensions in
>>>>>>> extensions.js file.
>>>>>>>
>>>>>>>
>>>>>>> On Tue, Jan 12, 2016 at 12:44 PM, Surinder Kumar <
>>>>>>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Please find attached patch for the extension module.
>>>>>>>> Please review it and Let me know for any comments.
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Surinder Kumar
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Sent via pgadmin-hackers mailing list (
>>>>>>> pgadmin-hackers(at)postgresql(dot)org)
>>>>>>> To make changes to your subscription:
>>>>>>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Murtuza Zabuawala 2016-02-04 06:38:27 Re: PATCH: Tablespace Node [pgAdmin4]
Previous Message Harshal Dhumal 2016-02-04 06:17:11 Re: Updated patches