Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1evbpp-00006R-2E for pgadmin-hackers@arkaria.postgresql.org; Tue, 13 Mar 2018 04:46:49 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1evbpm-000658-D2 for pgadmin-hackers@arkaria.postgresql.org; Tue, 13 Mar 2018 04:46:46 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1evbpm-00064y-63 for pgadmin-hackers@lists.postgresql.org; Tue, 13 Mar 2018 04:46:46 +0000 Received: from mail-oi0-x229.google.com ([2607:f8b0:4003:c06::229]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1evbpg-0001hU-E1 for pgadmin-hackers@postgresql.org; Tue, 13 Mar 2018 04:46:45 +0000 Received: by mail-oi0-x229.google.com with SMTP id a207so14255125oii.10 for ; Mon, 12 Mar 2018 21:46:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=hP+2eYV1MLYIZ+sxMK7nHNTWTQeE0GcBPfa7GRMeJ3g=; b=A0/ZQm/RFgwEo9GCZCBv5pLSGIh2ySiheMvG0gVFuh/S487xExhfx+iyBFFf81ro9Z J3/J6myAMeSdGwGTUQZkUowV5Hdxguuakyyd8A8275E1cj7hWJIMwrSVtFd2t/0Y+XCC uvHyfKgVL94sdvPPfe6EICb+Lxde5VKJzco1+vFjTO7+FZ1n3epcIH2G9yTqSOXtxERS Kd5t/K10C1Up5VZ7FPyFcvT/zsZYAtJOmLUM59FvYu1ijb6F0Sbb/gMSf86A5gRohAk5 ZtIlvrbf0yW0YDgOLyBuDz9TZTgw8kmZRSeNoBFiHkvVzNLf30xxNrKAfnbmg3aEToGW sbxw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=hP+2eYV1MLYIZ+sxMK7nHNTWTQeE0GcBPfa7GRMeJ3g=; b=fnyq0TTkhKAW+d4uJc08PLtofdrQqodSRZsjE2LRNUZp0Phje0F6vSUFJt7za2MMlb q3dfeXz1aeaFTRaf1Y7rvdSEswBtRHfDJuOuCKaAeU6h+QUjNHsR8lDKI4dIUwOisFhx Ys/cy1q4E2MWs0mJiHKd9FByFdhCtNQ44Vnc1X5587WWuPFjwpOcjqElxnM7o92Bt3zY KVmC4kg31H3SBCk2RnoeZXDZRjyWZA1BaMjVJkX1qHuqDiSsxgN/FXHzNkY9/UOB9Eby zFBzWcKBe/jc+SO30waRZF2MeXsnIN+ycX4smOuPbpr0QoTSffNj2SAMgs/1k1jJZZps t9Lg== X-Gm-Message-State: AElRT7FiUBigGK+NMIlD3xAp7pio6x0fKyF5sD+vaj2Au0pJfF6Mzklf 5oBxGAVX3pFTPAR6pfz9tYeL9HwYYcdk1dnen0fOPQ== X-Google-Smtp-Source: AG47ELsKCtl+scJxpj8AqnEPFvh0FlB+5PtsRzA0ID+/tmo0r1Bz1tN5RbPDtcmxh1hRORosKGArgqC1yQ0ihsFI/+A= X-Received: by 10.202.218.197 with SMTP id r188mr6246636oig.203.1520916397655; Mon, 12 Mar 2018 21:46:37 -0700 (PDT) MIME-Version: 1.0 Received: by 10.74.203.154 with HTTP; Mon, 12 Mar 2018 21:46:36 -0700 (PDT) In-Reply-To: <21B515A7-C5D8-41C0-AB03-56DC2BA97F52@pgadmin.org> References: <21B515A7-C5D8-41C0-AB03-56DC2BA97F52@pgadmin.org> From: Khushboo Vashi Date: Tue, 13 Mar 2018 10:16:36 +0530 Message-ID: Subject: =?UTF-8?Q?Re=3A_=5BpgAdmin4=5D=5BPatch=5D=3A_RM_=232963_=2D_Backup_database=2C_R?= =?UTF-8?Q?estore_database_and_Maintenance_Database_failed_for_=C3=A9_objec?= =?UTF-8?Q?t=2E?= To: Dave Page Cc: pgadmin-hackers Content-Type: multipart/alternative; boundary="001a113d595a9da173056743f1f9" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --001a113d595a9da173056743f1f9 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Mar 13, 2018 at 9:39 AM, Dave Page wrote: > > > On 12 Mar 2018, at 23:12, Khushboo Vashi > wrote: > > > > On Tue, Mar 13, 2018 at 2:29 AM, Dave Page wrote: > >> So I was trying to test this, and every time I try to run a backup, I'm >> getting the following, with or without your patch: >> >> (sqlite3.ProgrammingError) You must not use 8-bit bytestrings unless you >> use a text_factory that can interpret 8-bit bytestrings (like text_facto= ry >> =3D str). It is highly recommended that you instead just switch your >> application to Unicode strings. [SQL: u'INSERT INTO process (pid, user_i= d, >> command, "desc", arguments, logdir, start_time, end_time, exit_code, >> acknowledge) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)'] [parameters: >> (180312205250107339, 1, u'/Library/PostgreSQL/9.4/bin/pg_dump', >> 'ccopy_reg\n_reconstructor\np0\n(cpgadmin.tools.backup\nBack >> upMessage\np1\nc__builtin__\nobject\np2\nNtp3\nRp4\n(dp5\nS\'cmd\'\np6\n= V >> --file "/Users/dpage/foo.dmp" --host "127.0.0.1" --port "5432" --usernam= e >> "postgres" --no-password --verbose --format=3Dc --blobs >> "\xe9"\np7\nsS\'backup_type\'\np8\nI3\nsS\'database\'\np9\nV >> \xe9\np10\nsS\'bfile\'\np11\nS\'foo.dmp\'\np12\nsS\'sid\'\np13\nI1\nsb.'= , >> u'--file,/Users/dpage/foo.dmp,--host,127.0.0.1,--port,5432,- >> -username,postgres,--no-password,--verbose,--format=3Dc,--blobs,\xe9', >> '/var/lib/pgadmin/sessions/process_logs/180312205250107339', None, None, >> None, None)] >> >> Any thoughts as to what's going on? I wasn't getting this on my other >> laptop, and I can't think what else we would have changed to cause this. >> >> Deleting all the records from the process table from SQLITE will solve > this problem. > There were few issues related to encoding-decoding in my old patches, you > may have applied those. > > > I deleted the database entirely, and still saw the problem. > > I have tried many things to reproduce this but couldn't. I faced this issue when I was fixing the issue but not now. Please make sure to delete the old session and a process table (or database) and apply my latest patch. (of course you do this :) ) I have PY - 2.7.12, psycopg2 - 2.7.4 and SQLAlhemy -1.0.14 to be more specific. > On Mon, Mar 12, 2018 at 2:00 AM, Khushboo Vashi < >> khushboo.vashi@enterprisedb.com> wrote: >> >>> >>> Hi Dave, >>> >>> On Fri, Mar 9, 2018 at 9:09 PM, Dave Page wrote: >>> >>>> Hi >>>> >>>> On Fri, Mar 9, 2018 at 3:32 PM, Dave Page wrote: >>>> >>>>> Hi >>>>> >>>>> On Fri, Mar 9, 2018 at 3:54 AM, Khushboo Vashi < >>>>> khushboo.vashi@enterprisedb.com> wrote: >>>>> >>>>>> Hi, >>>>>> >>>>>> Please find the attached patch to fix below issues: >>>>>> >>>>>> 1. #2963 - Backup database, Restore database and Maintenance Databas= e >>>>>> failed for =C3=A9 object >>>>>> 2. #3157 - Process viewer doesn't show complete command executed. >>>>>> >>>>>> Test cases are not included for these fixes as we don't have test >>>>>> cases for these modules (backup, restore, maintenance). >>>>>> I will create one separate RM for the same which will cover this. >>>>>> >>>>> >>>>> Interesting that you fix these together, as together they also exhibi= t >>>>> another bug :-). Backing up the =C3=A9 database displays the followin= g >>>>> command: >>>>> >>>>> /usr/local/pgsql/bin/pg_dump --file "/Users/dpage/foo.bak" --host >>>>> "localhost" --port "5432" --username "postgres" --no-password --verbo= se >>>>> --format=3Dc --blobs "é" >>>>> >>>> >>>> I can reproduce this issue only with notification dialogue (which I >>> have fixed in the attached patch) not with the details dialogue. Please >>> refer the screenshots for the same. >>> >>>> Also, what tests can we add for backup/restore? We have nothing at all >>>> at the moment, and it is pretty troublesome. I'd like to ensure that w= e can >>>> backup and restore a database correctly, and ensure that the displayed >>>> commands are what we expect and that we get valid output from >>>> pg_dump/pg_restore (though, it may change from PG version to PG versio= n, so >>>> maybe we should just check for something small and generic). I guess t= his >>>> might need some config parameters for the tests to specify the pg_* ut= ility >>>> paths for each server. >>>> >>>> I'd suggest maybe having a feature test that opens the prefs, sets the >>>> appropriate path, then runs a backup, waits for it to finish, checks t= he >>>> process monitor output, then restores the same backup to a new databas= e, >>>> checking the process monitor output again, and then checking that the >>>> restored database contains at least one object from the original datab= ase >>>> (we don't need to check all of pg_dump/pg_restore, just that something >>>> expected was restored). We should use a (partial) database name and ba= ckup >>>> filename from the advanced test config file, and I think both should >>>> default to some interesting non-ASCII strings to ensure quoting works. >>>> >>> I was thinking of writing the unit test cases for the processes.py file >>> as all the major functionalities for backup/restore/maintenance jobs do= ne >>> by this file, but by this we can not achieve the front-end string >>> validation esp for non-ASCII strings. >>> So, I am thinking of writing feature tests (as you have suggested) firs= t >>> and after that if needed I will write unit test cases. >>> >>>> >>>> -- >>>> 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 >> > > --001a113d595a9da173056743f1f9 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Tue, Mar 13, 2018 at 9:39 AM, Dave Page <dpage@pgadmin.org><= /span> wrote:


On 12 Mar 2018, at 23:12, Khushboo Vashi <khushboo.vash= i@enterprisedb.com> wrote:



On Tue, Mar 13, 2018 at 2:29 AM, Dave Page &l= t;dpage@pgadmin.org<= /a>> wrote:
So= I was trying to test this, and every time I try to run a backup, I'm g= etting the following, with or without your patch:

(sqlit= e3.ProgrammingError) You must not use 8-bit bytestrings unless you use a te= xt_factory that can interpret 8-bit bytestrings (like text_factory =3D str)= . It is highly recommended that you instead just switch your application to= Unicode strings. [SQL: u'INSERT INTO process (pid, user_id, command, &= quot;desc", arguments, logdir, start_time, end_time, exit_code, acknow= ledge) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)'] [parameters: (1803122052= 50107339, 1, u'/Library/PostgreSQL/9.4/bin/pg_dump', 'ccop= y_reg\n_reconstructor\np0\n(cpgadmin.tools.backup\nBackupMessage\= np1\nc__builtin__\nobject\np2\nNtp3\nRp4\n(dp5\nS\'cmd\'\= np6\nV --file "/Users/dpage/foo.dmp" --host "127.0.0.1"= --port "5432" --username "postgres" --no-password --ve= rbose --format=3Dc --blobs "\xe9"\np7\nsS\'backup_type\'\= np8\nI3\nsS\'database\'\np9\nV\xe9\np10\nsS\'bfile\&#= 39;\np11\nS\'foo.dmp\'\np12\nsS\'sid\'\np13\nI1\n= sb.', u'--file,/Users/dpage/foo.dmp,--host,127.0.0.1,--port,54= 32,--username,postgres,--no-password,--verbose,--format=3Dc,--blobs,\xe9', '/var/lib/pgadmin/sessions/process_logs/1803122= 05250107339', None, None, None, None)]

Any= thoughts as to what's going on? I wasn't getting this on my other = laptop, and I can't think what else we would have changed to cause this= .

Deleting all the records from the process table from SQLITE wil= l solve this problem.
There were few issues related to encoding-d= ecoding in my old patches, you may have applied those.


On Mon, Mar 12, 2018 at 2:00 AM, Khushboo Vashi <= khushb= oo.vashi@enterprisedb.com> wrote:

Hi Dave,

On Fri, Ma= r 9, 2018 at 9:09 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Fri, Mar 9, 2018 at 3:32 PM= , Dave Page <dpage@pgadmin.org> wrote:
Hi

On Fri, Mar 9, 2018 at 3:54 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
<= /span>
Hi,

Please find the attached patch to fix below issues:

<= /div>
1. #2963 - Backup database, Restore database and Maintenance Data= base failed for =C3=A9 object
2. #3157 - Process viewer doesn'= ;t show complete command executed.

Test cases are = not included for these fixes as we don't have test cases for these modu= les (backup, restore, maintenance).
I will create one separate RM= for the same which will cover this.
Interesting that you fix these together, as together they also= exhibit another bug :-). Backing up the =C3=A9= =C2=A0database displays the following command:

<= div>/usr/local/pgsql/bin/pg_dump --file &quo= t;/Users/dpage/foo.bak" --host "localhost" --port "5432= " --username "postgres" --no-password --verbose --format=3Dc= --blobs "&#233;"=C2=A0

I ca= n reproduce this issue only with notification dialogue (which I have fixed = in the attached patch) not with the details dialogue. Please refer the scre= enshots for the same.
Also, what tests can we add for backup/restore? We have nothi= ng at all at the moment, and it is pretty troublesome. I'd like to ensu= re that we can backup and restore a database correctly, and ensure that the= displayed commands are what we expect and that we get valid output from pg= _dump/pg_restore (though, it may change from PG version to PG version, so m= aybe we should just check for something small and generic). I guess this mi= ght need some config parameters for the tests to specify the pg_* utility p= aths for each server.=C2=A0

I'd suggest maybe = having a feature test that opens the prefs, sets the appropriate path, then= runs a backup, waits for it to finish, checks the process monitor output, = then restores the same backup to a new database, checking the process monit= or output again, and then checking that the restored database contains at l= east one object from the original database (we don't need to check all = of pg_dump/pg_restore, just that something expected was restored). We shoul= d use a (partial) database name and backup filename from the advanced test = config file, and I think both should default to some interesting non-ASCII = strings to ensure quoting works.
I was thinking of writing the unit test cases for the processes.py fi= le as all the major functionalities for backup/restore/maintenance jobs don= e by this file, but by this we can not achieve the front-end string validat= ion esp for non-ASCII strings.
So, I am thinking of writing featu= re tests (as you have suggested) first and after that if needed I will writ= e unit test cases.

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

Enterpr= iseDB UK: http://= www.enterprisedb.com
The Enterprise PostgreSQL Company




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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Co= mpany


--001a113d595a9da173056743f1f9--