Re: [patch][pgAdmin] Fix for pgadmin4-linux-qa #1651 failure

2021-06-30 Thread Rahul Shirsat
Hi All,

Please find the attached patch for resolving this issue wrt above
suggestion.

Let me know if anyone has any queries.

On Tue, Jun 29, 2021 at 9:20 PM Rahul Shirsat <
rahul.shir...@enterprisedb.com> wrote:

> Hi Dave / Aditya,
>
> For a time being, Let's make a call to gettext conditional instead of
> passing dynamic parameters for this scenario at least. With this, we won't
> be touching the *.po files and translations will do its task smoothly.
> I have already checked for the string with weird unprintable characters,
> and there were none.
>
> e.g.
> Instead of -  *title = gettext('%s objects?', obj.label);*
>
> *if (drop) {*
> *   title = gettext('Drop objects?');*
> *}*
> *else {*
>*title = gettext('Reassign objects?');*
> *}*
>
> I have tried the above code and it looks good to me.
>
> Please suggest.
>
> On Tue, Jun 29, 2021 at 7:31 PM Dave Page  wrote:
>
>>
>>
>> On Tue, Jun 29, 2021 at 2:55 PM Aditya Toshniwal <
>> aditya.toshni...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> On Tue, Jun 29, 2021 at 7:14 PM Dave Page  wrote:
>>>


 On Tue, Jun 29, 2021 at 2:41 PM Aditya Toshniwal <
 aditya.toshni...@enterprisedb.com> wrote:

> Dave,
>
> Somehow, the new text strings are added to PO with incorrect
> translations. That is causing the issue.
> Either they should be empty or fixed.
>

 Then the source problem should be fixed. There's no point at all in
 putting fixes directly in the PO files as they'll be overwritten prior to
 release anyway.

>>> The translations submitted by translators are updated in the PO file
>>> right ? And then they're compiled to MO ?
>>>
>>
>> Right.
>>
>>
>>> It's the same like Rahul will be submitting the translations. Please
>>> correct me if I'm wrong.
>>>
>>
>> That depends if he's changed the original message or the translated
>> message. It's impossible to see without reading megabytes of text.
>>
>> And in any case, he's updated translations that haven't been touched in
>> ages - why would they suddenly have broken? (hint: if the upstream message
>> has changed, it'll be marked as fuzzy or untranslated - in other words,
>> gettext knows how to handle that).
>>
>>
>>>

>
> On Tue, Jun 29, 2021 at 7:01 PM Dave Page  wrote:
>
>> Hi
>>
>> Please send the patch without updates to the po files. Those get
>> updated as part of the release process.
>>
>> Thanks.
>>
>> On Tue, Jun 29, 2021 at 2:00 PM Rahul Shirsat <
>> rahul.shir...@enterprisedb.com> wrote:
>>
>>> Hi Hackers,
>>>
>>> Thanks Aditya for pointing out the issue. Please find the attached
>>> patch which contains all the .po files corrected with %s.
>>>
>>> Regards,
>>> Rahul Shirsat.
>>>
>>> On Tue, Jun 29, 2021 at 4:31 PM Aditya Toshniwal <
>>> aditya.toshni...@enterprisedb.com> wrote:
>>>
 Hi Rahul,

 I did "make msg-extract" and "make msg-update" and looking at the
 PO files I think it is not updated correctly.
 For instance, the below message has msgstr without %s. I corrected
 it and the error was gone.

 #: pgadmin/browser/server_groups/servers/roles/static/js/role.js:766
 #, fuzzy, python-format
 msgid "%s Objects"
 msgstr "Obiekty"

 The one below had 2 %s in msgstr and I corrected it to fix the
 error.

 #: pgadmin/browser/server_groups/servers/roles/static/js/role.js:767
 #, fuzzy, python-format
 msgid "Are you sure you wish to %s all the objects owned by the
 selected role?"
 msgstr "Czy na pewno skasować %s \"%s\" i wszystkie obiekty zależne
 od niego?"


 You have to update the .po files to match the total %s and send the
 patch.

 On Tue, Jun 29, 2021 at 1:56 PM Dave Page 
 wrote:

> Hi
>
> On Tue, Jun 29, 2021 at 4:38 AM Rahul Shirsat <
> rahul.shir...@enterprisedb.com> wrote:
>
>> I feel gettext sometimes won't escape the characters as it should
>> be.
>>
>> I now tried to escape those using some utils.
>>
>
> That won't work either. The string being passed to gettext()
> *must* be in the gettext call.
>
> When gettext extracts strings to create/update the catalogs, it
> will search the code for all gettext calls, and then extract a string
> constant from the first argument. You cannot have variables, function 
> calls
> or expressions in there. It *must* be a string constant.
>
> Keep in mind that msgextract is scanning the source code; it's not
> executing it. There are many examples in the code, e.g. (from 
> node.js):
>
> title = gettext('Drop %s?', obj.label);
>
> I don't see anyth

Re: [patch][pgAdmin] Fix for pgadmin4-linux-qa #1651 failure

2021-06-30 Thread Dave Page
Hi

On Wed, Jun 30, 2021 at 8:28 AM Rahul Shirsat <
rahul.shir...@enterprisedb.com> wrote:

> Hi All,
>
> Please find the attached patch for resolving this issue wrt above
> suggestion.
>

Well that may fix the problem (and is a reasonable change), however, I
think it's important that we understand the root cause. Why is this failing
on Linux only? Why does the following from node.js (which follows the same
pattern) work fine?

var type_label = gettext('%s Script',stype.toUpperCase());



> Let me know if anyone has any queries.
>
> On Tue, Jun 29, 2021 at 9:20 PM Rahul Shirsat <
> rahul.shir...@enterprisedb.com> wrote:
>
>> Hi Dave / Aditya,
>>
>> For a time being, Let's make a call to gettext conditional instead of
>> passing dynamic parameters for this scenario at least. With this, we won't
>> be touching the *.po files and translations will do its task smoothly.
>> I have already checked for the string with weird unprintable characters,
>> and there were none.
>>
>> e.g.
>> Instead of -  *title = gettext('%s objects?', obj.label);*
>>
>> *if (drop) {*
>> *   title = gettext('Drop objects?');*
>> *}*
>> *else {*
>>*title = gettext('Reassign objects?');*
>> *}*
>>
>> I have tried the above code and it looks good to me.
>>
>> Please suggest.
>>
>> On Tue, Jun 29, 2021 at 7:31 PM Dave Page  wrote:
>>
>>>
>>>
>>> On Tue, Jun 29, 2021 at 2:55 PM Aditya Toshniwal <
>>> aditya.toshni...@enterprisedb.com> wrote:
>>>
 Hi,

 On Tue, Jun 29, 2021 at 7:14 PM Dave Page  wrote:

>
>
> On Tue, Jun 29, 2021 at 2:41 PM Aditya Toshniwal <
> aditya.toshni...@enterprisedb.com> wrote:
>
>> Dave,
>>
>> Somehow, the new text strings are added to PO with incorrect
>> translations. That is causing the issue.
>> Either they should be empty or fixed.
>>
>
> Then the source problem should be fixed. There's no point at all in
> putting fixes directly in the PO files as they'll be overwritten prior to
> release anyway.
>
 The translations submitted by translators are updated in the PO file
 right ? And then they're compiled to MO ?

>>>
>>> Right.
>>>
>>>
 It's the same like Rahul will be submitting the translations. Please
 correct me if I'm wrong.

>>>
>>> That depends if he's changed the original message or the translated
>>> message. It's impossible to see without reading megabytes of text.
>>>
>>> And in any case, he's updated translations that haven't been touched in
>>> ages - why would they suddenly have broken? (hint: if the upstream message
>>> has changed, it'll be marked as fuzzy or untranslated - in other words,
>>> gettext knows how to handle that).
>>>
>>>

>
>>
>> On Tue, Jun 29, 2021 at 7:01 PM Dave Page  wrote:
>>
>>> Hi
>>>
>>> Please send the patch without updates to the po files. Those get
>>> updated as part of the release process.
>>>
>>> Thanks.
>>>
>>> On Tue, Jun 29, 2021 at 2:00 PM Rahul Shirsat <
>>> rahul.shir...@enterprisedb.com> wrote:
>>>
 Hi Hackers,

 Thanks Aditya for pointing out the issue. Please find the attached
 patch which contains all the .po files corrected with %s.

 Regards,
 Rahul Shirsat.

 On Tue, Jun 29, 2021 at 4:31 PM Aditya Toshniwal <
 aditya.toshni...@enterprisedb.com> wrote:

> Hi Rahul,
>
> I did "make msg-extract" and "make msg-update" and looking at the
> PO files I think it is not updated correctly.
> For instance, the below message has msgstr without %s. I corrected
> it and the error was gone.
>
> #:
> pgadmin/browser/server_groups/servers/roles/static/js/role.js:766
> #, fuzzy, python-format
> msgid "%s Objects"
> msgstr "Obiekty"
>
> The one below had 2 %s in msgstr and I corrected it to fix the
> error.
>
> #:
> pgadmin/browser/server_groups/servers/roles/static/js/role.js:767
> #, fuzzy, python-format
> msgid "Are you sure you wish to %s all the objects owned by the
> selected role?"
> msgstr "Czy na pewno skasować %s \"%s\" i wszystkie obiekty
> zależne od niego?"
>
>
> You have to update the .po files to match the total %s and send
> the patch.
>
> On Tue, Jun 29, 2021 at 1:56 PM Dave Page 
> wrote:
>
>> Hi
>>
>> On Tue, Jun 29, 2021 at 4:38 AM Rahul Shirsat <
>> rahul.shir...@enterprisedb.com> wrote:
>>
>>> I feel gettext sometimes won't escape the characters as it
>>> should be.
>>>
>>> I now tried to escape those using some utils.
>>>
>>
>> That won't work either. The string being passed to gettext()
>> *must* be in the gettext call.
>>
>> When gettext

pgAdmin 4 commit: Disable email deliverability check that was introduce

2021-06-30 Thread Dave Page
Disable email deliverability check that was introduced in flask-security-too by 
default to maintain backwards compatibility.

Fixes #6550

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=b98e881d8440737b387debb7a8dbaf910f4d2349

Modified Files
--
docs/en_US/release_notes_5_5.rst |  1 +
web/config.py| 10 ++
2 files changed, 11 insertions(+)



pgAdmin 4 commit: PEP-8 fix.

2021-06-30 Thread Dave Page
PEP-8 fix.

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=a2b67b933e4b00ac266d55649dc7424366511a78

Modified Files
--
web/config.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)



RM 6427 - Newlines in text columns of resultset show an allegedly "empty" field

2021-06-30 Thread Dave Page
Here's a simple patch for $SUBJECT. It will remove any leading spaces from
the grid cell display and replace them with '[...] '. In edit mode, the
data is unchanged.

The only downside I can see to this is that the tooltip will show the
modified data when it probably should actually show the un-modified data.
Unfortunately that's handled by a SlickGrid plugin, which would have to be
vendorised and modified (perhaps non-trivially). I don't think that's a big
issue though.

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

EDB: https://www.enterprisedb.com


RM_6427.diff
Description: Binary data


Re: RM 6427 - Newlines in text columns of resultset show an allegedly "empty" field

2021-06-30 Thread Khushboo Vashi
Hi Dave,

On Wed, Jun 30, 2021 at 3:35 PM Dave Page  wrote:

> Here's a simple patch for $SUBJECT. It will remove any leading spaces from
> the grid cell display and replace them with '[...] '. In edit mode, the
> data is unchanged.
>
> The patch looks good to me.
Do we need to specify somewhere what is [...] ?

The only downside I can see to this is that the tooltip will show the
> modified data when it probably should actually show the un-modified data.
> Unfortunately that's handled by a SlickGrid plugin, which would have to be
> vendorised and modified (perhaps non-trivially). I don't think that's a big
> issue though.
>
> --
> Dave Page
> Blog: https://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EDB: https://www.enterprisedb.com
>
>


Re: RM 6427 - Newlines in text columns of resultset show an allegedly "empty" field

2021-06-30 Thread Dave Page
Hi

On Wed, Jun 30, 2021 at 12:36 PM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi Dave,
>
> On Wed, Jun 30, 2021 at 3:35 PM Dave Page  wrote:
>
>> Here's a simple patch for $SUBJECT. It will remove any leading spaces
>> from the grid cell display and replace them with '[...] '. In edit mode,
>> the data is unchanged.
>>
>> The patch looks good to me.
>

Thanks.


> Do we need to specify somewhere what is [...] ?
>

I don't think there's much point.


>
> The only downside I can see to this is that the tooltip will show the
>> modified data when it probably should actually show the un-modified data.
>> Unfortunately that's handled by a SlickGrid plugin, which would have to be
>> vendorised and modified (perhaps non-trivially). I don't think that's a big
>> issue though.
>>
>> --
>> Dave Page
>> Blog: https://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EDB: https://www.enterprisedb.com
>>
>>

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

EDB: https://www.enterprisedb.com


Re: [patch][pgAdmin] Fix for pgadmin4-linux-qa #1651 failure

2021-06-30 Thread Dave Page
Hi

On Wed, Jun 30, 2021 at 9:22 AM Dave Page  wrote:

> Hi
>
> On Wed, Jun 30, 2021 at 8:28 AM Rahul Shirsat <
> rahul.shir...@enterprisedb.com> wrote:
>
>> Hi All,
>>
>> Please find the attached patch for resolving this issue wrt above
>> suggestion.
>>
>
> Well that may fix the problem (and is a reasonable change), however, I
> think it's important that we understand the root cause. Why is this failing
> on Linux only? Why does the following from node.js (which follows the same
> pattern) work fine?
>
> var type_label = gettext('%s Script',stype.toUpperCase());
>

Rahul and I figured out the root cause. The issue is occuring because the
previous string had no parameters (i.e. no %s's). Because fuzzy matching is
used for the translations, when updating the catalogs it was matching with
the old translation, which at runtime would likely have caused a crash
because the catalogs would have contained something like:

#: pgadmin/browser/static/js/node.js:209
#, fuzzy, python-format
msgid "Search %s Objects"
msgstr "Typy obiektów"

There are a few of ways around this:

- Manually fix the translations in each catalog. This is not a good idea
because we don't speak all those languages and will probably mess the
translations up.

- Run something like 'make msg-extract && pybabel update
--no-fuzzy-matching -i web/pgadmin/messages.pot -d web/pgadmin/translations
&& make msg-compile', then commit the results. This will remove all fuzzy
matches from the catalogs, which means more work for the translators on the
next release, but will likely also result in them becoming much cleaner.

- Change the code to use the conditional fix.

I'm leaning towards the second option. In the worst case, it'll lose about
58 fuzzy translations (meaning 58 messages to re-translate), but at least
we'll know they are clean. See
https://www.pgadmin.org/development/translations/ for current stats.

Thoughts?

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

EDB: https://www.enterprisedb.com


Re: [patch][pgAdmin] Fix for pgadmin4-linux-qa #1651 failure

2021-06-30 Thread Aditya Toshniwal
Hi,

On Wed, Jun 30, 2021 at 6:55 PM Dave Page  wrote:

> Hi
>
> On Wed, Jun 30, 2021 at 9:22 AM Dave Page  wrote:
>
>> Hi
>>
>> On Wed, Jun 30, 2021 at 8:28 AM Rahul Shirsat <
>> rahul.shir...@enterprisedb.com> wrote:
>>
>>> Hi All,
>>>
>>> Please find the attached patch for resolving this issue wrt above
>>> suggestion.
>>>
>>
>> Well that may fix the problem (and is a reasonable change), however, I
>> think it's important that we understand the root cause. Why is this failing
>> on Linux only? Why does the following from node.js (which follows the same
>> pattern) work fine?
>>
>> var type_label = gettext('%s Script',stype.toUpperCase());
>>
>
> Rahul and I figured out the root cause. The issue is occuring because the
> previous string had no parameters (i.e. no %s's). Because fuzzy matching is
> used for the translations, when updating the catalogs it was matching with
> the old translation, which at runtime would likely have caused a crash
> because the catalogs would have contained something like:
>
> #: pgadmin/browser/static/js/node.js:209
> #, fuzzy, python-format
> msgid "Search %s Objects"
> msgstr "Typy obiektów"
>
> There are a few of ways around this:
>
> - Manually fix the translations in each catalog. This is not a good idea
> because we don't speak all those languages and will probably mess the
> translations up.
>
> - Run something like 'make msg-extract && pybabel update
> --no-fuzzy-matching -i web/pgadmin/messages.pot -d web/pgadmin/translations
> && make msg-compile', then commit the results. This will remove all fuzzy
> matches from the catalogs, which means more work for the translators on the
> next release, but will likely also result in them becoming much cleaner.
>
> - Change the code to use the conditional fix.
>
> I'm leaning towards the second option. In the worst case, it'll lose about
> 58 fuzzy translations (meaning 58 messages to re-translate), but at least
> we'll know they are clean. See
> https://www.pgadmin.org/development/translations/ for current stats.
>
> Thoughts?
>
Right. I'm in favor of removing the incorrect translations. For which, the
right way is the second option.
Shouldn't we add the no fuzzy flag in the makefile ? This problem can occur
again.

>
> --
> Dave Page
> Blog: https://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EDB: https://www.enterprisedb.com
>
>

-- 
Thanks,
Aditya Toshniwal
pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*

"Don't Complain about Heat, Plant a TREE"


Re: [patch][pgAdmin] Fix for pgadmin4-linux-qa #1651 failure

2021-06-30 Thread Dave Page
On Wed, Jun 30, 2021 at 2:42 PM Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:

> Hi,
>
> On Wed, Jun 30, 2021 at 6:55 PM Dave Page  wrote:
>
>> Hi
>>
>> On Wed, Jun 30, 2021 at 9:22 AM Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Wed, Jun 30, 2021 at 8:28 AM Rahul Shirsat <
>>> rahul.shir...@enterprisedb.com> wrote:
>>>
 Hi All,

 Please find the attached patch for resolving this issue wrt above
 suggestion.

>>>
>>> Well that may fix the problem (and is a reasonable change), however, I
>>> think it's important that we understand the root cause. Why is this failing
>>> on Linux only? Why does the following from node.js (which follows the same
>>> pattern) work fine?
>>>
>>> var type_label = gettext('%s Script',stype.toUpperCase());
>>>
>>
>> Rahul and I figured out the root cause. The issue is occuring because the
>> previous string had no parameters (i.e. no %s's). Because fuzzy matching is
>> used for the translations, when updating the catalogs it was matching with
>> the old translation, which at runtime would likely have caused a crash
>> because the catalogs would have contained something like:
>>
>> #: pgadmin/browser/static/js/node.js:209
>> #, fuzzy, python-format
>> msgid "Search %s Objects"
>> msgstr "Typy obiektów"
>>
>> There are a few of ways around this:
>>
>> - Manually fix the translations in each catalog. This is not a good idea
>> because we don't speak all those languages and will probably mess the
>> translations up.
>>
>> - Run something like 'make msg-extract && pybabel update
>> --no-fuzzy-matching -i web/pgadmin/messages.pot -d web/pgadmin/translations
>> && make msg-compile', then commit the results. This will remove all fuzzy
>> matches from the catalogs, which means more work for the translators on the
>> next release, but will likely also result in them becoming much cleaner.
>>
>> - Change the code to use the conditional fix.
>>
>> I'm leaning towards the second option. In the worst case, it'll lose
>> about 58 fuzzy translations (meaning 58 messages to re-translate), but at
>> least we'll know they are clean. See
>> https://www.pgadmin.org/development/translations/ for current stats.
>>
>> Thoughts?
>>
> Right. I'm in favor of removing the incorrect translations. For which, the
> right way is the second option.
> Shouldn't we add the no fuzzy flag in the makefile ? This problem can
> occur again.
>

Maybe :-). That's the point of this email - to gather such opinions.

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

EDB: https://www.enterprisedb.com


Re: [patch][pgAdmin] Fix for pgadmin4-linux-qa #1651 failure

2021-06-30 Thread Khushboo Vashi
On Wed, Jun 30, 2021 at 6:55 PM Dave Page  wrote:

> Hi
>
> On Wed, Jun 30, 2021 at 9:22 AM Dave Page  wrote:
>
>> Hi
>>
>> On Wed, Jun 30, 2021 at 8:28 AM Rahul Shirsat <
>> rahul.shir...@enterprisedb.com> wrote:
>>
>>> Hi All,
>>>
>>> Please find the attached patch for resolving this issue wrt above
>>> suggestion.
>>>
>>
>> Well that may fix the problem (and is a reasonable change), however, I
>> think it's important that we understand the root cause. Why is this failing
>> on Linux only? Why does the following from node.js (which follows the same
>> pattern) work fine?
>>
>> var type_label = gettext('%s Script',stype.toUpperCase());
>>
>
> Rahul and I figured out the root cause. The issue is occuring because the
> previous string had no parameters (i.e. no %s's). Because fuzzy matching is
> used for the translations, when updating the catalogs it was matching with
> the old translation, which at runtime would likely have caused a crash
> because the catalogs would have contained something like:
>
> #: pgadmin/browser/static/js/node.js:209
> #, fuzzy, python-format
> msgid "Search %s Objects"
> msgstr "Typy obiektów"
>
> There are a few of ways around this:
>
> - Manually fix the translations in each catalog. This is not a good idea
> because we don't speak all those languages and will probably mess the
> translations up.
>
> - Run something like 'make msg-extract && pybabel update
> --no-fuzzy-matching -i web/pgadmin/messages.pot -d web/pgadmin/translations
> && make msg-compile', then commit the results. This will remove all fuzzy
> matches from the catalogs, which means more work for the translators on the
> next release, but will likely also result in them becoming much cleaner.
>
> +1 for pybabel update with -N option

> - Change the code to use the conditional fix.
>
> I'm leaning towards the second option. In the worst case, it'll lose about
> 58 fuzzy translations (meaning 58 messages to re-translate), but at least
> we'll know they are clean. See
> https://www.pgadmin.org/development/translations/ for current stats.
>
> Thoughts?
>
> --
> Dave Page
> Blog: https://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EDB: https://www.enterprisedb.com
>
>