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 1evbGJ-0006ju-AZ for pgadmin-hackers@arkaria.postgresql.org; Tue, 13 Mar 2018 04:10:08 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1evbGH-0005MI-MA for pgadmin-hackers@arkaria.postgresql.org; Tue, 13 Mar 2018 04:10:05 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1evbGG-0005M5-QQ for pgadmin-hackers@lists.postgresql.org; Tue, 13 Mar 2018 04:10:05 +0000 Received: from mail-qt0-x235.google.com ([2607:f8b0:400d:c0d::235]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1evbG8-0005h9-AG for pgadmin-hackers@postgresql.org; Tue, 13 Mar 2018 04:10:03 +0000 Received: by mail-qt0-x235.google.com with SMTP id a23so21370436qtn.0 for ; Mon, 12 Mar 2018 21:09:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pgadmin-org.20150623.gappssmtp.com; s=20150623; h=mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=x0nmcV/LTN2hm+tOR7E9kVLaKMB/Z1VILFoUHEz30I0=; b=BgHNdQUZrVUFsOEykikKsOdQeqjwPsAggw5KS1l7r0s+XhS9q5EovL7rPVNyuA4msW EiTNqeYVnxcpbMWF0mFmuG1HNXxU4Mi2g5w6xSkofjp4JmIh2SuYfFdUh8ZRjo2xW5L6 gRnLTP0DQFrHsg4nIHPHC06nBaenOzwaJGMelllfhrtSd6SZ2bq730FVV9LnwgaeKfPL ARvZJ6DPEq58razyD2+7igpD0Dn8XQTw7/Ddp8PtYUQt/i5TY0xdyguLSKIwu9aIbwwn 49DhXL/0F+TAGSNJewkGRdF3l8MYq5C47Cw5LcWKn/YzNrdTlUvX28bqrDoJGRt6BHTJ +l9g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=x0nmcV/LTN2hm+tOR7E9kVLaKMB/Z1VILFoUHEz30I0=; b=KAbRIINsobics12vWSDq2zRfiTb4TmBrYbj26zjxtQg5qKjwSe/37Bsa2kSgdi9/K6 SZBoN5GPeVXm6zJYZxF4TJTK7C6wMJFaqHQ1l+jIODktl3iUTTRGxbPAZ0c/amawrGO3 qvA4QPZTU7IXIZ6YTFtjortsqh75VeKXxqW8ojrJdEGm3DY3DOVGni/fm8OZdfbA8bD5 CUsLBJnEXL2T+ka1kw4NSluz7hh5vrShsnOOl+PWZroNjw3xONs6vjf7/978W837tK8h yJWZJ40ZfrrE0ymPoY/ikjIl1p7z0wFu8RSierwY5rreNey3vqldjkp8GA017191K0dU VVaA== X-Gm-Message-State: AElRT7H43cbw4aOrWolLPBuN/N+FIRwCNUoYdwzfs/Bh31Kg7QNgXKSg mdwEnJ5/Vi13swfT4gIoLJ+v1Q== X-Google-Smtp-Source: AG47ELuAk/gXKRXonxBkq2Gftjv0Ff3rc8DPsBtPAGYZqo0pXF8BTA9VPkfbH4k1JqtvSpQ/CxpLZw== X-Received: by 10.200.63.102 with SMTP id w35mr14992675qtk.318.1520914194091; Mon, 12 Mar 2018 21:09:54 -0700 (PDT) Received: from [172.20.0.177] ([50.226.244.131]) by smtp.gmail.com with ESMTPSA id y35sm6680710qth.31.2018.03.12.21.09.52 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 12 Mar 2018 21:09:52 -0700 (PDT) Content-Type: multipart/alternative; boundary=Apple-Mail-74756592-EF5D-4AE8-BDBC-669EC671EC8E Mime-Version: 1.0 (1.0) Subject: =?utf-8?Q?Re:_[pgAdmin4][Patch]:_RM_#2963_-_Backup_database,_Res?= =?utf-8?Q?tore_database_and_Maintenance_Database_failed_for_?= =?utf-8?Q?=C3=A9_object.?= From: Dave Page X-Mailer: iPad Mail (15D100) In-Reply-To: Date: Tue, 13 Mar 2018 00:09:50 -0400 Cc: pgadmin-hackers Content-Transfer-Encoding: 7bit Message-Id: <21B515A7-C5D8-41C0-AB03-56DC2BA97F52@pgadmin.org> References: To: Khushboo Vashi List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --Apple-Mail-74756592-EF5D-4AE8-BDBC-669EC671EC8E Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable > On 12 Mar 2018, at 23:12, Khushboo Vashi = wrote: >=20 >=20 >=20 >> 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 g= etting the following, with or without your patch: >>=20 >> (sqlite3.ProgrammingError) You must not use 8-bit bytestrings unless you u= se a text_factory that can interpret 8-bit bytestrings (like text_factory =3D= str). It is highly recommended that you instead just switch your applicatio= n to Unicode strings. [SQL: u'INSERT INTO process (pid, user_id, command, "d= esc", arguments, logdir, start_time, end_time, exit_code, acknowledge) VALUE= S (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)'] [parameters: (180312205250107339, 1, u'/L= ibrary/PostgreSQL/9.4/bin/pg_dump', 'ccopy_reg\n_reconstructor\np0\n(cpgadmi= n.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 "5= 432" --username "postgres" --no-password --verbose --format=3Dc --blobs "\xe= 9"\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,--ver= bose,--format=3Dc,--blobs,\xe9', '/var/lib/pgadmin/sessions/process_logs/180= 312205250107339', None, None, None, None)] >>=20 >> Any thoughts as to what's going on? I wasn't getting this on my other lap= top, and I can't think what else we would have changed to cause this. >>=20 > Deleting all the records from the process table from SQLITE will solve thi= s problem. > There were few issues related to encoding-decoding in my old patches, you m= ay have applied those. I deleted the database entirely, and still saw the problem. >>> On Mon, Mar 12, 2018 at 2:00 AM, Khushboo Vashi wrote: >>>=20 >>> Hi Dave, >>>=20 >>>> On Fri, Mar 9, 2018 at 9:09 PM, Dave Page wrote: >>>> Hi >>>>=20 >>>>> On Fri, Mar 9, 2018 at 3:32 PM, Dave Page wrote: >>>>> Hi >>>>>=20 >>>>>> On Fri, Mar 9, 2018 at 3:54 AM, Khushboo Vashi wrote: >>>>>> Hi, >>>>>>=20 >>>>>> Please find the attached patch to fix below issues: >>>>>>=20 >>>>>> 1. #2963 - Backup database, Restore database and Maintenance Database= failed for =C3=A9 object >>>>>> 2. #3157 - Process viewer doesn't show complete command executed. >>>>>>=20 >>>>>> Test cases are not included for these fixes as we don't have test cas= es for these modules (backup, restore, maintenance). >>>>>> I will create one separate RM for the same which will cover this. >>>>>=20 >>>>> Interesting that you fix these together, as together they also exhibit= another bug :-). Backing up the =C3=A9 database displays the following comm= and: >>>>>=20 >>>>> /usr/local/pgsql/bin/pg_dump --file "/Users/dpage/foo.bak" --host "loc= alhost" --port "5432" --username "postgres" --no-password --verbose --format= =3Dc --blobs "é"=20 >>>>=20 >>>=20 >>> I can reproduce this issue only with notification dialogue (which I have= fixed in the attached patch) not with the details dialogue. Please refer th= e screenshots for the same. >>>> Also, what tests can we add for backup/restore? We have nothing at all a= t the moment, and it is pretty troublesome. I'd like to ensure that we can b= ackup and restore a database correctly, and ensure that the displayed comman= ds 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 maybe we should just= check for something small and generic). I guess this might need some config= parameters for the tests to specify the pg_* utility paths for each server.= =20 >>>>=20 >>>> I'd suggest maybe having a feature test that opens the prefs, sets the a= ppropriate path, then runs a backup, waits for it to finish, checks the proc= ess monitor output, then restores the same backup to a new database, checkin= g the process monitor output again, and then checking that the restored data= base contains at least one object from the original database (we don't need t= o check all of pg_dump/pg_restore, just that something expected was restored= ). We should use a (partial) database name and backup filename from the adva= nced test config file, and I think both should default to some interesting n= on-ASCII strings to ensure quoting works. >>>=20 >>> I was thinking of writing the unit test cases for the processes.py file a= s all the major functionalities for backup/restore/maintenance jobs done by t= his 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) first= and after that if needed I will write unit test cases. >>>>=20 >>>> --=20 >>>> Dave Page >>>> Blog: http://pgsnake.blogspot.com >>>> Twitter: @pgsnake >>>>=20 >>>> EnterpriseDB UK: http://www.enterprisedb.com >>>> The Enterprise PostgreSQL Company >>>=20 >>=20 >>=20 >>=20 >> --=20 >> Dave Page >> Blog: http://pgsnake.blogspot.com >> Twitter: @pgsnake >>=20 >> EnterpriseDB UK: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company >=20 --Apple-Mail-74756592-EF5D-4AE8-BDBC-669EC671EC8E Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable

On 12 Mar 2018, at 23:12, Khus= hboo Vashi <khushboo.v= ashi@enterprisedb.com> wrote:

=


On Tue, Mar 13, 2018 at 2:29 AM, Dave Page <dpage@pgadmin.org>= wrote:
So I was tr= ying to test this, and every time I try to run a backup, I'm getting the fol= lowing, with or without your patch:

(sqlite3.ProgrammingE= rror) You must not use 8-bit bytestrings unless you use a text_factory that c= an interpret 8-bit bytestrings (like text_factory =3D str). It is highly rec= ommended that you instead just switch your application to Unicode strings. [= SQL: u'INSERT INTO process (pid, user_id, command, "desc", arguments, logdir= , start_time, end_time, exit_code, acknowledge) VALUES (?, ?, ?, ?, ?, ?, ?,= ?, ?, ?)'] [parameters: (180312205250107339, 1, u'/Library/PostgreSQL/9.4/b= in/pg_dump', 'ccopy_reg\n_reconstructor\np0\n(cpgadmin.tools.backu= p\nBackupMessage\np1\nc__builtin__\nobject\np2\nNtp3\nRp4\n(d= p5\nS\'cmd\'\np6\nV --file "/Users/dpage/foo.dmp" --host "127.0.0.1" --port "= 5432" --username "postgres" --no-password --verbose --format=3Dc --blobs "\x= e9"\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,--userna= me,postgres,--no-password,--verbose,--format=3Dc,--blobs,\xe9', '/= var/lib/pgadmin/sessions/process_logs/180312205250107339', None, N= one, None, None)]

Any thoughts as to what's goi= ng 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 th= is problem.
There were few issues related to encoding-decoding in m= y old patches, you may have applied those.

I deleted the database entirely, and still saw t= he problem.

On Mon, Mar 12, 2018 at 2:00 AM, Khushboo Vashi <= span dir=3D"ltr"><khushboo.vashi@enterprisedb.com> wrote:
=

Hi Dave,

On Fri, Mar 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:
<= blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px #= ccc solid;padding-left:1ex">
Hi,

Pl= ease find the attached patch to fix below issues:

1= . #2963 - Backup database, Restore database and Maintenance Database failed f= or =C3=A9 object
2. #3157 - Process viewer doesn't show complete c= ommand executed.

Test cases are not included for th= ese fixes as we don't have test cases for these modules (backup, restore, ma= intenance).
I will create one separate RM for the same which will c= over this.

Interesting th= at you fix these together, as together they also exhibit another bug :-). Ba= cking up the =C3=A9 database displays the f= ollowing command:

/usr/lo= cal/pgsql/bin/pg_dump --file "/Users/dpage/foo.bak" --host "localhost" --por= t "5432" --username "postgres" --no-password --verbose --format=3Dc --blobs "= &#233;" 

=
I can reproduce this issue= only with notification dialogue (which I have fixed in the attached patch) n= ot with the details dialogue. Please refer the screenshots for the same.
Also, what tes= ts can we add for backup/restore? We have nothing at all at the moment, and i= t is pretty troublesome. I'd like to ensure that we can backup and restore a= database correctly, and ensure that the displayed commands are what we expe= ct and that we get valid output from pg_dump/pg_restore (though, it may chan= ge from PG version to PG version, so maybe we should just check for somethin= g small and generic). I guess this might need some config parameters for the= tests to specify the pg_* utility paths for each server. 
I'd suggest maybe having a feature test that opens the prefs, s= ets 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 monitor output again, and then checking that the rest= ored database contains at least one object from the original database (we do= n't need to check all of pg_dump/pg_restore, just that something expected wa= s restored). We should use a (partial) database name and backup filename fro= m the advanced test config file, and I think both should default to some int= eresting non-ASCII strings to ensure quoting works.
<= /blockquote>
I was thinking of writing the unit test cases for th= e processes.py file as all the major functionalities for backup/restore/main= tenance jobs done 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) first and after that if neede= d I will write unit test cases.

--
Dave Pa= ge
Blog: http:/= /pgsnake.blogspot.com
Twitter: @pgsnake

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




--
D= ave Page
Blog: = http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK:= http://www.enterp= risedb.com
The Enterprise PostgreSQL Company

= --Apple-Mail-74756592-EF5D-4AE8-BDBC-669EC671EC8E--