Re: [pgAdmin4][Debugger]: Initial Patch

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Neel Patel <neel(dot)patel(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Debugger]: Initial Patch
Date: 2016-04-20 13:02:15
Message-ID: CA+OCxoxQTncHeTw-Wd=Vmoa1BcouenV00MypEOwz0sGBRJobRA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Thanks - applied!

On Tuesday, April 19, 2016, Neel Patel <neel(dot)patel(at)enterprisedb(dot)com> wrote:

> Hi Dave,
>
> Please find the attached patch file with below fix.
>
> - Remove the duplicate CSS and used the actual class for the debugger
> button toolbar.
> - As per the Ashesh's comment, previously we used 2 wcDocker instance
> to arrange the multiple tabs to main debugger panel. Now with this patch
> file, we have used only 1 wcDocker instance.
>
> Do review it and let us know for comments.
>
> Thanks,
> Neel Patel
>
> On Mon, Apr 18, 2016 at 6:07 PM, Dave Page <dpage(at)pgadmin(dot)org
> <javascript:_e(%7B%7D,'cvml','dpage(at)pgadmin(dot)org');>> wrote:
>
>> Hi
>>
>> On Monday, April 18, 2016, Neel Patel <neel(dot)patel(at)enterprisedb(dot)com
>> <javascript:_e(%7B%7D,'cvml','neel(dot)patel(at)enterprisedb(dot)com');>> wrote:
>>
>>> Hi Dave,
>>>
>>> Please find inline comments with all the fixes.
>>> Attached is the updated patch file. Do review it and let me know for any
>>> comments.
>>>
>>> If you find any issues, let me know .Now, Working on the remaining TODOs
>>> as mentioned in below email.
>>>
>>
>> Thanks - committed with some minor tweaks. One problem partly still
>> remains though - you've partially copied the toolbar styling. Please use
>> the actual classes used by the Properties panel. I've already updated the
>> query tool in that way. Whilst your version looks much closer, it's missing
>> the minimum button widths, and duplicates CSS unnecessarily.
>>
>> Thanks.
>>
>>
>>>
>>> On Fri, Apr 15, 2016 at 2:09 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>> Hi
>>>>
>>>> On Thu, Apr 14, 2016 at 1:52 PM, Neel Patel <
>>>> neel(dot)patel(at)enterprisedb(dot)com> wrote:
>>>> > Hi,
>>>> >
>>>> > Please find attached v2 patch file of the debugger which fixes the
>>>> below
>>>> > issues which was not present in the first patch.
>>>> > In this patch, we have added new table in sqlite database to store the
>>>> > functions arguments value user has given during debugging.
>>>> > After applying this patch, user needs to execute "setup.py" to create
>>>> the
>>>> > new table in pgadmin4.db file.
>>>> >
>>>> > In direct debugging, when user debug the function then arguments
>>>> values will
>>>> > be stored in the sqlite database so when user debug the same function
>>>> again
>>>> > then previous values will be filled in the user input dialog.
>>>> > Once the execution is completed then user will be able to do the
>>>> debug of
>>>> > the same function again by pressing the "Continue/Restart" button.
>>>> > User can debug the "procedure" which is supported in PPAS database.
>>>> > Replaced the "Glyphicon" with the "font-awesome" icons.
>>>>
>>>> Very cool! Committed, understanding that there are still improvements
>>>> to be made.
>>>>
>>>> > Below are the TODOs
>>>> >
>>>> > Validate the input arguments values changed by user while depositing
>>>> the
>>>> > value during debugging.
>>>> > Need to implement the code folding in the codemirror editor area.
>>>> > As per the Ashesh's suggestion, need to add debug logs information so
>>>> that
>>>> > we can get the state of the debug function. Also need to add "arrow"
>>>> next to
>>>> > breakpoint in the gutters as per the pgadmin3.
>>>> > Need to add "Debug package initializer" in the user input dialog for
>>>> the
>>>> > direct debugging.
>>>> > Last but not least "Review comments" :)
>>>>
>>>> Here you go :-)
>>>>
>>>> - Ensure all messages are gettext enabled.
>>>>
>>>
>>> Fixed.
>>>
>>>>
>>>> - Constructs like the following won't work, because Jinja will
>>>> evaluate the string " + err.errormsg + " before it ever gets evaluated
>>>> as JS by the browser.
>>>>
>>>> Alertify.alert("{{ _('" + err.errormsg + "') }}");
>>>>
>>>
>>> Fixed.
>>>
>>>
>>>>
>>>> - Please adjust the button bar to use the same styling as the button
>>>> bar on the Properties tab.
>>>>
>>>
>>> Fixed
>>>
>>>>
>>>> - Let's make the stack pane tab part of the tabset at the bottom of
>>>> the debugger, and ensure docking etc. works so tabs can be split off
>>>> and arranged around the main source window.
>>>>
>>>
>>> Fixed. Now stack pane will be displayed along with another panel at
>>> bottom and also docking has been introduced for all the panels so tabs will
>>> be arranged around main debugger panel.
>>>
>>>
>>>>
>>>> - Stepping is quite slow. What's causing that? I wonder if we really
>>>> need to have all the one line SQL templates - they're probably quite
>>>> expensive to process.
>>>>
>>> Fixed. This is due to polling timeout was high (1 second) and we are
>>> getting delay in getting the results. Now polling timeout has reduced to to
>>> 200ms.
>>>
>>>>
>>>> - Is backend_running.sql required? I've removed both versions as I
>>>> can't find any references to them. Are any other templates not
>>>> required?
>>>>
>>> Ok. No other templates.
>>>
>>>>
>>>> Will log any other issues that come up in further work.
>>>>
>>>> > Below functionalities are implemented but testing are pending.
>>>> >
>>>> > Trigger functions need to test with the debugger.
>>>> > Functions are tested with data types (like text, integer etc.) but
>>>> it needs
>>>> > to be tested with all the data types for direct debugging.
>>>> > Functions/Procedures need to test with PPAS 9.2 and earlier version
>>>> where
>>>> > debugger version is different.
>>>>
>>>> Thanks!
>>>>
>>>> --
>>>> 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
>>
>>
>

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

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

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Meet Bhagdev 2016-04-20 14:49:37 pgAdmin4 download
Previous Message Dave Page 2016-04-20 13:02:02 pgAdmin 4 commit: Cleanup debugger dashboard