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 1f1tkE-00044v-A7 for pgadmin-hackers@arkaria.postgresql.org; Fri, 30 Mar 2018 13:07:02 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1f1tkC-0001G6-RO for pgadmin-hackers@arkaria.postgresql.org; Fri, 30 Mar 2018 13:07:00 +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 1f1tkC-0001Fv-CY for pgadmin-hackers@lists.postgresql.org; Fri, 30 Mar 2018 13:07:00 +0000 Received: from mail-it0-x22f.google.com ([2607:f8b0:4001:c0b::22f]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1f1tk4-0004Tu-Rr for pgadmin-hackers@postgresql.org; Fri, 30 Mar 2018 13:06:59 +0000 Received: by mail-it0-x22f.google.com with SMTP id z7-v6so1633298iti.1 for ; Fri, 30 Mar 2018 06:06:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=6Yioxzuk60B46AccfLOQmTWB3atl+3DvHiuy+ZVpIrs=; b=uKq4iKyVLtXSuSoVNDHmyteEEx2njB+ouRuWAWKzWE1cwGRR0rxJUrgHUTl2z3xKU5 gIKsmbCVDQTeXpn2bTD+IbIGRiFEio5ph7FvwNp1AmDuV2U1qUr9VymauL3rppzZvve3 Fk4nSszwcfXYrO3Oyu7CPgpmIh4q3QCvctpdvtS9TF2fxSgSyKCiF2wTd3o5ZLnNNmPz gTXvG/TsTPdRC+wY69V/zpJdXS0FwsCafBWhqac+4R0fwJqk6TyuwT40ib3YtnJTqWFF 1ywhoFS2omIaQWmSj1gK6CvcHIsa04A5gvvtV2mj9btGKr9q2EjKdZ82PkfDna3nhDs8 7Lag== 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=6Yioxzuk60B46AccfLOQmTWB3atl+3DvHiuy+ZVpIrs=; b=dHyg7AfLnVrfPxJZnEv+v3oqnoBzKlyYATxxEApIhGXQmCkxjyLcXmRXirACR3udY3 LjTDW9W0gKlp9zsKLFtTqo6EeGtHuBAYaIqEsVGSV2sEtK02un2guNIg11Bwg4k3KQwJ wbHz/VwOxk3gNQf0cmKSpZutUX7/9l0cE1gpijZ+Luo7ciizFmVeQ1Cbi29DoxGUMB0f 99wWX524RdZYEG1oKVFdocWfarpgzcr8Jsn7xfdo83owEN8PdXf49DRsJfzXscOXaiHI ce4M8dbMAEhMW/21Wrk2/wvEHerjsDAmnH2uCVbcDWTWCM2L3c4Wlel+5f2pi3xgY2j+ 0Rvw== X-Gm-Message-State: AElRT7GAevklXVCuuYXK7af1zkZXWVOqQBquZYk+h4V2P5HBftrgmoRo h22aje9Yf6pJlsybi0K9+vt9xGVQGni4EtdyYzs= X-Google-Smtp-Source: AIpwx49w1gdtPogCOM4bX2v4VjPcB3/pkbAC/mmUiocSvaDWBxsHDcYqp4lV3DeUYVuZwWrlvs64wu1frIR7CJe88uo= X-Received: by 2002:a24:4505:: with SMTP id y5-v6mr2751243ita.5.1522415211954; Fri, 30 Mar 2018 06:06:51 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.26.212 with HTTP; Fri, 30 Mar 2018 06:06:50 -0700 (PDT) In-Reply-To: References: From: Rahul Soshte Date: Fri, 30 Mar 2018 18:36:50 +0530 Message-ID: Subject: Re: [pgAdmin4][Patch][Feature #1998] Appends .sql if extension not given when using 'save' or 'save as' feature To: Murtuza Zabuawala Cc: Joao De Almeida Pereira , pgadmin-hackers Content-Type: multipart/mixed; boundary="000000000000e9c20a0568a0e9a9" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --000000000000e9c20a0568a0e9a9 Content-Type: multipart/alternative; boundary="000000000000e9c2050568a0e9a7" --000000000000e9c2050568a0e9a7 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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/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/ sqleditor/utils/tests/test_save_query_to_file_utils.py", line 42, in runTes= t file_path_result =3D save_query_to_file(self.file_data) File "/var/www/flask/pgadmin4/pgadmin4/web/pgadmin/tools/ sqleditor/utils/save_query_to_file_utils.py", line 15, in save_query_to_fil= e storage_manager_path =3D get_storage_directory() File "/var/www/flask/pgadmin4/local/lib/python2.7/site-packages/flask_log= in.py", line 788, in decorated_view if current_app.login_manager._login_disabled: File "/var/www/flask/pgadmin4/local/lib/python2.7/site-packages/werkzeug/= local.py", line 338, in __getattr__ return getattr(self._get_current_object(), name) File "/var/www/flask/pgadmin4/local/lib/python2.7/site-packages/werkzeug/= local.py", line 297, in _get_current_object return self.__local() File "/var/www/flask/pgadmin4/local/lib/python2.7/site-packages/flask/glo= bals.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 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 excep= t >> 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 o= r >> not. Lets see what other people think. >> > =E2=80=8BYes, I also think it should append .sql only if the sql extensio= n 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 ca= n 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 >> 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) >>> >>> > --000000000000e9c2050568a0e9a7 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
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/tests/save_query_to_<= wbr>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= ):
=C2=A0 File "/var/www/flask/pgadmin4/pgadmin4/web/pgadmin/t= ools/sqleditor/utils/tests/test_save_query_to_file_utils.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/flask/pgadmin4/= pgadmin4/web/pgadmin/tools/sqleditor/utils/save_query_to_fil= e_utils.py", line 15, in save_query_to_file
=C2=A0=C2=A0=C2=A0 stor= age_manager_path =3D get_storage_directory()
=C2=A0 File "/var/www/= flask/pgadmin4/local/lib/python2.7/site-packages/flask_login.py&q= uot;, line 788, in decorated_view
=C2=A0=C2=A0=C2=A0 if current_app.logi= n_manager._login_disabled:
=C2=A0 File "/var/www/flask/pgadmin= 4/local/lib/python2.7/site-packages/werkzeug/local.py", line= 338, in __getattr__
=C2=A0=C2=A0=C2=A0 return getattr(self._get_current= _object(), name)
=C2=A0 File "/var/www/flask/pgadmin4/loc= al/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 Fi= le "/var/www/flask/pgadmin4/local/lib/python2.7/site-package= s/flask/globals.py", line 51, in _find_app
=C2=A0=C2=A0=C2=A0 raise= RuntimeError(_app_ctx_err_msg)
RuntimeError: Working outside of applica= tion context.

How do I test the extracted code inside con= text? 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 but= ton in the File Dialog Box So I made it general purpose ( I guess I will ha= ve to make one then and then if I select SQL all .sql files should be liste= d, and if I select All files then every type of file is shown in the File D= ialog 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 R= ahul,
I see you extracted some code, that is a pretty good move :D

We run the patch through the testing pipeline and ever= ything is green GJ :D
Also tested the functionality by hand and l= ooks like it is working except for "add the .sql extension when format is set to SQL." if yo= u set it to All Files=C2=A0 the extension is also added. Not sure if this i= s 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 extension is selected and user has not provided extension= .=E2=80=8B
=C2=A0
=E2=80=8BLet say If I want = to save the file with .tx= t 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. C= an 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:=C2=A0test_save_query_to_file.py = is empty, it is missing some tests there.


--000000000000e9c2050568a0e9a7-- --000000000000e9c20a0568a0e9a9 Content-Type: text/x-python; charset="US-ASCII"; name="save_query_to_file_utils.py" Content-Disposition: attachment; filename="save_query_to_file_utils.py" Content-Transfer-Encoding: base64 X-Attachment-Id: f_jfdy91zy0 aW1wb3J0IG9zCmZyb20gcGdhZG1pbi5taXNjLmZpbGVfbWFuYWdlciBpbXBvcnQgRmlsZW1hbmFn ZXIKZnJvbSBwZ2FkbWluLnV0aWxzIGltcG9ydCBnZXRfc3RvcmFnZV9kaXJlY3RvcnkKZnJvbSBw Z2FkbWluLnV0aWxzLmFqYXggaW1wb3J0IG1ha2VfanNvbl9yZXNwb25zZSwgYmFkX3JlcXVlc3Qs IFwKICAgIHN1Y2Nlc3NfcmV0dXJuLCBpbnRlcm5hbF9zZXJ2ZXJfZXJyb3IsIHVuYXV0aG9yaXpl ZAoKdHJ5OgogICAgZnJvbSB1cmxsaWIgaW1wb3J0IHVucXVvdGUKZXhjZXB0IEltcG9ydEVycm9y OgogICAgZnJvbSB1cmxsaWIucGFyc2UgaW1wb3J0IHVucXVvdGUKCgpkZWYgc2F2ZV9xdWVyeV90 b19maWxlKGZpbGVfZGF0YSk6CiAgICAjIHJldHJpZXZlIHN0b3JhZ2UgZGlyZWN0b3J5IHBhdGgK ICAgIHN0b3JhZ2VfbWFuYWdlcl9wYXRoID0gZ2V0X3N0b3JhZ2VfZGlyZWN0b3J5KCkKCiAgICAj IGdlbmVyYXRlIGZ1bGwgcGF0aCBvZiBmaWxlCiAgICBmaWxlX3BhdGggPSB1bnF1b3RlKGZpbGVf ZGF0YVsnZmlsZV9uYW1lJ10pCiAgICBpZiBoYXNhdHRyKHN0ciwgJ2RlY29kZScpOgogICAgICAg IGZpbGVfcGF0aCA9IHVucXVvdGUoCiAgICAgICAgICAgIGZpbGVfZGF0YVsnZmlsZV9uYW1lJ10K ICAgICAgICApLmVuY29kZSgndXRmLTgnKS5kZWNvZGUoJ3V0Zi04JykKCiAgICBpZiBub3QgZmls ZV9wYXRoLmVuZHN3aXRoKCcuc3FsJyk6CiAgICAgICAgZmlsZV9wYXRoID0gZmlsZV9wYXRoICsg Ii5zcWwiCgogICAgdHJ5OgogICAgICAgIEZpbGVtYW5hZ2VyLmNoZWNrX2FjY2Vzc19wZXJtaXNz aW9uKHN0b3JhZ2VfbWFuYWdlcl9wYXRoLCBmaWxlX3BhdGgpCiAgICBleGNlcHQgRXhjZXB0aW9u IGFzIGU6CiAgICAgICAgcmV0dXJuIGludGVybmFsX3NlcnZlcl9lcnJvcihlcnJvcm1zZz1zdHIo ZSkpCgogICAgIyBsc3RyaXAoKSByZXR1cm5zIGEgY29weSBvZiB0aGUgc3RyaW5nCiAgICAjIGlu IHdoaWNoIGFsbCBjaGFycyBoYXZlIGJlZW4gc3RyaXBwZWQKICAgICMgZnJvbSB0aGUgYmVnaW5u aW5nIG9mIHRoZSBzdHJpbmcgKGRlZmF1bHQgd2hpdGVzcGFjZSBjaGFyYWN0ZXJzKS4KICAgIGlm IHN0b3JhZ2VfbWFuYWdlcl9wYXRoIGlzIG5vdCBOb25lOgogICAgICAgIGZpbGVfcGF0aCA9IG9z LnBhdGguam9pbigKICAgICAgICAgICAgc3RvcmFnZV9tYW5hZ2VyX3BhdGgsCiAgICAgICAgICAg IGZpbGVfcGF0aC5sc3RyaXAoJy8nKS5sc3RyaXAoJ1xcJykKICAgICAgICApCgogICAgaWYgaGFz YXR0cihzdHIsICdkZWNvZGUnKToKICAgICAgICBmaWxlX2NvbnRlbnQgPSBmaWxlX2RhdGFbJ2Zp bGVfY29udGVudCddCiAgICBlbHNlOgogICAgICAgIGZpbGVfY29udGVudCA9IGZpbGVfZGF0YVsn ZmlsZV9jb250ZW50J10uZW5jb2RlKCkKCiAgICAjIHdyaXRlIHRvIGZpbGUKICAgIHRyeToKICAg ICAgICB3aXRoIG9wZW4oZmlsZV9wYXRoLCAnd2IrJykgYXMgb3V0cHV0X2ZpbGU6CiAgICAgICAg ICAgIGlmIGhhc2F0dHIoc3RyLCAnZGVjb2RlJyk6CiAgICAgICAgICAgICAgICBvdXRwdXRfZmls ZS53cml0ZShmaWxlX2NvbnRlbnQuZW5jb2RlKCd1dGYtOCcpKQogICAgICAgICAgICAgICAgcmV0 dXJuIGZpbGVfcGF0aAogICAgICAgICAgICBlbHNlOgogICAgICAgICAgICAgICAgb3V0cHV0X2Zp bGUud3JpdGUoZmlsZV9jb250ZW50KQogICAgICAgICAgICAgICAgcmV0dXJuIGZpbGVfcGF0aAog ICAgZXhjZXB0IElPRXJyb3IgYXMgZToKICAgICAgICBpZiBlLnN0cmVycm9yID09ICdQZXJtaXNz aW9uIGRlbmllZCc6CiAgICAgICAgICAgIGVycl9tc2cgPSAiRXJyb3I6IHswfSIuZm9ybWF0KGUu c3RyZXJyb3IpCiAgICAgICAgZWxzZToKICAgICAgICAgICAgZXJyX21zZyA9ICJFcnJvcjogezB9 Ii5mb3JtYXQoZS5zdHJlcnJvcikKICAgICAgICByZXR1cm4gaW50ZXJuYWxfc2VydmVyX2Vycm9y KGVycm9ybXNnPWVycl9tc2cpCiAgICBleGNlcHQgRXhjZXB0aW9uIGFzIGU6CiAgICAgICAgZXJy X21zZyA9ICJFcnJvcjogezB9Ii5mb3JtYXQoZS5zdHJlcnJvcikKICAgICAgICByZXR1cm4gaW50 ZXJuYWxfc2VydmVyX2Vycm9yKGVycm9ybXNnPWVycl9tc2cpCg== --000000000000e9c20a0568a0e9a9 Content-Type: text/x-python; charset="US-ASCII"; name="test_save_query_to_file_utils.py" Content-Disposition: attachment; filename="test_save_query_to_file_utils.py" Content-Transfer-Encoding: base64 X-Attachment-Id: f_jfdy96em1 aW1wb3J0IHN5cwpmcm9tIHBnYWRtaW4udXRpbHMucm91dGUgaW1wb3J0IEJhc2VUZXN0R2VuZXJh dG9yCmZyb20gcGdhZG1pbi50b29scy5zcWxlZGl0b3IudXRpbHMuc2F2ZV9xdWVyeV90b19maWxl X3V0aWxzIGltcG9ydCBcCiAgICBzYXZlX3F1ZXJ5X3RvX2ZpbGUKZnJvbSBmbGFzayBpbXBvcnQg Rmxhc2sKCmlmIHN5cy52ZXJzaW9uX2luZm8gPCAoMywgMyk6CiAgICBmcm9tIG1vY2sgaW1wb3J0 IHBhdGNoLCBBTlkKZWxzZToKICAgIGZyb20gdW5pdHRlc3QubW9jayBpbXBvcnQgcGF0Y2gsIEFO WQoKY2xhc3MgVGVzdFNhdmVRdWVyeVRvRmlsZShCYXNlVGVzdEdlbmVyYXRvcik6CiAgICAiIiIK ICAgIENoZWNrIHRoYXQgdGhlIHNhdmVfcXVlcnkgbWV0aG9kIHdvcmtzIGFzIGludGVuZGVkCgog ICAgIiIiCgogICAgc2NlbmFyaW9zID0gWwogICAgICAgICgKICAgICAgICAgICAgJ1doZW4gdXNl ciBoYXMgbm90IGVudGVyZWQgdGhlIGV4dGVuc2lvbiAuc3FsIHdoaWxlIHNhdmluZyB0aGUgZmls ZScsCiAgICAgICAgICAgIGRpY3QoCiAgICAgICAgICAgICAgICBmaWxlX2RhdGE9IHsKICAgICAg ICAgICAgICAgICAgICAnZmlsZV9uYW1lJzogJy9hYmMveHl6JywKICAgICAgICAgICAgICAgICAg ICAnZmlsZV9jb250ZW50JzogJ3NvbWUgZGF0YSBoZXJlJywgCiAgICAgICAgICAgICAgICB9LAog ICAgICAgICAgICAgICAgZXhwZWN0ZWRfcmV0dXJuX3ZhbHVlPScvYWJjL3h5ei5zcWwnCiAgICAg ICAgICAgICkKICAgICAgICApLAogICAgICAgICgKICAgICAgICAgICAgJ1doZW4gdXNlciBoYXMg ZW50ZXJlZCB0aGUgZXh0ZW5zaW9uIC5zcWwgdG8gdGhlIGZpbGUgd2hpbGUgc2F2aW5nJywKICAg ICAgICAgICAgZGljdCgKICAgICAgICAgICAgICAgIGZpbGVfZGF0YT0gewogICAgICAgICAgICAg ICAgICAgICdmaWxlX25hbWUnOiAnL2FiYy94eXguc3FsJywKICAgICAgICAgICAgICAgICAgICAn ZmlsZV9jb250ZW50JzogJ3NvbWUgZGF0YSBoZXJlJywKICAgICAgICAgICAgICAgIH0sCiAgICAg ICAgICAgICAgICBleHBlY3RlZF9yZXR1cm5fdmFsdWU9Jy9hYmMveHl4LnNxbCcKICAgICAgICAg ICAgKQogICAgICAgICksCiAgICBdCiAgICAKICAgIGRlZiBydW5UZXN0KHNlbGYpOgogICAgICAg ICAgICBmaWxlX3BhdGhfcmVzdWx0ID0gc2F2ZV9xdWVyeV90b19maWxlKHNlbGYuZmlsZV9kYXRh KQogICAgICAgICAgICBzZWxmLmFzc2VydEVxdWFscyhmaWxlX3BhdGhfcmVzdWx0LCBzZWxmLmV4 cGVjdGVkX3JldHVybl92YWx1ZSkK --000000000000e9c20a0568a0e9a9--