Re: [GSoC][Patch] Automatic Mode Detection V1

From: Yosry Muhammad <yosrym93(at)gmail(dot)com>
To: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>, Dave Page <dpage(at)pgadmin(dot)org>, pgadmin-hackers(at)postgresql(dot)org
Subject: Re: [GSoC][Patch] Automatic Mode Detection V1
Date: 2019-06-24 16:43:16
Message-ID: CAFSMqn-62TQoHT-8fKDq7cWhofQ8XtLbHQV5q++u+NQw=dXDOg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi,

Please find attached a patch file with the following updates (last patch +
updates) attached:
- Changed the color to $color-gray-lighter and added the shortcut for the
new button.
- Added a preferences option to enable/disable prompting on uncommited
transactions on exiting.
- Changed call_render_after_poll_specs test to be in sync with code
changes, also fixed a mix up in the test descriptions in the same file.
- Fixed a bug with a recent patch 'Allow editing of data where a primary
key column includes a % sign in the value.' that occurred when the primary
key was a number.

- After running python and feature tests, changes were made to nearly all
>> the files (git status shows modifications in a ton of files), is there
>> something I have done wrong?
>>
> What command did you use, can you share the screenshot of the files
> changed?
>

I tried it again after a proper test_config.json as you mentioned and
everything worked fine. All tests pass for this patch except for 3 feature
tests that all fail because of a TimeoutException related to selenium.
Please find a log file of the feature tests attached.

>
> - What else is missing from this patch to make it applicable ? I would
>> like to produce a release-ready patch if possible. If so, I can continue
>> working on the project on following patches, I just want to know what is
>> the minimum amount of work needed to make this patch release-ready
>> (especially that changes are being made in the master branch that require
>> me to re-edit parts of the code that I have written before to keep things
>> in-sync).
>>
> @Dave Page is the right person to answer this.
>

Waiting for his reply :D

- For the bug that I reported before (generated queries in Query History
>> appear in a distorted way for the user), to get the actual query that is
>> being executed I can use the mogirfy() function of psycopg2 but I need
>> access to a cursor. I can get one directly in save_changed_data() function
>> by using conn.conn.cursor() but then I would be bypassing the wrapper
>> Connection class. Should I modify the wrapper Connection class and add a
>> function that can provide a cursor (or a wrapper around cursor.mogrify() )?
>> Thoughts?
>>
> Could you please share the query/screenshot ? The query history just
> stores the SQL text and fetches back to show in CodeMirror. No
> modifications/generation of queries is done by Query History.
>
>>
>>
By 'generated queries' I meant the querie that are generated by pgAdmin to
save changes to the data grid to the database. Here is a screenshot from
the released version (not the version I am working on).
[image: pg-query-history-bug.png]
Scenario:
- Opened View Data on a table (public.kweek)
- Modified a cell in a column named media_url with a primary key (id = 50)
to 'new link'
- Instead of showing 'new link' in the query %(media_url) is shown.

This can be fixed in save_changed_data() function in my patch but I need
access to a cursor as previously mentioned. Thoughts?

Thanks a lot for your help!

--
*Yosry Muhammad Yosry*

Computer Engineering student,
The Faculty of Engineering,
Cairo University (2021).
Class representative of CMP 2021.
https://www.linkedin.com/in/yosrym93/

Attachment Content-Type Size
query_tool_automatic_mode_switch_v3.patch text/x-patch 105.3 KB
regression.log text/x-log 19.2 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2019-06-24 20:14:27 pgAdmin 4 commit: Update version for release
Previous Message Aditya Toshniwal 2019-06-24 11:03:15 Re: [GSoC][Patch] Automatic Mode Detection V1