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 1f1wuA-0008Gg-MW for pgadmin-hackers@arkaria.postgresql.org; Fri, 30 Mar 2018 16:29:31 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1f1wu9-0002EY-K2 for pgadmin-hackers@arkaria.postgresql.org; Fri, 30 Mar 2018 16:29:29 +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 1f1wu9-0002Da-37 for pgadmin-hackers@lists.postgresql.org; Fri, 30 Mar 2018 16:29:29 +0000 Received: from mail-ot0-x236.google.com ([2607:f8b0:4003:c0f::236]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1f1wu5-0000TR-DC for pgadmin-hackers@postgresql.org; Fri, 30 Mar 2018 16:29:27 +0000 Received: by mail-ot0-x236.google.com with SMTP id j8-v6so2472180ota.7 for ; Fri, 30 Mar 2018 09:29:24 -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=1NkidQDq1aLDB0XbBeCbcQqy/G1sXfyGChena8XXzJc=; b=1RWZYgYNV5BEfu4xO7Bfg2aMmvAYyETUq7LnH55xPQmUXsxWCOyaGx4K5k89VSQxn9 PgI5dB03rjoUWuTEQA/ROwjGzxnILDdaOyPxUK+a1sY8EVDkFE4i+WJecIeDXuyPBP3y YbxDeWCLgRQ9rQP7IZ75gNSmYny9Tc+fo4pblHpys4llsASwgKNeARGFzk0BXKV/fdD/ 2OsuQtsS9gJZOkLT9thq0QcV480NViqy3qeTP33udIF/pI1SP7nWr+k1EwwU+qYQCuC1 VFbdP0kP0zsVitlehJ7XSxfXc/yO8CX7Kmf6WgP0wkpbDz3yhPOdwp1b2VAhISyjq0zO 9KeQ== 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=1NkidQDq1aLDB0XbBeCbcQqy/G1sXfyGChena8XXzJc=; b=ezauTPG42fcHBtTRLcP8c0/QU0Ubgd/Qz1Xz8L2W5BNfDBikMo1o/5r0xtwKei+16a 9KUkySVQIka87tC60Rz1SP5JxHS5C09Zdeto/c7HcTKmqONwipEm4AtdgvStCf/BYFPo t3z97RKzZUmDydx9Kfo/uQjWHoXkb59g6Wnoz/OYcej75kTB4acswY8o8Cs9R1JWVqmz S82ueniv+dQBjwROJ1UWws/QlVdG6Qkqmui2ynq6EnkcKvbwEhnubq09wTyhvi/+kU93 MUKzgcOZoZysn8bOMINAvG/nKUYEpmOz1NqlGCCPAlDWG/sjVhdVOCp87jCSvHQHgbDz E5xw== X-Gm-Message-State: AElRT7FzvFHuv1Npt3QvAv2pNu1wptHUZ1DspX7Fp5Fw+pKA9HxK4r2z ZeOTrZgPbfzDITvX0djPpU9w4QDHADeUkKYsGg6ZgA== X-Google-Smtp-Source: AIpwx4+qQEBLMJSdy/BYYHX7jg3iqUsokeKqzHJZQA2GbhW8/ViGd/PKMLg37HP61Nr6DAvvzME9oqtZM8asuaXtpIk= X-Received: by 2002:a9d:4e16:: with SMTP id p22-v6mr8173444otf.311.1522427363193; Fri, 30 Mar 2018 09:29:23 -0700 (PDT) MIME-Version: 1.0 Received: by 10.138.6.138 with HTTP; Fri, 30 Mar 2018 09:29:02 -0700 (PDT) In-Reply-To: References: From: Murtuza Zabuawala Date: Fri, 30 Mar 2018 21:59:02 +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="0000000000002e10640568a3be3c" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --0000000000002e10640568a3be3c Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable =E2=80=8BI don't think so, Could you inspect html/css code on 'Save as' dia= log 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 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 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/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/flas= k_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/werk= zeug/local.py", >>>> line 338, in __getattr__ >>>> return getattr(self._get_current_object(), name) >>>> File "/var/www/flask/pgadmin4/local/lib/python2.7/site-packages/werk= zeug/local.py", >>>> line 297, in _get_current_object >>>> return self.__local() >>>> File "/var/www/flask/pgadmin4/local/lib/python2.7/site-packages/flas= k/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 Dialog >>>> Box So I made it general purpose ( I guess I will have to make one the= n 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 wil= l 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 gree= n >>>>>> 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 th= is 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 exte= nsion 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 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 (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 >>>>>> 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) >>>>>>> >>>>>>> >>>>> >>>> >>> >> > --0000000000002e10640568a3be3c Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
=E2=80=8BI don't think so, Could you in= spect html/css code on 'Save as' dialog within your browser window = and see if it's present or not?=E2=80=8B

<= /div>

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.enterprisedb.com
The Enterprise PostgreS= QL Company


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)







--0000000000002e10640568a3be3c--