There is an error when checking the major version of servers in schema diff

2021-01-17 Thread Huang, Jun
Hi,

The function get_round_val() outputs the wrong value when major version is 
before 10.
For example,
   Source server version: 90500
   Target server version: 90502
Or
   Source server version: 90502
   Target server version: 90602

The attached patch shows the changes.
--
Thanks && regards
HuangJ





check-major-version-of-servers.patch
Description: check-major-version-of-servers.patch


Re: [pgAdmin][RM1802] ERD Tool (Beta)

2021-01-17 Thread Aditya Toshniwal
Hi Akshay,

I forgot to remove few of the dependencies which are not required as of now
(may be in future). Attached patch removes those dependencies from
package.json.

On Sat, Jan 16, 2021 at 5:08 PM Akshay Joshi 
wrote:

> Thanks, patch applied.
>
> On Fri, Jan 15, 2021 at 7:01 PM Aditya Toshniwal <
> aditya.toshni...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> I've fixed the issues. You can find the comments inline.
>> I've also added PropTypes for the components for increased validation.
>>
>> On Tue, Jan 12, 2021 at 12:18 PM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi Aditya,
>>>
>>> The functionalities and the code looks good to me, however some of the 
>>> comments as below:
>>>
>>>
>>>- Correct the comments at some places (3 occurrences found  in 
>>> /erd/__init__.py)  which mention Schema diff instead of ERD.
>>>
>>> Some comments in the JS/JSX file regarding components/functions (For 
>>> example, IconButton (forwardRef), Bodywidget etc.) would
>>>
>>> be great help as we all are new to React.
>>>
>>> Done. Added comments to the components.
>>
>>>
>>>- Remove the unused imports (for ex bad_request) in /erd/__init__.py
>>>
>>> Removed.
>>
>>>
>>>- Remove commented code
>>>
>>> # req_args = request.args
>>> # if ('recreate' in req_args and
>>> # req_args['recreate'] == '1'):
>>> # connect = False
>>>
>>> Removed.
>>
>>>
>>>- TableNode.jsx, below two lines can be combined.
>>>
>>> import { PortModelAlignment, DefaultNodeModel } from
>>> '@projectstorm/react-diagrams';
>>> import { PortWidget } from '@projectstorm/react-diagrams';
>>>
>>> Done.
>>
>>>
>>>- onImageClick function in BodyWidget.jsx is no use I think, so it 
>>> should be removed.
>>>
>>> I wanted to keep the code as it will be used in future. Anyway, I've
>> removed the code.
>>
>>>
>>>- I got some console errors while adding/editing tables. Refer to the 
>>> attached screenshot.
>>>
>>> I tried but I didn't get any. Looking at the screenshot, the error is
>> from the underlying library. Can't do much in this.
>>
>>>
>>>- In the column Edit Mode, while deleting the primary key, it gives the 
>>> error, which does not go away with any further modifications.
>>>
>>> Fixed.
>>
>>>
>>>- While generating the SQL, if the server is disconnected, a proper 
>>> error message should be thrown, right now some server side error is coming.
>>>
>>> It will show connection lost error now. Fixed.
>>
>>>
>>>- Please remove ... from the menu title (New ERD Project(Beta)...) as it 
>>> is not opening a dialog.
>>>
>>> Done.
>>
>>>
>>>- For large data sets, generate ERD hangs.
>>>
>>> It shows the spinner and waits for the response to come from the back
>> end. I've used the existing table fetching code which is used at other
>> places. I'll create an RM to improve the back end code for fetching the
>> tables data which will help the schema diff tool as well.
>>
>>>
>>>- Opening the ERD panel in a new window is not working, it opens in the 
>>> same tab even if you have set the Preference "Open in new browser tab" to 
>>> True.
>>>
>>> Fixed. Added the setting in "Tab settings".
>>
>>>
>>>- No shortcut is provided to open the ERD Tool.
>>>
>>> A shortcut is helpful if we are using it frequently. ERD tool won't be
>> used that frequently. We already have a limited number of keys available
>> for shortcuts. I think we should roll out without shortcut for now. If
>> there is a user demand for it then we can think of adding it.
>>
>>>
>>>- SonarQube fixes required.
>>>
>>> Fixed.
>>
>>>
>>>-
>>>
>>> *Suggestion:*
>>>
>>> While removal of the FK link, If any of the table is selected, it is being 
>>> deleted with FK link.
>>> Either we should warn the user OR make 2 different buttons for FK removal 
>>> and table removal as the user may be confused if the selected table is also 
>>> removed with the FK.
>>>
>>> I've added a confirmation dialog which will show the number of tables
>> and links selected. This way user will know what he has selected before
>> deleting.
>>
>>> *Observations:*
>>>
>>> Lodash has been used in this module in place of Underscore, though the
>>> dependency is already introduced by another module,
>>> but we have mentioned it in the package.json file, which is somewhat not
>>> convincing to me.
>>> Lodash is more advanced than Underscore but we should pick anyone as it
>>> will be easy to manage.
>>>
>>> TL;DR; we cannot.
>> lodash is a peer dependency for react-diagrams (and some existing modules
>> in pgAdmin) so it will come to package.json without choice. We cannot
>> remove underscore because it is a dependency of backbone. Underscore is
>> outdated, and I cannot migrate the complete pgAdmin code. So, I decided
>> to go with 100/0 method. All the new codes will use lodash only as we'll
>> phase out underscore with time. Just like jQuery vs ReactJS.
>>
>>>
>>>
>>> Table dialog code is duplicate of the table node, as it was difficult to
>>> extend 

Patch for SonarQube fixes.

2021-01-17 Thread Nikhil Mohite
Hi Team,

I have fixed a few sonarQube issues, PFA patch
Details as follows:

1. Preferences:

   - Refactor this function to reduce its Cognitive Complexity from 18 to
   the 15 allowed.
   - Merge this if statement with the enclosing one.
   - Define a constant instead of duplicating this literal 'tab settings' 5
   times.

2. Connection:

   - Remove this unneeded "pass".

3. Sqleditor:

   - Remove this useless assignment to variable "msgDiv".

4. Debugger:

   - Review this useless assignment: "index" already holds the assigned
   value along all execution paths.
   - 'label' is already declared in the upper scope.
   - 'label' is already declared in the upper scope.
   - 'browser_preferences' is already declared in the upper scope.

5. FileManager:

   - 'path' is already declared in the upper scope.


-- 
*Thanks & Regards,*
*Nikhil Mohite*
*Software Engineer.*
*EDB Postgres* 
*Mob.No: +91-7798364578.*


SonarQubeCodeSmellFixes.patch
Description: Binary data


Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

2021-01-17 Thread Khushboo Vashi
Hi,

Please find the attached updated patch with the below changes:

- Dependencies are added into Linux packages in the RPM/DEBs.
- Dev packages are added in the setup scripts for Linux.
- The required packages are added in the Dockerfile.
- Conditional gssapi 1.6.2 dependency is added for Python 3.5 in
requirements.txt.
- krb5 libs are not bundled with the Desktop packages, so added the gssapi
dependency into the try/catch block.
- .dockerignore is introduced to ignore unwanted files/folders like
node_modules etc., which will make the docker build fast. (By Ashesh Vashi)

Thanks,
Khushboo

On Fri, Jan 15, 2021 at 3:48 PM Dave Page  wrote:

> And another thought...
>
> Some of the Jenkins QA jobs setup the virtual environment for running
> tests themselves. I believe these might actually be the cause of some of
> the failures we saw initially with the commit - I'll review those, and
> ensure they won't try to build the gssapi module from source on Windows.
>
> On Thu, Jan 14, 2021 at 4:34 PM Dave Page  wrote:
>
>> FYI, I did a quick test (and browse of PyPI):
>>
>> - On Windows, it seems there is a binary wheel available:
>>
>> (gssapi) C:\Users\dpage>pip install gssapi
>> Collecting gssapi
>>   Downloading gssapi-1.6.12-cp39-cp39-win_amd64.whl (670 kB)
>>  || 670 kB 3.3 MB/s
>> Collecting decorator
>>   Downloading decorator-4.4.2-py2.py3-none-any.whl (9.2 kB)
>> Installing collected packages: decorator, gssapi
>> Successfully installed decorator-4.4.2 gssapi-1.6.12
>>
>> - On macOS, the wheel is built by pip, but it doesn't seem to have any
>> additional binary dependencies.
>>
>> This should simplify things a lot - we just need to ensure the build
>> scripts use the binary package on Windows, and install the build deps on
>> the Linux/Docker environments (and update the package builds with the
>> additional dependencies of course).
>>
>>
>> On Thu, Jan 14, 2021 at 4:04 PM Dave Page  wrote:
>>
>>> Hi Khushboo,
>>>
>>> As you know, this has been rolled back as the buildfarm blew up. I think
>>> there are a number of TODOs that need to be addressed, given that the
>>> gssapi Python module is dependent on MIT Kerberos:
>>>
>>> In the patch:
>>>
>>> - Linux packages will need the additional dependencies to be declared in
>>> the RPM/DEBs.
>>> - The setup scripts for Linux will need to have the -dev packages added
>>> as appropriate.
>>> - The various READMEs that describe how to build packages will need to
>>> be updated.
>>> - The Dockerfile will need to be modified to add the required packages.
>>> - The Windows build will need to be updated so the installer ships
>>> additional required DLLs.
>>> - Are there any additional macOS dependencies? If so, they need to be
>>> handled.
>>>
>>> In the buildfarm:
>>>
>>> - All Linux build VMs need to be updated with the additional
>>> dependencies.
>>> - On Windows, we need to figure out how to build/ship KfW. It's a pain
>>> to build, which we would typically do ourselves to ensure we're
>>> consistently using the same buildchain. If we do build it ourselves:
>>>   - Will the Python package find it during it's build?
>>>   - We'll need to create a Jenkins job to perform the build.
>>> - Is any work required on macOS, or does it ship with everything that's
>>> needed? If not, we'll need to build it, and create the Jenkins job.
>>>
>>> One final thought: on Windows/macOS, can we force a binary installation
>>> from PIP (pip install --only-binary=gssapi gssapi)? If so, will that
>>> include the required libraries, as psycopg2-binary does?
>>>
>>>
>>> On Thu, Jan 14, 2021 at 8:18 AM Akshay Joshi <
>>> akshay.jo...@enterprisedb.com> wrote:
>>>
 Thanks, patch applied.

 On Thu, Jan 14, 2021 at 1:42 PM Khushboo Vashi <
 khushboo.va...@enterprisedb.com> wrote:

> Hi,
>
> Please ignore my previous patch, attached the updated one.
>
> Thanks,
> Khushboo
>
> On Thu, Jan 14, 2021 at 12:17 PM Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> Please find the attached updated patch.
>>
>> Thanks,
>> Khushboo
>>
>> On Thu, Jan 14, 2021 at 12:00 PM Akshay Joshi <
>> akshay.jo...@enterprisedb.com> wrote:
>>
>>> Hi Khushboo
>>>
>>> Seems you have attached the wrong patch. Please send the updated
>>> patch.
>>>
>>> On Wed, Jan 13, 2021 at 2:35 PM Khushboo Vashi <
>>> khushboo.va...@enterprisedb.com> wrote:
>>>
 Hi,

 Please find the attached updated patch.

 Thanks,
 Khushboo

 On Fri, Jan 1, 2021 at 1:07 PM Aditya Toshniwal <
 aditya.toshni...@enterprisedb.com> wrote:

> Hi Khushboo,
>
> I've just done the code review. Apart from below, the patch looks
> good to me:
>
> 1) Move the auth source constants -ldap, kerberos out of app
> object. They don't belong th

pgAdmin 4 commit: Remove few dependencies which are not required for ER

2021-01-17 Thread Akshay Joshi
Remove few dependencies which are not required for ERD Tool.

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=5afa4f1995d7d1fa5d2641f62e166b47101aa30f
Author: Aditya Toshniwal 

Modified Files
--
web/package.json |  3 ---
web/yarn.lock| 66 +---
2 files changed, 1 insertion(+), 68 deletions(-)



pgAdmin 4 commit: Fixed issues reported by SonarQube.

2021-01-17 Thread Akshay Joshi
Fixed issues reported by SonarQube.

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=d4a3e4d92c397fc8789f3f5dde45c61c04138bf1
Author: Nikhil Mohite 

Modified Files
--
.../browser/register_browser_preferences.py| 10 ++---
web/pgadmin/misc/file_manager/static/js/utility.js | 20 -
web/pgadmin/preferences/__init__.py|  6 +--
web/pgadmin/tools/debugger/static/js/debugger.js   | 12 ++---
.../tools/debugger/static/js/debugger_ui.js|  4 +-
.../tools/debugger/static/js/debugger_utils.js |  2 +-
web/pgadmin/tools/sqleditor/static/js/sqleditor.js |  1 -
web/pgadmin/utils/driver/psycopg2/connection.py|  1 -
web/pgadmin/utils/preferences.py   | 51 --
9 files changed, 56 insertions(+), 51 deletions(-)



Re: Patch for SonarQube fixes.

2021-01-17 Thread Akshay Joshi
Thanks, patch applied.

On Mon, Jan 18, 2021 at 11:15 AM Nikhil Mohite <
nikhil.moh...@enterprisedb.com> wrote:

> Hi Team,
>
> I have fixed a few sonarQube issues, PFA patch
> Details as follows:
>
> 1. Preferences:
>
>- Refactor this function to reduce its Cognitive Complexity from 18 to
>the 15 allowed.
>- Merge this if statement with the enclosing one.
>- Define a constant instead of duplicating this literal 'tab settings'
>5 times.
>
> 2. Connection:
>
>- Remove this unneeded "pass".
>
> 3. Sqleditor:
>
>- Remove this useless assignment to variable "msgDiv".
>
> 4. Debugger:
>
>- Review this useless assignment: "index" already holds the assigned
>value along all execution paths.
>- 'label' is already declared in the upper scope.
>- 'label' is already declared in the upper scope.
>- 'browser_preferences' is already declared in the upper scope.
>
> 5. FileManager:
>
>- 'path' is already declared in the upper scope.
>
>
> --
> *Thanks & Regards,*
> *Nikhil Mohite*
> *Software Engineer.*
> *EDB Postgres* 
> *Mob.No: +91-7798364578.*
>


-- 
*Thanks & Regards*
*Akshay Joshi*
*pgAdmin Hacker | Principal Software Architect*
*EDB Postgres *

*Mobile: +91 976-788-8246*


Re: [pgAdmin][RM1802] ERD Tool (Beta)

2021-01-17 Thread Akshay Joshi
Thanks, patch applied.

On Mon, Jan 18, 2021 at 10:34 AM Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:

> Hi Akshay,
>
> I forgot to remove few of the dependencies which are not required as of
> now (may be in future). Attached patch removes those dependencies from
> package.json.
>
> On Sat, Jan 16, 2021 at 5:08 PM Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>> Thanks, patch applied.
>>
>> On Fri, Jan 15, 2021 at 7:01 PM Aditya Toshniwal <
>> aditya.toshni...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> I've fixed the issues. You can find the comments inline.
>>> I've also added PropTypes for the components for increased validation.
>>>
>>> On Tue, Jan 12, 2021 at 12:18 PM Khushboo Vashi <
>>> khushboo.va...@enterprisedb.com> wrote:
>>>
 Hi Aditya,

 The functionalities and the code looks good to me, however some of the 
 comments as below:


- Correct the comments at some places (3 occurrences found  in 
 /erd/__init__.py)  which mention Schema diff instead of ERD.

 Some comments in the JS/JSX file regarding components/functions (For 
 example, IconButton (forwardRef), Bodywidget etc.) would

 be great help as we all are new to React.

 Done. Added comments to the components.
>>>

- Remove the unused imports (for ex bad_request) in /erd/__init__.py

 Removed.
>>>

- Remove commented code

 # req_args = request.args
 # if ('recreate' in req_args and
 # req_args['recreate'] == '1'):
 # connect = False

 Removed.
>>>

- TableNode.jsx, below two lines can be combined.

 import { PortModelAlignment, DefaultNodeModel } from
 '@projectstorm/react-diagrams';
 import { PortWidget } from '@projectstorm/react-diagrams';

 Done.
>>>

- onImageClick function in BodyWidget.jsx is no use I think, so it 
 should be removed.

 I wanted to keep the code as it will be used in future. Anyway, I've
>>> removed the code.
>>>

- I got some console errors while adding/editing tables. Refer to the 
 attached screenshot.

 I tried but I didn't get any. Looking at the screenshot, the error is
>>> from the underlying library. Can't do much in this.
>>>

- In the column Edit Mode, while deleting the primary key, it gives the 
 error, which does not go away with any further modifications.

 Fixed.
>>>

- While generating the SQL, if the server is disconnected, a proper 
 error message should be thrown, right now some server side error is coming.

 It will show connection lost error now. Fixed.
>>>

- Please remove ... from the menu title (New ERD Project(Beta)...) as 
 it is not opening a dialog.

 Done.
>>>

- For large data sets, generate ERD hangs.

 It shows the spinner and waits for the response to come from the back
>>> end. I've used the existing table fetching code which is used at other
>>> places. I'll create an RM to improve the back end code for fetching the
>>> tables data which will help the schema diff tool as well.
>>>

- Opening the ERD panel in a new window is not working, it opens in the 
 same tab even if you have set the Preference "Open in new browser tab" to 
 True.

 Fixed. Added the setting in "Tab settings".
>>>

- No shortcut is provided to open the ERD Tool.

 A shortcut is helpful if we are using it frequently. ERD tool won't be
>>> used that frequently. We already have a limited number of keys
>>> available for shortcuts. I think we should roll out without shortcut for
>>> now. If there is a user demand for it then we can think of adding it.
>>>

- SonarQube fixes required.

 Fixed.
>>>

-

 *Suggestion:*

 While removal of the FK link, If any of the table is selected, it is being 
 deleted with FK link.
 Either we should warn the user OR make 2 different buttons for FK removal 
 and table removal as the user may be confused if the selected table is 
 also removed with the FK.

 I've added a confirmation dialog which will show the number of tables
>>> and links selected. This way user will know what he has selected before
>>> deleting.
>>>
 *Observations:*

 Lodash has been used in this module in place of Underscore, though the
 dependency is already introduced by another module,
 but we have mentioned it in the package.json file, which is somewhat
 not convincing to me.
 Lodash is more advanced than Underscore but we should pick anyone as it
 will be easy to manage.

 TL;DR; we cannot.
>>> lodash is a peer dependency for react-diagrams (and some existing
>>> modules in pgAdmin) so it will come to package.json without choice. We
>>> cannot remove underscore because it is a dependency of backbone. Underscore
>>> is out