Re: [pgAdmin4][Patch] - RM 5422 - Synonym Dependencies tab shows incorrect data
Hi Navnath, On Wed, Apr 29, 2020 at 12:16 PM navnath gadakh < navnath.gad...@enterprisedb.com> wrote: > Hi Khushboo, > > Patch looks good to me except there are no API test cases written for > dependents and dependencies. > > Thanks for the review. We don't have API test cases for dependents and dependencies for none of the objects. If the team agrees, I will create a separate RM for that. Thanks, Khushboo > Thanks! > > On Wed, Apr 29, 2020 at 10:40 AM navnath gadakh < > navnath.gad...@enterprisedb.com> wrote: > >> Hi , >> >> I'm reviewing this task. >> >> On Wed, Apr 29, 2020 at 9:36 AM Khushboo Vashi < >> khushboo.va...@enterprisedb.com> wrote: >> >>> Hi, >>> >>> Please find the attached patch to fix the RM 5422 - Synonym Dependencies >>> tab shows incorrect data. >>> >>> Thanks, >>> Khushboo >>> >> >> >> -- >> Regards, >> Navnath Gadakh >> > > > -- > Regards, > Navnath Gadakh >
[pgAdmin4] RESQL tests for Column & Type nodes
Hi, PFA patch to add RESQL tests for Column and Type nodes. -- Regards, Murtuza Zabuawala EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company msql_tests_columns_types.diff Description: Binary data
Re: [pgAdmin4] RESQL tests for Column & Type nodes
I will review and test it. On Wed, Apr 29, 2020 at 2:27 PM Murtuza Zabuawala < murtuza.zabuaw...@enterprisedb.com> wrote: > Hi, > > PFA patch to add RESQL tests for Column and Type nodes. > > > -- > Regards, > Murtuza Zabuawala > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > > -- Regards, Navnath Gadakh
Re: Raise an exception under Python < 3.4
Hi Dave, Patch looks good to me except one condition as we do support from Python 3.4 onwards. I have modified that condition and attached the updated patch. I have also removed the other references of Python2. Attached is the patch. Do review it and let me know for comments. Thanks, Neel Patel On Mon, Apr 27, 2020 at 9:27 PM Dave Page wrote: > The attached patch raises an exception if run under Python < 3.4. It also > cleans up the README to remove references to Python 2 and removes Python > 2-isms from the main config. > > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > Throw_an_exception_if_run_under_Python__=_3_3,_and_update_the_docs_and_main_config_for_Pyt_v2.patch Description: Binary data Remove_Python2_Reference_Core_files.patch Description: Binary data
Re: Raise an exception under Python < 3.4
Hi On Wed, Apr 29, 2020 at 10:13 AM Neel Patel wrote: > Hi Dave, > > Patch looks good to me except one condition as we do support from Python > 3.4 onwards. I have modified that condition and attached the updated patch. > What did you change exactly (curious about what I missed)? > > I have also removed the other references of Python2. Attached is the > patch. Do review it and let me know for comments. > I think there's a lot more than that - try searching the entire tree for PY2 or sys.version. This was on my personal todo as a next step, but feel free to update your patch :-) Thanks! > > Thanks, > Neel Patel > > On Mon, Apr 27, 2020 at 9:27 PM Dave Page wrote: > >> The attached patch raises an exception if run under Python < 3.4. It also >> cleans up the README to remove references to Python 2 and removes Python >> 2-isms from the main config. >> >> -- >> 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
Re: Raise an exception under Python < 3.4
Hi,
On Wed, Apr 29, 2020 at 2:52 PM Dave Page wrote:
> Hi
>
> On Wed, Apr 29, 2020 at 10:13 AM Neel Patel
> wrote:
>
>> Hi Dave,
>>
>> Patch looks good to me except one condition as we do support from Python
>> 3.4 onwards. I have modified that condition and attached the updated patch.
>>
>
> What did you change exactly (curious about what I missed)?
>
Previously it was like,
*+if sys.version_info < (3, 3):*
*+raise Exception('This application must be run under Python 3.4 or
later.')*
Modified as below.
*+if sys.version_info < (3, 4):*
*+raise Exception('This application must be run under Python 3.4 or
later.')*
>
>>
>> I have also removed the other references of Python2. Attached is the
>> patch. Do review it and let me know for comments.
>>
>
> I think there's a lot more than that - try searching the entire tree for
> PY2 or sys.version. This was on my personal todo as a next step, but feel
> free to update your patch :-)
>
Sure.
>
> Thanks!
>
>
>>
>> Thanks,
>> Neel Patel
>>
>> On Mon, Apr 27, 2020 at 9:27 PM Dave Page wrote:
>>
>>> The attached patch raises an exception if run under Python < 3.4. It
>>> also cleans up the README to remove references to Python 2 and removes
>>> Python 2-isms from the main config.
>>>
>>> --
>>> 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
>
Re: [pgAdmin][RM4279] : Issue with File Browser Home button
Hi all, Here is an updated patch as per the review comments mentioned above. Please review the updated patch. PFA. RM4279v4.patch Description: Binary data
pgAdmin 4 commit: Fix the runtime build with Python 3.8 on Linux
Fix the runtime build with Python 3.8 on Linux Branch -- master Details --- https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=3138f36e05ba843d1c386fc59d597049fb82446d Modified Files -- runtime/pgAdmin4.pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
pgAdmin 4 commit: Sigh. Remove extraneous closing bracket. It's always
Sigh. Remove extraneous closing bracket. It's always a bracket. Branch -- master Details --- https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=f7ef86dcedaf3630002b54aaeacf0da86bec71de Modified Files -- runtime/pgAdmin4.pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
pgAdmin 4 commit: OK, another attempt at this as it fails on non-Linux
OK, another attempt at this as it fails on non-Linux non-Python 3.8. Branch -- master Details --- https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=505956d1faffe2ed1f5d88169eade47c86be5eb7 Modified Files -- runtime/pgAdmin4.pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
RM3694-If database is already connected and click on database then connect database should not displayed in Menu
Hi Hackers,
In the patch attached, we are gracefully informing the end user, using an
alert message, that the database is already connected when they click
"Connect Database..." after right clicking on a disconnected database.
As this problem deals with racing conditions, it is highly complex to show
the "Disconnect database" option in the menu upon right click. So we are
alerting the end user with the information of "Database already connected".
Thanks,
Sathish V
diff --git a/web/pgadmin/browser/server_groups/servers/databases/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/__init__.py
index 9216f21..a931371 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/__init__.py
@@ -456,6 +456,7 @@ class DatabaseView(PGChildNodeView):
from pgadmin.utils.driver import get_driver
manager = get_driver(PG_DEFAULT_DRIVER).connection_manager(sid)
conn = manager.connection(did=did, auto_reconnect=True)
+info_already_connected = conn.connected()
status, errmsg = conn.connect()
if not status:
@@ -469,13 +470,14 @@ class DatabaseView(PGChildNodeView):
else:
current_app.logger.info('Connection Established for Database Id: \
%s' % (did))
-
return make_json_response(
success=1,
-info=_("Database connected."),
+info=_("Database already connected") if info_already_connected
+else _("Database connected"),
data={
'icon': 'pg-icon-database',
'connected': True,
+'info_already_connected': info_already_connected,
'info_prefix': '{0}/{1}'.
format(Server.query.filter_by(id=sid)[0].name, conn.db)
}
diff --git a/web/pgadmin/browser/server_groups/servers/databases/static/js/database.js b/web/pgadmin/browser/server_groups/servers/databases/static/js/database.js
index f458d69..24e7205 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/static/js/database.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/static/js/database.js
@@ -534,7 +534,13 @@ define('pgadmin.node.database', [
res.info = `${_.escape(res.data.info_prefix)} - ${res.info}`;
}
- Alertify.success(res.info);
+ if(res.data.info_already_connected){
+Alertify.error(res.info);
+ } else {
+Alertify.success(res.info);
+ }
+
+
obj.trigger('connected', obj, item, data);
pgBrowser.Events.trigger(
'pgadmin:database:connected', item, data
pgAdmin 4 commit: Don't rely on python-config sending the help message
Don't rely on python-config sending the help message to stderr, as older versions seem to use stdout. Branch -- master Details --- https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=97be3d8179637d946a93d7bb778a6fd6e7e69a8d Modified Files -- runtime/pgAdmin4.pro | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-)
pgAdmin 4 commit: Another attempt at fixing the 'do we need the --embed
Another attempt at fixing the 'do we need the --embed flag for Python' problem. This change looks at the output from python-config --help (which may go to stderr or stdout), and if it includes --embed, adds it to the later call to get the libs string. Branch -- master Details --- https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=e8a4b46ba15cb7dd0736081347e7b9ff86cb89f3 Modified Files -- runtime/pgAdmin4.pro | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-)
Re: RM3694-If database is already connected and click on database then connect database should not displayed in Menu
Hi Satish,
As per the RM, the fix is supposed to be at the front-end but it seems
difficult at the moment as on the selection of the database, we connect it
and at the same time the context menu is being called.
As you have tried to fix at the backend, some of the review comments are
below.
1. If the database is already connected, no need to call conn.connect
again.
info_already_connected = conn.connected()
status, errmsg = conn.connect()
2. If you want to raise an error at the client side, use the
appropriate function or status (check file ajax.py) at the server side
and send the appropriate response.
Below code needs to be changed.
if(res.data.info_already_connected){
Alertify.error(res.info);
} else {
Alertify.success(res.info);
}
Thanks,
Khushboo
On Wed, Apr 29, 2020 at 8:20 PM Satish V wrote:
> Hi Hackers,
>
> In the patch attached, we are gracefully informing the end user, using an
> alert message, that the database is already connected when they click
> "Connect Database..." after right clicking on a disconnected database.
>
> As this problem deals with racing conditions, it is highly complex to show
> the "Disconnect database" option in the menu upon right click. So we are
> alerting the end user with the information of "Database already connected".
>
> Thanks,
> Sathish V
>
Re: RM3694-If database is already connected and click on database then connect database should not displayed in Menu
Hi Kushboo,
Thanks for the update. I will check the same and make appropriate changes.
Thanks,
Sathish
On Thu, Apr 30, 2020 at 9:20 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:
> Hi Satish,
>
> As per the RM, the fix is supposed to be at the front-end but it seems
> difficult at the moment as on the selection of the database, we connect it
> and at the same time the context menu is being called.
> As you have tried to fix at the backend, some of the review comments are
> below.
>
> 1. If the database is already connected, no need to call conn.connect
> again.
>
>
> info_already_connected = conn.connected()
>
> status, errmsg = conn.connect()
>
> 2. If you want to raise an error at the client side, use the appropriate
> function or status (check file ajax.py) at the server side and send the
> appropriate response.
>
> Below code needs to be changed.
>
> if(res.data.info_already_connected){
>
> Alertify.error(res.info);
>
> } else {
>
> Alertify.success(res.info);
>
> }
>
> Thanks,
>
> Khushboo
>
>
>
> On Wed, Apr 29, 2020 at 8:20 PM Satish V
> wrote:
>
>> Hi Hackers,
>>
>> In the patch attached, we are gracefully informing the end user, using an
>> alert message, that the database is already connected when they click
>> "Connect Database..." after right clicking on a disconnected database.
>>
>> As this problem deals with racing conditions, it is highly complex to
>> show the "Disconnect database" option in the menu upon right click. So we
>> are alerting the end user with the information of "Database already
>> connected".
>>
>> Thanks,
>> Sathish V
>>
>
Re: RM3694-If database is already connected and click on database then connect database should not displayed in Menu
Hi,
On Thu, Apr 30, 2020 at 9:41 AM Satish V wrote:
> Hi Kushboo,
>
> Thanks for the update. I will check the same and make appropriate changes.
>
> Thanks,
> Sathish
>
> On Thu, Apr 30, 2020 at 9:20 AM Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>> Hi Satish,
>>
>> As per the RM, the fix is supposed to be at the front-end but it seems
>> difficult at the moment as on the selection of the database, we connect it
>> and at the same time the context menu is being called.
>> As you have tried to fix at the backend, some of the review comments are
>> below.
>>
>> 1. If the database is already connected, no need to call conn.connect
>> again.
>>
>>
>> info_already_connected = conn.connected()
>>
>> status, errmsg = conn.connect()
>>
>> I've noticed conn.connected() is misleading sometimes. Let's say if the
PG server is stopped and no query is fired from pgadmin after that,
then conn.connected()
will still give True. It is updated only when a query is fired to the PG
server. I would suggest let it connect again as there is no harm and this
function is very important. We don't want to mess it up for the sake of a
message.
> 2. If you want to raise an error at the client side, use the appropriate
> function or status (check file ajax.py) at the server side and send the
> appropriate response.
>>
>> Below code needs to be changed.
>>
>> if(res.data.info_already_connected){
>>
>> Alertify.error(res.info);
>>
>> } else {
>>
>> Alertify.success(res.info);
>>
>> }
>>
>> The problem is test cases. Almost all the test cases have checked for
response "Database connected.". If the function is changed at the server
side then all the test cases (around 85+) would have to be changed to
handle this.
> Thanks,
>>
>> Khushboo
>>
>>
>>
>> On Wed, Apr 29, 2020 at 8:20 PM Satish V
>> wrote:
>>
>>> Hi Hackers,
>>>
>>> In the patch attached, we are gracefully informing the end user, using
>>> an alert message, that the database is already connected when they click
>>> "Connect Database..." after right clicking on a disconnected database.
>>>
>>> As this problem deals with racing conditions, it is highly complex to
>>> show the "Disconnect database" option in the menu upon right click. So we
>>> are alerting the end user with the information of "Database already
>>> connected".
>>>
>>> Thanks,
>>> Sathish V
>>>
>>
--
Thanks and Regards,
Aditya Toshniwal
pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"
Re: RM3694-If database is already connected and click on database then connect database should not displayed in Menu
On Thu, Apr 30, 2020 at 10:14 AM Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:
> Hi,
>
>
> On Thu, Apr 30, 2020 at 9:41 AM Satish V
> wrote:
>
>> Hi Kushboo,
>>
>> Thanks for the update. I will check the same and make appropriate changes.
>>
>> Thanks,
>> Sathish
>>
>> On Thu, Apr 30, 2020 at 9:20 AM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi Satish,
>>>
>>> As per the RM, the fix is supposed to be at the front-end but it seems
>>> difficult at the moment as on the selection of the database, we connect it
>>> and at the same time the context menu is being called.
>>> As you have tried to fix at the backend, some of the review comments are
>>> below.
>>>
>>> 1. If the database is already connected, no need to call conn.connect
>>> again.
>>>
>>>
>>> info_already_connected = conn.connected()
>>>
>>> status, errmsg = conn.connect()
>>>
>>> I've noticed conn.connected() is misleading sometimes. Let's say if the
> PG server is stopped and no query is fired from pgadmin after that, then
> conn.connected()
> will still give True. It is updated only when a query is fired to the PG
> server. I would suggest let it connect again as there is no harm and this
> function is very important. We don't want to mess it up for the sake of a
> message.
>
It's true that conn.connected() is misleading but we already get the
connection before checking conn.connected() with conn = manager.connection(
did=did, auto_reconnect=True).
So, if the database server is stopped, it will throw the error.
> 2. If you want to raise an error at the client side, use the appropriate
> function or status (check file ajax.py) at the server side and send the
> appropriate response.
>>>
>>> Below code needs to be changed.
>>>
>>> if(res.data.info_already_connected){
>>>
>>> Alertify.error(res.info);
>>>
>>> } else {
>>>
>>> Alertify.success(res.info);
>>>
>>> }
>>>
>>> The problem is test cases. Almost all the test cases have checked for
> response "Database connected.". If the function is changed at the server
> side then all the test cases (around 85+) would have to be changed to
> handle this.
>
It's a valid point, so I would suggest to show the warning/info at the
client side, no need to show the error message.
> Thanks,
>>>
>>> Khushboo
>>>
>>>
>>>
>>> On Wed, Apr 29, 2020 at 8:20 PM Satish V
>>> wrote:
>>>
Hi Hackers,
In the patch attached, we are gracefully informing the end user, using
an alert message, that the database is already connected when they click
"Connect Database..." after right clicking on a disconnected database.
As this problem deals with racing conditions, it is highly complex to
show the "Disconnect database" option in the menu upon right click. So we
are alerting the end user with the information of "Database already
connected".
Thanks,
Sathish V
>>>
>
> --
> Thanks and Regards,
> Aditya Toshniwal
> pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
> "Don't Complain about Heat, Plant a TREE"
>
Re: RM3694-If database is already connected and click on database then connect database should not displayed in Menu
Hi,
On Thu, Apr 30, 2020 at 10:48 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:
>
>
> On Thu, Apr 30, 2020 at 10:14 AM Aditya Toshniwal <
> aditya.toshni...@enterprisedb.com> wrote:
>
>> Hi,
>>
>>
>> On Thu, Apr 30, 2020 at 9:41 AM Satish V
>> wrote:
>>
>>> Hi Kushboo,
>>>
>>> Thanks for the update. I will check the same and make
>>> appropriate changes.
>>>
>>> Thanks,
>>> Sathish
>>>
>>> On Thu, Apr 30, 2020 at 9:20 AM Khushboo Vashi <
>>> khushboo.va...@enterprisedb.com> wrote:
>>>
Hi Satish,
As per the RM, the fix is supposed to be at the front-end but it seems
difficult at the moment as on the selection of the database, we connect it
and at the same time the context menu is being called.
As you have tried to fix at the backend, some of the review comments
are below.
1. If the database is already connected, no need to call conn.connect
again.
info_already_connected = conn.connected()
status, errmsg = conn.connect()
I've noticed conn.connected() is misleading sometimes. Let's say if the
>> PG server is stopped and no query is fired from pgadmin after that, then
>> conn.connected()
>> will still give True. It is updated only when a query is fired to the PG
>> server. I would suggest let it connect again as there is no harm and this
>> function is very important. We don't want to mess it up for the sake of a
>> message.
>>
> It's true that conn.connected() is misleading but we already get the
> connection before checking conn.connected() with conn =
> manager.connection(did=did, auto_reconnect=True).
> So, if the database server is stopped, it will throw the error.
>
I just checked the manager.connection function, and it is not connecting or
checking. It will just give the connection object stored in the memory.
> 2. If you want to raise an error at the client side, use the appropriate
> function or status (check file ajax.py) at the server side and send the
> appropriate response.
Below code needs to be changed.
if(res.data.info_already_connected){
Alertify.error(res.info);
} else {
Alertify.success(res.info);
}
The problem is test cases. Almost all the test cases have checked for
>> response "Database connected.". If the function is changed at the server
>> side then all the test cases (around 85+) would have to be changed to
>> handle this.
>>
> It's a valid point, so I would suggest to show the warning/info at the
> client side, no need to show the error message.
>
Yes, info is a good option.
> Thanks,
Khushboo
On Wed, Apr 29, 2020 at 8:20 PM Satish V
wrote:
> Hi Hackers,
>
> In the patch attached, we are gracefully informing the end user, using
> an alert message, that the database is already connected when they click
> "Connect Database..." after right clicking on a disconnected database.
>
> As this problem deals with racing conditions, it is highly complex to
> show the "Disconnect database" option in the menu upon right click. So we
> are alerting the end user with the information of "Database already
> connected".
>
> Thanks,
> Sathish V
>
>>
>> --
>> Thanks and Regards,
>> Aditya Toshniwal
>> pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
>> "Don't Complain about Heat, Plant a TREE"
>>
>
--
Thanks and Regards,
Aditya Toshniwal
pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"
Re: [pgAdmin4] RESQL tests for Column & Type nodes
Hi Murtuza, Patch looks good to me. Thanks! On Wed, Apr 29, 2020 at 2:38 PM navnath gadakh < navnath.gad...@enterprisedb.com> wrote: > I will review and test it. > > On Wed, Apr 29, 2020 at 2:27 PM Murtuza Zabuawala < > murtuza.zabuaw...@enterprisedb.com> wrote: > >> Hi, >> >> PFA patch to add RESQL tests for Column and Type nodes. >> >> >> -- >> Regards, >> Murtuza Zabuawala >> EnterpriseDB: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company >> >> > > -- > Regards, > Navnath Gadakh > -- Regards, Navnath Gadakh
Re: RM3694-If database is already connected and click on database then connect database should not displayed in Menu
On Thu, Apr 30, 2020 at 10:55 AM Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:
> Hi,
>
> On Thu, Apr 30, 2020 at 10:48 AM Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>>
>>
>> On Thu, Apr 30, 2020 at 10:14 AM Aditya Toshniwal <
>> aditya.toshni...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>>
>>> On Thu, Apr 30, 2020 at 9:41 AM Satish V
>>> wrote:
>>>
Hi Kushboo,
Thanks for the update. I will check the same and make
appropriate changes.
Thanks,
Sathish
On Thu, Apr 30, 2020 at 9:20 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:
> Hi Satish,
>
> As per the RM, the fix is supposed to be at the front-end but it seems
> difficult at the moment as on the selection of the database, we connect it
> and at the same time the context menu is being called.
> As you have tried to fix at the backend, some of the review comments
> are below.
>
> 1. If the database is already connected, no need to call conn.connect
> again.
>
>
> info_already_connected = conn.connected()
>
> status, errmsg = conn.connect()
>
> I've noticed conn.connected() is misleading sometimes. Let's say if
>>> the PG server is stopped and no query is fired from pgadmin after that,
>>> then conn.connected() will still give True. It is updated only when a
>>> query is fired to the PG server. I would suggest let it connect again as
>>> there is no harm and this function is very important. We don't want to mess
>>> it up for the sake of a message.
>>>
>> It's true that conn.connected() is misleading but we already get the
>> connection before checking conn.connected() with conn =
>> manager.connection(did=did, auto_reconnect=True).
>> So, if the database server is stopped, it will throw the error.
>>
> I just checked the manager.connection function, and it is not connecting
> or checking. It will just give the connection object stored in the memory.
>
:).
databases/__init__.py : def connect()
manager = get_driver(PG_DEFAULT_DRIVER).connection_manager(sid)
conn = manager.connection(did=did, auto_reconnect=True)
Above 2 lines will handle which I just just mentioned above.
2. If you want to raise an error at the client side, use the
appropriate function or status (check file ajax.py) at the server side
and send the appropriate response.
>
> Below code needs to be changed.
>
> if(res.data.info_already_connected){
>
> Alertify.error(res.info);
>
> } else {
>
> Alertify.success(res.info);
>
> }
>
> The problem is test cases. Almost all the test cases have checked for
>>> response "Database connected.". If the function is changed at the server
>>> side then all the test cases (around 85+) would have to be changed to
>>> handle this.
>>>
>> It's a valid point, so I would suggest to show the warning/info at the
>> client side, no need to show the error message.
>>
> Yes, info is a good option.
>
>> Thanks,
>
> Khushboo
>
>
>
> On Wed, Apr 29, 2020 at 8:20 PM Satish V
> wrote:
>
>> Hi Hackers,
>>
>> In the patch attached, we are gracefully informing the end user,
>> using an alert message, that the database is already connected when they
>> click "Connect Database..." after right clicking on a disconnected
>> database.
>>
>> As this problem deals with racing conditions, it is highly complex to
>> show the "Disconnect database" option in the menu upon right click. So we
>> are alerting the end user with the information of "Database already
>> connected".
>>
>> Thanks,
>> Sathish V
>>
>
>>>
>>> --
>>> Thanks and Regards,
>>> Aditya Toshniwal
>>> pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
>>> "Don't Complain about Heat, Plant a TREE"
>>>
>>
>
> --
> Thanks and Regards,
> Aditya Toshniwal
> pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
> "Don't Complain about Heat, Plant a TREE"
>
Re: RM3694-If database is already connected and click on database then connect database should not displayed in Menu
Hi,
On Thu, Apr 30, 2020 at 11:55 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:
>
>
> On Thu, Apr 30, 2020 at 10:55 AM Aditya Toshniwal <
> aditya.toshni...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> On Thu, Apr 30, 2020 at 10:48 AM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>>
>>>
>>> On Thu, Apr 30, 2020 at 10:14 AM Aditya Toshniwal <
>>> aditya.toshni...@enterprisedb.com> wrote:
>>>
Hi,
On Thu, Apr 30, 2020 at 9:41 AM Satish V
wrote:
> Hi Kushboo,
>
> Thanks for the update. I will check the same and make
> appropriate changes.
>
> Thanks,
> Sathish
>
> On Thu, Apr 30, 2020 at 9:20 AM Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>> Hi Satish,
>>
>> As per the RM, the fix is supposed to be at the front-end but it
>> seems difficult at the moment as on the selection of the database, we
>> connect it and at the same time the context menu is being called.
>> As you have tried to fix at the backend, some of the review comments
>> are below.
>>
>> 1. If the database is already connected, no need to call
>> conn.connect again.
>>
>>
>> info_already_connected = conn.connected()
>>
>> status, errmsg = conn.connect()
>>
>> I've noticed conn.connected() is misleading sometimes. Let's say if
the PG server is stopped and no query is fired from pgadmin after that,
then conn.connected() will still give True. It is updated only when a
query is fired to the PG server. I would suggest let it connect again as
there is no harm and this function is very important. We don't want to mess
it up for the sake of a message.
>>> It's true that conn.connected() is misleading but we already get the
>>> connection before checking conn.connected() with conn =
>>> manager.connection(did=did, auto_reconnect=True).
>>> So, if the database server is stopped, it will throw the error.
>>>
>> I just checked the manager.connection function, and it is not connecting
>> or checking. It will just give the connection object stored in the memory.
>>
> :).
>
>
> databases/__init__.py : def connect()
>
>
> manager = get_driver(PG_DEFAULT_DRIVER).connection_manager(sid)
>
> conn = manager.connection(did=did, auto_reconnect=True)
>
>
>
> Above 2 lines will handle which I just just mentioned above.
>
I didn't find any code which will call the connect, apart
from auto_reconnect being used when we execute some query. Nevertheless, I
still believe we should not remove the connect call for the sake of a
message, for which I think the user won't even bother.
>
> 2. If you want to raise an error at the client side, use the appropriate
> function or status (check file ajax.py) at the server side and send the
> appropriate response.
>>
>> Below code needs to be changed.
>>
>> if(res.data.info_already_connected){
>>
>> Alertify.error(res.info);
>>
>> } else {
>>
>> Alertify.success(res.info);
>>
>> }
>>
>> The problem is test cases. Almost all the test cases have checked for
response "Database connected.". If the function is changed at the server
side then all the test cases (around 85+) would have to be changed to
handle this.
>>> It's a valid point, so I would suggest to show the warning/info at the
>>> client side, no need to show the error message.
>>>
>> Yes, info is a good option.
>>
>>> Thanks,
>>
>> Khushboo
>>
>>
>>
>> On Wed, Apr 29, 2020 at 8:20 PM Satish V
>> wrote:
>>
>>> Hi Hackers,
>>>
>>> In the patch attached, we are gracefully informing the end user,
>>> using an alert message, that the database is already connected when they
>>> click "Connect Database..." after right clicking on a disconnected
>>> database.
>>>
>>> As this problem deals with racing conditions, it is highly complex
>>> to show the "Disconnect database" option in the menu upon right click.
>>> So
>>> we are alerting the end user with the information of "Database already
>>> connected".
>>>
>>> Thanks,
>>> Sathish V
>>>
>>
--
Thanks and Regards,
Aditya Toshniwal
pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"
>>>
>>
>> --
>> Thanks and Regards,
>> Aditya Toshniwal
>> pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
>> "Don't Complain about Heat, Plant a TREE"
>>
>
--
Thanks and Regards,
Aditya Toshniwal
pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"
Re: RM3694-If database is already connected and click on database then connect database should not displayed in Menu
On Thu, Apr 30, 2020 at 12:04 PM Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:
> Hi,
>
> On Thu, Apr 30, 2020 at 11:55 AM Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>>
>>
>> On Thu, Apr 30, 2020 at 10:55 AM Aditya Toshniwal <
>> aditya.toshni...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> On Thu, Apr 30, 2020 at 10:48 AM Khushboo Vashi <
>>> khushboo.va...@enterprisedb.com> wrote:
>>>
On Thu, Apr 30, 2020 at 10:14 AM Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:
> Hi,
>
>
> On Thu, Apr 30, 2020 at 9:41 AM Satish V
> wrote:
>
>> Hi Kushboo,
>>
>> Thanks for the update. I will check the same and make
>> appropriate changes.
>>
>> Thanks,
>> Sathish
>>
>> On Thu, Apr 30, 2020 at 9:20 AM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi Satish,
>>>
>>> As per the RM, the fix is supposed to be at the front-end but it
>>> seems difficult at the moment as on the selection of the database, we
>>> connect it and at the same time the context menu is being called.
>>> As you have tried to fix at the backend, some of the review comments
>>> are below.
>>>
>>> 1. If the database is already connected, no need to call
>>> conn.connect again.
>>>
>>>
>>> info_already_connected = conn.connected()
>>>
>>> status, errmsg = conn.connect()
>>>
>>> I've noticed conn.connected() is misleading sometimes. Let's say if
> the PG server is stopped and no query is fired from pgadmin after that,
> then conn.connected() will still give True. It is updated only when a
> query is fired to the PG server. I would suggest let it connect again as
> there is no harm and this function is very important. We don't want to
> mess
> it up for the sake of a message.
>
It's true that conn.connected() is misleading but we already get the
connection before checking conn.connected() with conn =
manager.connection(did=did, auto_reconnect=True).
So, if the database server is stopped, it will throw the error.
>>> I just checked the manager.connection function, and it is not
>>> connecting or checking. It will just give the connection object stored in
>>> the memory.
>>>
>> :).
>>
>>
>> databases/__init__.py : def connect()
>>
>>
>> manager = get_driver(PG_DEFAULT_DRIVER).connection_manager(sid)
>>
>> conn = manager.connection(did=did, auto_reconnect=True)
>>
>>
>>
>> Above 2 lines will handle which I just just mentioned above.
>>
> I didn't find any code which will call the connect, apart
> from auto_reconnect being used when we execute some query. Nevertheless, I
> still believe we should not remove the connect call for the sake of a
> message, for which I think the user won't even bother.
>
>>
>>
> To understand the code properly, comment out the line conn.connect() from
the def connect(), restart the pgadmin server and try to connect the
database from the client side.
So, you get to know that there is no harm in my suggestion.
> 2. If you want to raise an error at the client side, use the appropriate
> function or status (check file ajax.py) at the server side and send the
> appropriate response.
>>>
>>> Below code needs to be changed.
>>>
>>> if(res.data.info_already_connected){
>>>
>>> Alertify.error(res.info);
>>>
>>> } else {
>>>
>>> Alertify.success(res.info);
>>>
>>> }
>>>
>>> The problem is test cases. Almost all the test cases have checked
> for response "Database connected.". If the function is changed at the
> server side then all the test cases (around 85+) would have to be changed
> to handle this.
>
It's a valid point, so I would suggest to show the warning/info at the
client side, no need to show the error message.
>>> Yes, info is a good option.
>>>
Thanks,
>>>
>>> Khushboo
>>>
>>>
>>>
>>> On Wed, Apr 29, 2020 at 8:20 PM Satish V
>>> wrote:
>>>
Hi Hackers,
In the patch attached, we are gracefully informing the end user,
using an alert message, that the database is already connected when
they
click "Connect Database..." after right clicking on a disconnected
database.
As this problem deals with racing conditions, it is highly complex
to show the "Disconnect database" option in the menu upon right click.
So
we are alerting the end user with the information of "Database already
connected".
Thanks,
Sathish V
>>>
>
> --
> Thanks and Regards,
> Aditya Toshniwal
> pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
> "Don't Complain about Heat, Plant a TREE"
>
>>>
>>> --
>>> Thanks and Regards,
>>> Aditya Toshniwal
>>> pgAdmin Hacke
