Subject: PATCH: To fix the issue in Debugger module (pgAdmin4)

From: Murtuza Zabuawala <[email-masked]>
Date: Mon, 26 Sep 2016 15:39:47 +0530

Hi,

PFA patch to fix the issue where it was not disabling buttons after
execution gets finished.
RM#1227

*Issue:*
If user clicks on buttons after execution is complete then it was throwing
error, expected behaviour was all button should gets disabled except
execute button.

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

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

---

From: Dave Page <[email-masked]>
Date: Mon, 26 Sep 2016 12:38:09 +0100

Hi

On Mon, Sep 26, 2016 at 11:09 AM, Murtuza Zabuawala
<[email-masked]> wrote:
> Issue:
> error, expected behaviour was all button should gets disabled except execute
> button.

This is an improvement I think, but not complete:

- The info messages are now shown, but:
  - Not until execution ends, which limits their usefulness for
additional debugging. Not sure if that's easily changeable though.
  - The line breaks in messages are displaying like this: "INFO:
Employee 1 not found <br>SELECT 1"

- The first execution of the function worked OK, but following the
second execution, I again saw: "Debugger: Step into execution error"
as I stepped out of the RETURN statement on line 18.

Thanks.

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

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

---

From: Murtuza Zabuawala <[email-masked]>
Date: Mon, 26 Sep 2016 17:44:03 +0530

On Mon, Sep 26, 2016 at 5:08 PM, Dave Page <[email-masked]> wrote:

> execute

> Employee 1 not found <br>SELECT 1"
It's showing properly on my side, Please clear browser cache & try again.

> as I stepped out of the RETURN statement on line 18.
This is transient issue, so need more debugging.

---

From: Murtuza Zabuawala <[email-masked]>
Date: Mon, 26 Sep 2016 17:58:16 +0530

Hi Dave,

PFA updated patch to fix `<br>` tag display.
Please clear cache & try again with this updated patch.

---

From: Dave Page <[email-masked]>
Date: Mon, 26 Sep 2016 13:54:13 +0100

> Please clear cache & try again with this updated patch.

OK, that fixes the display issue. Regarding the error message, on this
execution I noticed the following exception:

2016-09-26 13:52:48,181: INFO werkzeug: 127.0.0.1 - - [26/Sep/2016
13:52:48] "GET /debugger/poll_end_execution_result/3629301/ HTTP/1.1"
500 -
Traceback (most recent call last):
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 2000, in __call__
    return self.wsgi_app(environ, start_response)
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1991, 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 1567, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1988, in wsgi_app
    response = self.full_dispatch_request()
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1641, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1544, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1639, in full_dispatch_request
    rv = self.dispatch_request()
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1625, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask_login.py",
line 792, in decorated_view
    return func(*args, **kwargs)
  File "/Users/dpage/git/pgadmin4/web/pgadmin/tools/debugger/__init__.py",
line 1365, in poll_end_execution_result
    statusmsg = "<br>".join(additional_msgs) + "<br>" + statusmsg
TypeError: cannot concatenate 'str' and 'NoneType' objects

---

From: Murtuza Zabuawala <[email-masked]>
Date: Tue, 27 Sep 2016 12:10:40 +0530

Hi Dave,

PFA updated patch to fix mentioned issue as well as incremental msgs
updates in Messages Tab.

---

From: Dave Page <[email-masked]>
Date: Mon, 3 Oct 2016 13:35:54 +0100

Hi

On Tue, Sep 27, 2016 at 7:40 AM, Murtuza Zabuawala
<[email-masked]> wrote:
> Hi Dave,
>
> PFA updated patch to fix mentioned issue as well as incremental msgs updates
> in Messages Tab.

This doesn't seem to work well. In pgAdmin 4 I get the following:

====
SELECT 1
INFO: EMPNO ENAME

INFO: ----- -------

SELECT 1
SELECT 1
SELECT 1
INFO: 7369 SMITH

SELECT 1
SELECT 1
INFO: 7499 ALLEN

SELECT 1
SELECT 1
INFO: 7521 WARD

SELECT 1
SELECT 1
INFO: 7566 JONES

SELECT 1
SELECT 1
INFO: 7654 MARTIN

SELECT 1
INFO: 7698 BLAKE

SELECT 1
SELECT 1
SELECT 1
INFO: 7782 CLARK

SELECT 1
SELECT 1
INFO: 7788 SCOTT

SELECT 1
SELECT 1
INFO: 7839 KING

SELECT 1
SELECT 1
INFO: 7844 TURNER

SELECT 1
SELECT 1
INFO: 7876 ADAMS

SELECT 1
INFO: 7900 JAMES

SELECT 1
SELECT 1
INFO: 7902 FORD

SELECT 1
SELECT 1
SELECT 1
INFO: 7934 MILLER

SELECT 1
SELECT 1
SELECT 1
SELECT 1
====

Whilst in pgAdmin III I get:

====
INFO:  EMPNO    ENAME
INFO:  -----    -------
INFO:  7369     SMITH
INFO:  7499     ALLEN
INFO:  7521     WARD
INFO:  7566     JONES
INFO:  7654     MARTIN
INFO:  7698     BLAKE
INFO:  7782     CLARK
INFO:  7788     SCOTT
INFO:  7839     KING
INFO:  7844     TURNER
INFO:  7876     ADAMS
INFO:  7900     JAMES
INFO:  7902     FORD
INFO:  7934     MILLER
SELECT 1
====

Sidenote: pgAdmin III uses a fixed-width font for this output which
works far better than pgAdmin 4's variable width font.

---

From: Murtuza Zabuawala <[email-masked]>
Date: Wed, 5 Oct 2016 17:39:08 +0530

Hi Dave,

PFA updated patch for the same.
RM#1227

Please review.

On Mon, Oct 3, 2016 at 6:05 PM, Dave Page <[email-masked]> wrote:

> updates

---

From: Dave Page <[email-masked]>
Date: Wed, 5 Oct 2016 14:59:25 +0100

OK, that's much better. I think I've figured out what's causing the
remaining execution error as well - we're not disabling the buttons
until we get a response from the server, so if you click too quickly,
you make multiple requests at once. I tested this by adding a "SELECT
pg_sleep(2);" to a loop in a function.

If you can fix that as well, then I think we can call this issue fixed.

Thanks!

---

From: Murtuza Zabuawala <[email-masked]>
Date: Thu, 6 Oct 2016 14:35:16 +0530

Hi Dave,

PFA updated patch with suggestion given.

---

From: Dave Page <[email-masked]>
Date: Fri, 7 Oct 2016 11:46:14 +0100

Hi

It still seems buggy. I started debugging list_emp() from our sample
DB, and just hit Step Into a few times randomly. It got as far as the
pg_sleep call I added, then never returned so I couldn't proceed. See
attached screenshot.

---

From: Murtuza Zabuawala <[email-masked]>
Date: Fri, 7 Oct 2016 17:12:55 +0530

Hi Dave,

I faced the same issue when I initially tried that, but then as per Neel
suggestion I changed SELECT pg_sleep() to PERFORM pg_sleep() in function.
You will face the same in pgAdmin3 if you use select pg_sleep() in your
function the debug call never returns from DB server.

---

From: Dave Page <[email-masked]>
Date: Fri, 7 Oct 2016 12:53:22 +0100

> function the debug call never returns from DB server.

In which case, doesn't that imply the debugger is missing critical
debug info? If I run the query in the query tool, I get:

====
INFO:  EMPNO    ENAME
INFO:  -----    -------
ERROR:  query has no destination for result data
HINT:  If you want to discard the results of a SELECT, use PERFORM instead.
CONTEXT:  PL/pgSQL function list_emp() line 11 at SQL statement

Query returned successfully in 2 secs.
====

It seems to me that the debugger should be able to give the same error.

Regardless of that, I'll test with PERFORM.

Thanks.

---

From: Dave Page <[email-masked]>
Date: Fri, 7 Oct 2016 13:02:54 +0100

> Regardless of that, I'll test with PERFORM.

Which I just did - and whilst it seemed to be fine when stepping
through, after a few iterations I hit the continue button, at which
point it froze again on "PERFORM pg_sleep(2)", didn't print any more
of the 14 names in the emp table, and didn't return :-(

---

From: Murtuza Zabuawala <[email-masked]>
Date: Thu, 20 Oct 2016 13:44:27 +0530

Hi Dave,

*Issue:*
We were not properly fetching result from server in case of direct
debugging when we restart debugging of same object.

Thanks to Neel for helping in this issue.

Please review.

---

From: Dave Page <[email-masked]>
Date: Fri, 21 Oct 2016 12:18:50 +0100

Hi

There are still issues I'm afraid:

- When execution stops, we seem to keep polling for more results indefinitely.

- When executing for a second time, the messages tab isn't cleared,
and new messages don't seem to be appended to it either. I would
expect the tab to be cleared.

On Thu, Oct 20, 2016 at 9:14 AM, Murtuza Zabuawala
<[email-masked]> wrote:
> We were not properly fetching result from server in case of direct debugging
> when we restart debugging of same object.
>

---

From: Neel Patel <[email-masked]>
Date: Fri, 21 Oct 2016 17:02:37 +0530

Hi,

> - When execution stops, we seem to keep polling for more results
Do you mean after completion of first successful debugging ?
If yes, we are polling because user can start same function for debugging
again and we have to listen for the result set for that session.

> expect the tab to be cleared.

Ok. We will fix this issue.

> debugging

---

From: Dave Page <[email-masked]>
Date: Fri, 21 Oct 2016 12:33:40 +0100

Hi

> again and we have to listen for the result set for that session.

Yes (or the second). But shouldn't we stop polling until debugging is restarted?

---

From: Neel Patel <[email-masked]>
Date: Fri, 21 Oct 2016 17:57:47 +0530

Hi,

> Yes (or the second). But shouldn't we stop polling until debugging is

I think yes, that can be done.

> >> >> >> I faced the same issue when I initially tried that, but then as
> per
> >> >> >> Neel

---

From: Murtuza Zabuawala <[email-masked]>
Date: Fri, 11 Nov 2016 17:32:19 +0530

Hi Dave,

PFA updated patch, Both of the issues pointed by you in last email are
addressed in this patch.
Please review.

RM#1227

> Yes (or the second). But shouldn't we stop polling until debugging is
Fixed

> Fixed

>> >> >> >> You will face the same in pgAdmin3 if you use select pg_sleep()
>> in
>> >> >> >> your

---

From: Dave Page <[email-masked]>
Date: Fri, 11 Nov 2016 14:25:58 +0000

Thanks - committed with minor changes to hide the busy cursor when
execution completes and to fix some comments.

On Fri, Nov 11, 2016 at 12:02 PM, Murtuza Zabuawala
<[email-masked]> wrote:
>>> >> >> > HINT:  If you want to discard the results of a SELECT, use
>>> >> >> > PERFORM
>>> >> >> > instead.