Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1f2A7e-0001Sw-M5 for pgadmin-hackers@arkaria.postgresql.org; Sat, 31 Mar 2018 06:36:19 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1f2A7d-0003dj-Cq for pgadmin-hackers@arkaria.postgresql.org; Sat, 31 Mar 2018 06:36:17 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1f2A7d-0003dZ-0r for pgadmin-hackers@lists.postgresql.org; Sat, 31 Mar 2018 06:36:17 +0000 Received: from mail-oi0-x234.google.com ([2607:f8b0:4003:c06::234]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1f2A7X-0004cn-Bh for pgadmin-hackers@postgresql.org; Sat, 31 Mar 2018 06:36:16 +0000 Received: by mail-oi0-x234.google.com with SMTP id u84-v6so9218009oie.10 for ; Fri, 30 Mar 2018 23:36:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=RrsMEiOgj5ZHDuss7O5q5JrZwkUzABNhI0ZqPUMu/so=; b=X8avdSSmz51PrNiMBqJ8sM2EcWLjionJcvT3C7H8sZO34yZ7XcBlkDW5YMiBuu+zXZ 7F3KF/HNNqj53DtgwH4qON01dk2p3d1sojeKAL0/r0JIWfYhzDZOrwEI1s8mRlmlkk74 LTAD1ToGu0VOEzhw1WdiaJz28ovUo1oWXLiHa3B7JA66YxIUrfqfqrsxGb4g1JIvh2b4 82YVEEbY117Eu9hFlGeuuZqQ/i6Gv5zCGbU8Y4YBmdYZzcD/MhhmY6C3Rq78J50J87wv eBygxIREepJguUanEPbbpQJeBB0xqZHr7pEWZWmxkqflEHjUkd6+y8nvA0WfrsIs8+pY ISdw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=RrsMEiOgj5ZHDuss7O5q5JrZwkUzABNhI0ZqPUMu/so=; b=hyrjJq7RoYL4w1ScnstpYoFwQq1AS8fGT0bDBbrh4JO+VpfQ92LFcoSSogKVCONOSq QHkcCjXEQ8dLr7mxeS4hVDfW6OLOIlMfYRbKZqM95sBmssq1duQGKNKfNkfzhonBR5CG W0nr3oNANc7kvf0zLctg9b5GKW1cbuB69Mvl9aAfmTpWKuYmFah7JLsLlq+ujJUnQeWC D6ggQiw3D2tgRMXUibdrxk5jn9VFkgFAHLvGhp4eXvGZxNGIhxvxwCBkONAdWODYn8hE Yrcaw1B0ZYmUJ1hRdcWbwX2o9kGUA6/Rh1eFk7uBfvKoqiq4dJAmw5+CwysTtYA0rG8h izRQ== X-Gm-Message-State: ALQs6tB/ob+z0pkD2kqOTSIq8b7eUyPTr36yMrOZEA/SHEiT9TNWtcso jpCmDg5C/WmjMgAVr5yXaV6dhWs67aJLjUMKHw2KBA== X-Google-Smtp-Source: AIpwx4+/G1MVDnZCoMf+uOUr1ZPzyCHEJ1q/O817tPaIFxK3VfwKVUcL5wZgrh5lA64j2BolFguRvPk1S/3Wu4n6lo8= X-Received: by 2002:aca:c60c:: with SMTP id w12-v6mr993587oif.192.1522478169059; Fri, 30 Mar 2018 23:36:09 -0700 (PDT) MIME-Version: 1.0 Received: by 10.138.6.138 with HTTP; Fri, 30 Mar 2018 23:35:48 -0700 (PDT) In-Reply-To: References: From: Murtuza Zabuawala Date: Sat, 31 Mar 2018 12:05:48 +0530 Message-ID: Subject: Re: [pgAdmin4][Patch][Feature #1998] Appends .sql if extension not given when using 'save' or 'save as' feature To: Rahul Soshte Cc: pgadmin-hackers Content-Type: multipart/alternative; boundary="000000000000720c2f0568af9218" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --000000000000720c2f0568af9218 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Then that's browser specific issue, please create one redmine ticket for the issue.=E2=80=8B On Fri, Mar 30, 2018 at 11:36 PM, Rahul Soshte wrote: > Yeah the code is present.I have attached the screenshot. > Also I have noticed that the format combobox appears clearly in my > Vivaldi Browser but it is not seen in my Firefox Browser. > > On Fri, Mar 30, 2018 at 9:59 PM, Murtuza Zabuawala enterprisedb.com> wrote: > >> =E2=80=8BI don't think so, Could you inspect html/css code on 'Save as' = dialog >> within your browser window and see if it's present or not?=E2=80=8B >> >> >> On Fri, Mar 30, 2018 at 8:30 PM, Rahul Soshte >> wrote: >> >>> Hi, >>> I don't know why that combobox is not seen in my environment.I am using >>> Ubuntu 17.10.I have attached the screenshot. >>> Is this a bug? >>> >>> >>> >>> On Fri, Mar 30, 2018 at 7:07 PM, Murtuza Zabuawala < >>> murtuza.zabuawala@enterprisedb.com> wrote: >>> >>>> ++ Attaching screenshot >>>> >>>> On Fri, Mar 30, 2018 at 7:06 PM, Murtuza Zabuawala < >>>> murtuza.zabuawala@enterprisedb.com> wrote: >>>> >>>>> Hi Rahul, >>>>> >>>>> When I said .sql extension, I meant selected sql option in 'Format' >>>>> combobox (check the screenshot I've attached) >>>>> >>>>> For the error you've mentioned you can create Fake application contex= t. >>>>> Ref: ../web/pgadmin/dashboard/tests/test_dashboard_templates.py +274 >>>>> >>>>> -- >>>>> Regards, >>>>> Murtuza Zabuawala >>>>> EnterpriseDB: http://www.enterprisedb.com >>>>> The Enterprise PostgreSQL Company >>>>> >>>>> >>>>> On Fri, Mar 30, 2018 at 6:36 PM, Rahul Soshte < >>>>> rahulsoshte360@gmail.com> wrote: >>>>> >>>>>> Hi, >>>>>> I tried writing tests in the web/pgadmin/tools/sqleditor/ut >>>>>> ils/tests/test_save_query_to_file_utils >>>>>> for the file web/pgadmin/tools/sqleditor/ut >>>>>> ils/tests/save_query_to_file_utils.py >>>>>> >>>>>> But I am getting a error, >>>>>> >>>>>> ERROR: runTest (pgadmin.tools.sqleditor.utils >>>>>> .tests.test_save_query_to_file_utils.TestSaveQueryToFile) >>>>>> When user has entered the extension .sql to the file while saving >>>>>> ------------------------------------------------------------ >>>>>> ---------- >>>>>> Traceback (most recent call last): >>>>>> File "/var/www/flask/pgadmin4/pgadmin4/web/pgadmin/tools/sqledito >>>>>> r/utils/tests/test_save_query_to_file_utils.py", line 42, in runTest >>>>>> file_path_result =3D save_query_to_file(self.file_data) >>>>>> File "/var/www/flask/pgadmin4/pgadmin4/web/pgadmin/tools/sqledito >>>>>> r/utils/save_query_to_file_utils.py", line 15, in save_query_to_file >>>>>> storage_manager_path =3D get_storage_directory() >>>>>> File "/var/www/flask/pgadmin4/local/lib/python2.7/site-packages/fl= ask_login.py", >>>>>> line 788, in decorated_view >>>>>> if current_app.login_manager._login_disabled: >>>>>> File "/var/www/flask/pgadmin4/local/lib/python2.7/site-packages/we= rkzeug/local.py", >>>>>> line 338, in __getattr__ >>>>>> return getattr(self._get_current_object(), name) >>>>>> File "/var/www/flask/pgadmin4/local/lib/python2.7/site-packages/we= rkzeug/local.py", >>>>>> line 297, in _get_current_object >>>>>> return self.__local() >>>>>> File "/var/www/flask/pgadmin4/local/lib/python2.7/site-packages/fl= ask/globals.py", >>>>>> line 51, in _find_app >>>>>> raise RuntimeError(_app_ctx_err_msg) >>>>>> RuntimeError: Working outside of application context. >>>>>> >>>>>> How do I test the extracted code inside context? How do I resolve >>>>>> this error. >>>>>> I have attached test_save_query_to_file_utils.py >>>>>> and save_query_to_file_utils.py >>>>>> >>>>>> Murtuza, Actually I didnt find any toggable button in the File Dialo= g >>>>>> Box So I made it general purpose ( I guess I will have to make one t= hen and >>>>>> then if I select SQL all .sql files should be listed, and if I selec= t All >>>>>> files then every type of file is shown in the File Dialog Box,this w= ill be >>>>>> a new feature, wouldnt it ? ) >>>>>> >>>>>> >>>>>> On Fri, Mar 30, 2018 at 4:10 PM, Murtuza Zabuawala < >>>>>> murtuza.zabuawala@enterprisedb.com> wrote: >>>>>> >>>>>>> >>>>>>> >>>>>>> On Thu, Mar 29, 2018 at 11:45 PM, Joao De Almeida Pereira < >>>>>>> jdealmeidapereira@pivotal.io> wrote: >>>>>>> >>>>>>>> Hi Rahul, >>>>>>>> I see you extracted some code, that is a pretty good move :D >>>>>>>> >>>>>>>> We run the patch through the testing pipeline and everything is >>>>>>>> green GJ :D >>>>>>>> Also tested the functionality by hand and looks like it is working >>>>>>>> except for "add the .sql extension when format is set to SQL." if >>>>>>>> you set it to All Files the extension is also added. Not sure if = this is a >>>>>>>> big deal or not. Lets see what other people think. >>>>>>>> >>>>>>> =E2=80=8BYes, I also think it should append .sql only if the sql ex= tension >>>>>>> is selected and user has not provided extension.=E2=80=8B >>>>>>> >>>>>>> =E2=80=8BLet say If I want to save the file with .txt extension the= n I can >>>>>>> use All Files. =E2=80=8B >>>>>>> >>>>>>> >>>>>>>> Codewise here are some of my comments: >>>>>>>> . You added the yarn-error.log file and a migration to the patch >>>>>>>> doesn't look intentional. Can you please remove them? >>>>>>>> . Also in the patch there are 2 file (moc_LogWindow.cpp and >>>>>>>> ui_LogWindow.h) that look like they do not belong to the patch (Di= d you >>>>>>>> rebase your branch before trying to create the patch? >>>>>>>> >>>>>>>> The test file: test_save_query_to_file.py is empty, it is missing >>>>>>>> some tests there. >>>>>>>> >>>>>>>> As a convention we user lower case names for functions and >>>>>>>> UpperCase for class >>>>>>>> >>>>>>>> Please, regenerate the patch following my previous comments. >>>>>>>> >>>>>>>> Thanks >>>>>>>> Joao >>>>>>>> >>>>>>>> On Thu, Mar 29, 2018 at 12:54 PM Rahul Soshte < >>>>>>>> rahulsoshte360@gmail.com> wrote: >>>>>>>> >>>>>>>>> Hi, >>>>>>>>> When using save or save as feature if .sql is not provided this >>>>>>>>> Patch appends it. >>>>>>>>> as clearly mentioned in this link. >>>>>>>>> >>>>>>>>> https://redmine.postgresql.org/issues/1998 >>>>>>>>> >>>>>>>>> I have ran pep8,regression and Jasmine tests too. >>>>>>>>> >>>>>>>>> I have primarily changed these files >>>>>>>>> web/pgadmin/tools/sqleditor/__init__.py >>>>>>>>> web/pgadmin/tools/sqleditor/static/js/sqleditor.js >>>>>>>>> web/pgadmin/tools/sqleditor/utils/save_query_to_file.py >>>>>>>>> >>>>>>>>> >>>>>>>>> Regards, >>>>>>>>> Rahul Soshte (Hunter) >>>>>>>>> >>>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> > --000000000000720c2f0568af9218 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable

Then that= 's browser specific issue, please create one redmine ticket for the issue.=E2=80= =8B

=

On Fri, Mar 30, 2018 at 11:36 PM, Rahul Sosh= te <rahulsoshte360@gmail.com> wrote:
Yeah the code is present.I have att= ached the screenshot.
Also=C2=A0 I have noticed that the format co= mbobox appears clearly in my Vivaldi Browser but it is not seen in my Firef= ox Browser.

On Fri, Mar 30, 2018 at 9:59 PM, Murtuza Zabuawala &l= t;m= urtuza.zabuawala@enterprisedb.com> wrote:
=E2=80=8BI don't think so, Could you inspec= t html/css code on 'Save as' dialog within your browser window and = see if it's present or not?=E2=80=8B

=

On Fri, Mar 30, 2018 at 8:30 PM, Rahul Sosht= e <rahulsoshte360@gmail.com> wrote:
Hi,
I don't = know why that combobox is not seen in my environment.I am using Ubuntu 17.1= 0.I have attached the screenshot.
Is this a bug?


=

On Fri, Mar= 30, 2018 at 7:07 PM, Murtuza Zabuawala <murtuza.zabuawal= a@enterprisedb.com> wrote:
++ Attaching screenshot

On Fri, Mar 30, 2018 at 7:06 PM, Murtuza Zab= uawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Rahul,

When I said = .sql extension, I meant selected sql option in 'Format' combobox (c= heck the screenshot I've attached)

For the error you've mentioned you = can create Fake application context.
Ref: ../web/pgadmin/dashboard/tests/test_dashboard_templates.p= y +274

=
= --Regards,=
Murtuza Zabuawala
EnterpriseDB:=C2=A0http://www.enterpris= edb.com
The Enterprise PostgreSQL Company
<= div><= img src=3D"https://drive.google.com/a/enterprisedb.com/uc?id=3D0B6jGeB3BfKR= MV0t4MEp0YnZCTTA&export=3Ddownload" width=3D"420" height=3D"31">

On Fri, Mar 30, 2018 at 6:36 PM, Rahul Sosht= e <rahulsoshte360@gmail.com> wrote:
Hi,
I tried writing tests in the= web/pgadmin/tools/sqleditor/utils/tests/test_save_query_to_file_= utils
for the file web/pgadmin/tools/sqleditor/utils/tes= ts/save_query_to_file_utils.py

But I am getting a er= ror,

ERROR: runTest (pgadmin.tools.sqleditor.utils.tests.test_s= ave_query_to_file_utils.TestSaveQueryToFile)
When user has entered = the extension .sql to the file while saving
----------------------------= ------------------------------------------
Traceback (most rec= ent call last):
=C2=A0 File "/var/www/flask/pgadmin4/pgadmin4/= web/pgadmin/tools/sqleditor/utils/tests/test_save_query_to_file_u= tils.py", line 42, in runTest
=C2=A0=C2=A0=C2=A0 file_path_result = =3D save_query_to_file(self.file_data)
=C2=A0 File "/var/www/f= lask/pgadmin4/pgadmin4/web/pgadmin/tools/sqleditor/utils/save_que= ry_to_file_utils.py", line 15, in save_query_to_file
=C2=A0=C2= =A0=C2=A0 storage_manager_path =3D get_storage_directory()
=C2=A0 File &= quot;/var/www/flask/pgadmin4/local/lib/python2.7/site-packages/fl= ask_login.py", line 788, in decorated_view
=C2=A0=C2=A0=C2=A0 if cu= rrent_app.login_manager._login_disabled:
=C2=A0 File "/var/www= /flask/pgadmin4/local/lib/python2.7/site-packages/werkzeug/local.= py", line 338, in __getattr__
=C2=A0=C2=A0=C2=A0 return getattr(sel= f._get_current_object(), name)
=C2=A0 File "/var/www/flask/pga= dmin4/local/lib/python2.7/site-packages/werkzeug/local.py", = line 297, in _get_current_object
=C2=A0=C2=A0=C2=A0 return self.__local(= )
=C2=A0 File "/var/www/flask/pgadmin4/local/lib/python2.7/sit= e-packages/flask/globals.py", line 51, in _find_app
=C2=A0=C2= =A0=C2=A0 raise RuntimeError(_app_ctx_err_msg)
RuntimeError: Working out= side of application context.

How do I test the extracted = code inside context? How do I resolve this error.
I have atta= ched test_save_query_to_file_utils.py
and save_query_to_= file_utils.py

Murtuza, Actually I didnt find a= ny toggable button in the File Dialog Box So I made it general purpose ( I = guess I will have to make one then and then if I select SQL all .sql files = should be listed, and if I select All files then every type of file is show= n in the File Dialog Box,this will be a new feature, wouldnt it ?=C2=A0 )


On Fri, Mar 30, 2018 at 4:10 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:


On Thu, Mar 29, 2018 at 11:45 PM, Joao= De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi Rahul,
I= see you extracted some code, that is a pretty good move :D

<= /div>
We run the patch through the testing pipeline and everything is g= reen GJ :D
Also tested the functionality by hand and looks like i= t is working except for "add = the .sql extension when format is set to SQL." if you set it to= All Files=C2=A0 the extension is also added. Not sure if this is a big dea= l or not. Lets see what other people think.
=
=E2=80=8BYes, I also think it should append .sql only if the sql extension is sele= cted and user has not provided extension.=E2=80=8B
=C2=A0
=E2=80=8BLe= t say If I want to save the file with .txt extension then I can use All Files. =E2=80=8B

<= /div>
Codewise here are some of my comments:
. You added the = yarn-error.log file and a migration to the patch doesn't look intention= al. Can you please remove them?
. Also in the patch there are 2 f= ile (moc_LogWindow.cpp and ui_LogWindow.h) that look like they do not belon= g to the patch (Did you rebase your branch before trying to create the patc= h?

The test file:=C2=A0test_save_query_to_file.py is empty, it is missing some tests there.

As = a convention we user lower case names for functions and UpperCase for class=

Please, regenerate the patch following my previou= s comments.

Thanks
Joao

On Thu, Mar 29, 2018 at 12:54 PM Rahul= Soshte <r= ahulsoshte360@gmail.com> wrote:
Hi,
When using save or sav= e as feature if .sql is not provided this Patch appends it.
as clearly m= entioned in this link.

https://redmine.postgresql.org/issues/1998=

I have ran pep8,regression and Jasmine tests too.

=
I have primarily changed these files
=C2=A0=C2=A0=C2=A0=C2=A0 web/= pgadmin/tools/sqleditor/__init__.py
=C2=A0=C2=A0=C2=A0=C2=A0 web/pg= admin/tools/sqleditor/static/js/sqleditor.js
=C2=A0=C2=A0=C2=A0=C2= =A0 web/pgadmin/tools/sqleditor/utils/save_query_to_file.py


Regards,
Rahul Soshte (Hunter)









--000000000000720c2f0568af9218--