Re: [pgAdmin4][Patch]: RM #2963 - Backup database, Restore database and Maintenance Database failed for é object.

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch]: RM #2963 - Backup database, Restore database and Maintenance Database failed for é object.
Date: 2018-03-12 20:59:45
Message-ID: CA+OCxoxajGF0w68UESWB0c_4e=MtoQDsco8DFGtMgpGviiykjA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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_factory
= str). It is highly recommended 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/bin/pg_dump',
'ccopy_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 --verbose --format=c --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=c,--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.

On Mon, Mar 12, 2018 at 2:00 AM, Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

>
> Hi Dave,
>
> On Fri, Mar 9, 2018 at 9:09 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> Hi
>>
>> On Fri, Mar 9, 2018 at 3:32 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>> Hi
>>>
>>> On Fri, Mar 9, 2018 at 3:54 AM, Khushboo Vashi <
>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi,
>>>>
>>>> Please find the attached patch to fix below issues:
>>>>
>>>> 1. #2963 - Backup database, Restore database and Maintenance Database
>>>> failed for é 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 exhibit
>>> another bug :-). Backing up the é database displays the following
>>> command:
>>>
>>> /usr/local/pgsql/bin/pg_dump --file "/Users/dpage/foo.bak" --host
>>> "localhost" --port "5432" --username "postgres" --no-password --verbose
>>> --format=c --blobs "&#233;"
>>>
>>
>> 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 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
>> 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.
>>
>> 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 monitor output again, and then checking that the
>> restored database contains at least 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 should 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 file as
> all the major functionalities for backup/restore/maintenance 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 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

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Joao De Almeida Pereira 2018-03-12 21:15:32 Re: pgAdmin 4 commit: Support for external tables in GPDB. Fixes #3168
Previous Message Dave Page 2018-03-12 20:46:11 Re: [pgAdmin4][RM#3140] Add service parameter