public inbox for [email protected]
help / color / mirror / Atom feedFrom: Khushboo Vashi <[email protected]>
To: Dave Page <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4][Patch]: RM #2963 - Backup database, Restore database and Maintenance Database failed for é object.
Date: Mon, 12 Mar 2018 17:17:16 +0530
Message-ID: <CAFOhELcd+i0gZL3Jg073O9a8xFq7FTi80CyyTtogmFVRHEcyvQ@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxoxBF8dT29ZYEf71mWuRp+bE+LhUy4ZGFPjdVxuW-n452Q@mail.gmail.com>
References: <CAFOhELf74n8cKE9y_4ppNs31Qi87WDLry+TV7K_bpAmdczM2bg@mail.gmail.com>
<CA+OCxozdsAg3-jv4yhv9G9Kre4KmO+zWYfkzmVTGrfjX=OkvWA@mail.gmail.com>
<CAFOhELfcJSs+nE3Z5bgFfNkVVmuavi8GvQ-uwrKPO7Y+hVf79w@mail.gmail.com>
<CA+OCxowgspEsUwg8cGcyZH=whCS8n26NePw0cPycSJ=H+PZT8Q@mail.gmail.com>
<CAFOhELdv_EFFctXmLNVTeH4dj7EdQ_oabAUxeC+oho+S4Q-Jww@mail.gmail.com>
<CA+OCxoxKoZ7Mijapqb=oFh34jFNrs5xeuRWNH+gZhMtxkGFu3w@mail.gmail.com>
<CA+OCxoz-N=7QMVh05R6dpQxugyx-z=+4TPOYB6z4vhZA_Ks3tA@mail.gmail.com>
<CAFOhELftpncVyoK=H4L-yZco_XWbk+x0XwRx1A5QgLkgXpu2MQ@mail.gmail.com>
<CA+OCxoxBF8dT29ZYEf71mWuRp+bE+LhUy4ZGFPjdVxuW-n452Q@mail.gmail.com>
On Mon, Mar 12, 2018 at 5:15 PM, Dave Page <[email protected]> wrote:
> Hi
>
> On Mon, Mar 12, 2018 at 2:00 AM, Khushboo Vashi <
> [email protected]> wrote:
>
>>
>> Hi Dave,
>>
>> On Fri, Mar 9, 2018 at 9:09 PM, Dave Page <[email protected]> wrote:
>>
>>> Hi
>>>
>>> On Fri, Mar 9, 2018 at 3:32 PM, Dave Page <[email protected]> wrote:
>>>
>>>> Hi
>>>>
>>>> On Fri, Mar 9, 2018 at 3:54 AM, Khushboo Vashi <
>>>> [email protected]> 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 "é"
>>>>
>>>
>>> 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.
>>
>
> Huh, interesting. What Python version were you using? I had 2.7.
>
>
PY 2.7 and PY 3.5
> 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.
>>
>
> Sounds good. Let's try to keep the feature testing minimal, and expand the
> coverage with unit tests.
>
> Sure.
> Thanks!
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
view thread (19+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: Re: [pgAdmin4][Patch]: RM #2963 - Backup database, Restore database and Maintenance Database failed for é object.
In-Reply-To: <CAFOhELcd+i0gZL3Jg073O9a8xFq7FTi80CyyTtogmFVRHEcyvQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox