Re: [pgAdmin4][Patch][Feature #1998] Appends .sql if extension not given when using 'save' or 'save as' feature

From: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
To: Rahul Soshte <rahulsoshte360(at)gmail(dot)com>
Cc: Joao De Almeida Pereira <jdealmeidapereira(at)pivotal(dot)io>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch][Feature #1998] Appends .sql if extension not given when using 'save' or 'save as' feature
Date: 2018-03-30 13:37:40
Message-ID: CAKKotZSCFBnwZxwGH3VKUnNYtXJFjqBePxt3LBCs=8PPYBT3cw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

++ Attaching screenshot

On Fri, Mar 30, 2018 at 7:06 PM, Murtuza Zabuawala <
murtuza(dot)zabuawala(at)enterprisedb(dot)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 <rahulsoshte360(at)gmail(dot)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/utils/tests/save_query_to_file_u
>> tils.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 = 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 = get_storage_directory()
>> File "/var/www/flask/pgadmin4/local/lib/python2.7/site-packages/flask_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/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/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 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(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>
>>>
>>>
>>> On Thu, Mar 29, 2018 at 11:45 PM, Joao De Almeida Pereira <
>>> jdealmeidapereira(at)pivotal(dot)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.
>>>>
>>> ​Yes, I also think it should append .sql only if the sql extension is
>>> selected and user has not provided extension.​
>>>
>>> ​Let say If I want to save the file with .txt extension then I can use
>>> All Files. ​
>>>
>>>
>>>> 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(at)gmail(dot)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)
>>>>>
>>>>>
>>>
>>
>

Attachment Content-Type Size
image/png 67.6 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Victoria Henry 2018-03-30 14:29:15 Re: [pgAdmin4][RM#3154] Update modules to latest version
Previous Message Murtuza Zabuawala 2018-03-30 13:36:40 Re: [pgAdmin4][Patch][Feature #1998] Appends .sql if extension not given when using 'save' or 'save as' feature