Re: PGAgent 4.0 error feedback

[Ashesh Vashi]

Hi Rob,

On Sun, Jun 10, 2018 at 2:26 AM, Rob Emery  wrote:

> Hi Guys,
>
> I've been testing out PGAgent 4 (build today from master at commit
> 86ca5c5ed1ad572075ba27e05e4680ebdf5b9feb) to check the connection
> handling on error is still fixed with the boost reimplementation
> (which it is!)
> and noticed a few issues compared to PGAgent 3 around feedback to the
> user if the connection string is incorrect.
>
> I've tested with incorrect username and password, user not in the hba,
> db doesn't exist, postgresql not running on the host (or incorrect
> hostaddr) and 4 seems to always returns the error :
> `
> ERROR: Couldn't find the function 'pgagent_schema_version' - please
> run pgagent_upgrade.sql.
> `
>
> whereas 3.4.1 tends to output from the pgconnection itself like:
> `
> Sat Jun  9 21:32:13 2018 : WARNING: Couldn't create the primary
> connection (attempt 1): FATAL:  no pg_hba.conf entry for host
> "172.30.0.16", user "pgagent_login_role", database
> "pgagent_login_role", SSL on
> FATAL:  no pg_hba.conf entry for host "172.30.0.16", user
> "pgagent_login_role", database "pgagent_login_role", SSL off
> `
>
> which at least points the user towards the actual error!
>
> Not sure if this is known already, but I thought I'd raise it prior to
> release.
>
Thanks for sharing the information.

Is it possible for you to share the logs with log level set to debug?
Also - please share the operating system details?

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company



*http://www.linkedin.com/in/asheshvashi*




>
> Thanks,
> Rob
> --
>
> --
>  
>
>
> Codeweavers May Newsletter
>   l  Codeweavers
> April
> Finance Trends
> 
>
>
> April’s Dealer Highlights
> 
>  >
>
> _
> _
>
>
>
> *Phone:* 0800 021 0888   Email: contac...@codeweavers.net
> 
> Codeweavers Ltd | Barn 4 | Dunston
> Business Village | Dunston | ST18 9AB
> Registered in England and Wales No.
> 04092394 | VAT registration no. 974 9705 63
>
>
>
>
> 
> 
> 
> 
>
>
>

[PATCH] pgAgent segfault

[Ashesh Vashi]

Hi Dave,

While debugging the issue

reported by Rob, I found one segfault.
Please find the attached patch to find that fix along with another one.

1. Connection object was being accessed, which was already released.

2. Changed the logic for checking the connection object existence.

We're using "if (!ms_primaryCon)", which should convert the logic as
boolean operator explicitly, which we defined for the 'DBconn' class.

I suspect some compilers (specifically windows one) may not behave that
way, and instead of using the boolean operator, it checks for the pointer
is NULL or not.

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company



*http://www.linkedin.com/in/asheshvashi*



pgagent_segfault.patch
Description: Binary data

Re: PGAgent 4.0 error feedback

[Ashesh Vashi]

Hi Rob,

On Mon, Jun 11, 2018 at 9:13 AM, Ashesh Vashi  wrote:

> Hi Rob,
>
> On Sun, Jun 10, 2018 at 2:26 AM, Rob Emery 
> wrote:
>
>> Hi Guys,
>>
>> I've been testing out PGAgent 4 (build today from master at commit
>> 86ca5c5ed1ad572075ba27e05e4680ebdf5b9feb) to check the connection
>> handling on error is still fixed with the boost reimplementation
>> (which it is!)
>> and noticed a few issues compared to PGAgent 3 around feedback to the
>> user if the connection string is incorrect.
>>
>> I've tested with incorrect username and password, user not in the hba,
>> db doesn't exist, postgresql not running on the host (or incorrect
>> hostaddr) and 4 seems to always returns the error :
>> `
>> ERROR: Couldn't find the function 'pgagent_schema_version' - please
>> run pgagent_upgrade.sql.
>> `
>>
>> whereas 3.4.1 tends to output from the pgconnection itself like:
>> `
>> Sat Jun  9 21:32:13 2018 : WARNING: Couldn't create the primary
>> connection (attempt 1): FATAL:  no pg_hba.conf entry for host
>> "172.30.0.16", user "pgagent_login_role", database
>> "pgagent_login_role", SSL on
>> FATAL:  no pg_hba.conf entry for host "172.30.0.16", user
>> "pgagent_login_role", database "pgagent_login_role", SSL off
>> `
>>
>> which at least points the user towards the actual error!
>>
>> Not sure if this is known already, but I thought I'd raise it prior to
>> release.
>>
> Thanks for sharing the information.
>
> Is it possible for you to share the logs with log level set to debug?
> Also - please share the operating system details?
>

I've sent a patch

to resolve the issue  based on some assumptions, to verify those
assumptions I would still like to see the logs (if you can provide), and
operating system information.

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company



*http://www.linkedin.com/in/asheshvashi
*

>
> --
>
> Thanks & Regards,
>
> Ashesh Vashi
> EnterpriseDB INDIA: Enterprise PostgreSQL Company
> 
>
>
> *http://www.linkedin.com/in/asheshvashi*
> 
>
>
>
>>
>> Thanks,
>> Rob
>> --
>>
>> --
>>  
>>
>>
>> Codeweavers May Newsletter
>>   l  Codeweavers
>> April
>> Finance Trends
>> 
>>
>>
>> April’s Dealer Highlights
>> 
>> > finance-trends>
>>
>> _
>> _
>>
>>
>>
>> *Phone:* 0800 021 0888   Email: contac...@codeweavers.net
>> 
>> Codeweavers Ltd | Barn 4 | Dunston
>> Business Village | Dunston | ST18 9AB
>> Registered in England and Wales No.
>> 04092394 | VAT registration no. 974 9705 63
>>
>>
>>
>>
>> 
>> 
>> 
>> 
>>
>>
>>
>

Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree

[Aditya Toshniwal]

Hi Victoria,

On Thu, Jun 7, 2018 at 8:51 PM, Victoria Henry  wrote:

> Hi Aditya
>
> Sure. I did not find moving web/pgadmin/tools/datagrid/static/js/datagrid.js.
>> Please correct me if I am missing anything.
>
> Generally speaking, I agree with moving/deleting files if it makes sense.
> But in regards to web/pgadmin/tools/datagrid/static/js/datagrid.js, it
> looks like this is still being used in web/pgadmin/tools/sqleditor/
> static/js/sqleditor.js with Datagrid.create_transaction
>
And this is another js which is not moved.(sqleditor.js). If we are moving
js files, why not this too.


> Sincerely,
>
> Victoria
> ​
>
>
>
> On Thu, Jun 7, 2018 at 12:35 AM Aditya Toshniwal  enterprisedb.com> wrote:
>
>> Hi Victoria,
>>
>> On Wed, Jun 6, 2018 at 8:55 PM, Victoria Henry  wrote:
>>
>>> Hi Aditya,
>>>
>>> 1) Why don't we start using webpack alias's instead of using absolute
 path. For eg,
 import {RestoreDialogWrapper} from '../../../pgadmin/static/js/
 restore/restore_dialog_wrapper';
 can be used as import {RestoreDialogWrapper} from
 'pgadmin_static/js/restore/restore_dialog_wrapper';
 by adding pgadmin_static alias to webpack config.
>>>
>>>
>>> Great point. In some areas of the code, we began making this change.
>>> There is already an alias in webpack shims for `
>>> ../../../pgadmin/static/js` called `sources`.  You can find an example
>>> of this in import statements for `supported_database_node.js`
>>>
>>> 2) Few of the js are left behind, the ones which are used in python
 __init__.py. Can't we move them too ? It would be nicer to not to leave
 behind a single js.
>>>
>>> I'm not sure what you mean.  Could you point to an example of a single
>>> js file?
>>>
>>
>> Sure. I did not find moving web/pgadmin/tools/datagrid/static/js/datagrid.js.
>> Please correct me if I am missing anything.
>>
>>>
>>> Sincerely,
>>>
>>> Victoria
>>>
>>> On Wed, Jun 6, 2018 at 7:07 AM Aditya Toshniwal >> enterprisedb.com> wrote:
>>>
 Hi Anthony/Victoria/Joao,

 I know I am very late to review on patch 004. The idea of moving js
 files from tools to static folder looks good, but I have a few suggestions:

 1) Why don't we start using webpack alias's instead of using absolute
 path. For eg,
 import {RestoreDialogWrapper} from '../../../pgadmin/static/js/
 restore/restore_dialog_wrapper';
 can be used as import {RestoreDialogWrapper} from
 'pgadmin_static/js/restore/restore_dialog_wrapper';
 by adding pgadmin_static alias to webpack config.

 2) Few of the js are left behind, the ones which are used in python
 __init__.py. Can't we move them too ? It would be nicer to not to leave
 behind a single js.

 Kindly let me know your views on this.


 Thanks and Regards,
 Aditya Toshniwal
 Software Engineer | EnterpriseDB Software Solutions | Pune
 "Don't Complain about Heat, Plant a tree"

 On Sat, Jun 2, 2018 at 12:47 AM, Victoria Henry 
 wrote:

> Hey Ashesh,
>
> LGTM!  The some of the CI tests failed but it looks like a flake.  But
> you can go ahead and merge this.
>
> Sincerely,
>
> Victoria
>
> On Fri, Jun 1, 2018 at 2:36 PM Ashesh Vashi <
> ashesh.va...@enterprisedb.com> wrote:
>
>> On Fri, Jun 1, 2018 at 10:09 PM, Victoria Henry 
>> wrote:
>>
>>> Hi Ashesh,
>>>
>>> We just attempted to apply your patch over master but it did not
>>> work.  We don't want to introduce any bugs or break any functionality.
>>> Please update the patch to make sure it is synced up with the master 
>>> branch.
>>>
>> Please find the updated patch.
>>
>>>
>>> Sincerely,
>>>
>>> Victoria
>>>
>>> On Fri, Jun 1, 2018 at 11:18 AM Anthony Emengo 
>>> wrote:
>>>
 Hey Ashesh,

 Thanks for the explanation. It was great and it really helped!

 C 
 pgadmin/browser/server_groups/servers/databases/schemas/static/js/child.js
 C 
 pgadmin/browser/server_groups/servers/databases/schemas/static/js/schema_child_tree_node.js

 It makes sense to remove duplication by extracting these attributes
 out and setting the canDrop and canCreate functions here. But is
 it possible to combine these two files into one since they are related 
 so
 we don’t need to import schema_child_tree_node?

>>> That was the original plan, but 'pgadmin/browser/static/js//node.js'
>> script has too many dependecies, which are not easily portable.
>> And - that may lead to change the scope of the patch.
>>
>> Hence - I decided to use the separate file to make sure we have
>> enough test coverage (which is more imprortant than changing the scope).
>>
>>> M pgadmin/static/js/tree/tree.js

 The creation of the ancestorNode function feels like a
 pre-opti