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 1f1uDL-00060L-TM for pgadmin-hackers@arkaria.postgresql.org; Fri, 30 Mar 2018 13:37:08 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1f1uDK-0005Gx-Aw for pgadmin-hackers@arkaria.postgresql.org; Fri, 30 Mar 2018 13:37:06 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1f1uDJ-0005Gm-QF for pgadmin-hackers@lists.postgresql.org; Fri, 30 Mar 2018 13:37:06 +0000 Received: from mail-ot0-x22b.google.com ([2607:f8b0:4003:c0f::22b]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1f1uDG-00057g-67 for pgadmin-hackers@postgresql.org; Fri, 30 Mar 2018 13:37:04 +0000 Received: by mail-ot0-x22b.google.com with SMTP id 23-v6so9588021otj.0 for ; Fri, 30 Mar 2018 06:37:01 -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=T7liRnhSQZFeFeAxc6DzSWWVxHVukR76VssUutmbOek=; b=W0APRbUWtYEpVDH5pxZvQdt9ERyWLHDqp5KT+espZPp7TtoigEGpuY3c/5s5lHcd4l kGJExcxUFvYxunouAkPlG/ZOHUTX8nlMyhOWtMfjCuvwodk6C2h9cuas4lv1cMRxBU97 yMoAVf7RjqqG6qHuMaDWyN8Tfism8JhRX+QbGR0qhzR+kurhgODbqMnqexaJoELgyPhR ej6gNxR118smb/jEAtHTfCSCJXUFCf2x3AbZ/wKCLpVpOBH8drw55oOfm1oSd4Iu6H06 YWh2PMQVBNY9k7JftS3hJGozybrLepf+1huQmAxMLvNAHl4H6/KKMYeEAeRsIMnfvXfk BShw== 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=T7liRnhSQZFeFeAxc6DzSWWVxHVukR76VssUutmbOek=; b=TGekcWr4TkE4mF3B5Cc4WXiWJoRjFuTiAbY8RvP/0WPBeDVve1gxII3DqjNyX3/K9f Z8u/g8bWfukdOch/I0MP2U+Me8mmrvJ8jJ25u6EZH0Oe3SVvBteudMXT9cMXk/6HJVlz KOTZIe0ZhswJqKHJb3fxa6aFDKCdTYqUG+rWchEKJCkUn19PnYn8rxnZ9F6xaXiqP5UB vlG6qq7OiU+IFVrAJBk8OnUCCtkwzbRuOWLSa7fmixXEqm0IV1eevVEl8e/jQoe3gLs1 XmQgsmmBmBrMyrS8ohCyJNcZt4K51aii74/ZB38d6t1WXkOu+PQXone/SrROTF6VsH1I a1fA== X-Gm-Message-State: ALQs6tC/lRMfpb5GndX4cgwjua72SFf3lxk8sWZBjQvu5oWdOyHPcu3M 2kH4kNqgk6Qob+hjxYBPpBvhQC9sKXcTrqC8LRVCkw== X-Google-Smtp-Source: AIpwx4/S2pWuITB9yxfG7KkUyTcPxLUP3/q+BjJdsWtUKs0zaZwqY9HD80ldTiCInX9c3ZyvhGDlrIvJJSeEQ2Y2yYk= X-Received: by 2002:a9d:5904:: with SMTP id t4-v6mr2622983oth.284.1522417020611; Fri, 30 Mar 2018 06:37:00 -0700 (PDT) MIME-Version: 1.0 Received: by 10.138.6.138 with HTTP; Fri, 30 Mar 2018 06:36:40 -0700 (PDT) In-Reply-To: References: From: Murtuza Zabuawala Date: Fri, 30 Mar 2018 19:06:40 +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: Joao De Almeida Pereira , pgadmin-hackers Content-Type: multipart/alternative; boundary="000000000000b6c6cd0568a15577" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --000000000000b6c6cd0568a15577 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 context. 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 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/utils/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/flask_l= ogin.py", > line 788, in decorated_view > if current_app.login_manager._login_disabled: > File "/var/www/flask/pgadmin4/local/lib/python2.7/site-packages/werkzeu= g/local.py", > line 338, in __getattr__ > return getattr(self._get_current_object(), name) > File "/var/www/flask/pgadmin4/local/lib/python2.7/site-packages/werkzeu= g/local.py", > line 297, in _get_current_object > return self.__local() > File "/var/www/flask/pgadmin4/local/lib/python2.7/site-packages/flask/g= lobals.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 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 shown in the File Dialog Box,this will b= e > a new feature, wouldnt it ? ) > > > On Fri, Mar 30, 2018 at 4:10 PM, 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 G= J >>> :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 extensi= on 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 then I c= an 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 (Did 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 fo= r >>> class >>> >>> Please, regenerate the patch following my previous comments. >>> >>> Thanks >>> Joao >>> >>> On Thu, Mar 29, 2018 at 12:54 PM Rahul Soshte >>> 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) >>>> >>>> >> > --000000000000b6c6cd0568a15577 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
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 context.
Ref: ../web/pgadmin/dashboard/tests/test_das= hboard_templates.py +274

=
--
Regards,
<= font size=3D"2">Murtuz= a Zabuawala
EnterpriseDB:=C2=A0http://www.enterprisedb.com
The= Enterprise PostgreSQL Company

<= /div>

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 ent= ered the extension .sql to the file while saving
-----------------------= -----------------------------------------------
Traceback (mos= t recent call last):
=C2=A0 File "/var/www/flask/pgadmin4/pgadmin4/web/pgadmin/tools/sqleditor/utils/tests/test_save_query_to_f= ile_utils.py", line 42, in runTest
=C2=A0=C2=A0=C2=A0 file_path_res= ult =3D save_query_to_file(self.file_data)
=C2=A0 File "/var/w= ww/flask/pgadmin4/pgadmin4/web/pgadmin/tools/sqleditor/utils/save= _query_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 "/var/www/flask/pgadmin4/local/lib/python2.7/site-packages/<= wbr>flask_login.py", line 788, in decorated_view
=C2=A0=C2=A0=C2=A0= if current_app.login_manager._login_disabled:
=C2=A0 File "/v= ar/www/flask/pgadmin4/local/lib/python2.7/site-packages/werkzeug/= local.py", line 338, in __getattr__
=C2=A0=C2=A0=C2=A0 return getat= tr(self._get_current_object(), name)
=C2=A0 File "/var/www/fla= sk/pgadmin4/local/lib/python2.7/site-packages/werkzeug/local.py&q= uot;, 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/site-packages/flask/globals.py", line 51, in _find_app
=C2= =A0=C2=A0=C2=A0 raise RuntimeError(_app_ctx_err_msg)
RuntimeError: Worki= ng outside of application context.

How do I test the extr= acted code inside context? How do I resolve this error.
I hav= e attached test_save_query_to_file_utils.py
and save_que= ry_to_file_utils.py

Murtuza, Actually I didnt = find any toggable button in the File Dialog Box So I made it general purpos= e ( 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 i= s shown 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 app= end .sql only if t= he sql extension is selected and user has not provided extension.=E2=80=8B<= /div>=C2=A0
=E2=80=8BLet say If I want to save the= file with .txt ex= tension then I can use All Files. =E2=80=8B


Codewise here are= some of my comments:
. You added the yarn-error.log file and a m= igration to the patch doesn't look intentional. Can you please remove t= hem?
. 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 (Did you reb= ase your branch before trying to create the patch?

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

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

P= lease, regenerate the patch following my previous comments.

<= /div>
Thanks
Joao

On Thu, Mar 29, 2018 at 12:54 PM Rahul Soshte <rahulsoshte360@gmail.com= > wrote:
<= div>
Hi,
When using save or save as feature if .sql is not= provided this Patch appends it.
as clearly mentioned in this link.
<= br>https://redmine.postgresql.org/issues/1998

I have ra= n pep8,regression and Jasmine tests too.

I have primarily chan= ged these files
=C2=A0=C2=A0=C2=A0=C2=A0 web/pgadmin/tools/sqleditor/__<= wbr>init__.py
=C2=A0=C2=A0=C2=A0=C2=A0 web/pgadmin/tools/sqleditor/static/js/sqleditor.js
=C2=A0=C2=A0=C2=A0=C2=A0 web/pgadmin/tools/sqledi= tor/utils/save_query_to_file.py


Regards,
Rahul Soshte (Hunter)




--000000000000b6c6cd0568a15577--