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

> On 12 Mar 2018, at 23:12, Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>
>
>> On Tue, Mar 13, 2018 at 2:29 AM, Dave Page <dpage(at)pgadmin(dot)org> 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_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.
>>
> 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.

>>> 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
>