Re: Patch for pgAdmin4 RPM package - Mailing list pgadmin-hackers

From Sandeep Thakkar
Subject Re: Patch for pgAdmin4 RPM package
Date
Msg-id CANFyU97Ofo-fTF5HmNqrfX=i6a=6eJayksNq=W2Ay2yhT4ig5g@mail.gmail.com
Whole thread Raw
In response to Re: Patch for pgAdmin4 RPM package  (Dave Page <dpage@pgadmin.org>)
Responses Re: Patch for pgAdmin4 RPM package  (Dave Page <dpage@pgadmin.org>)
List pgadmin-hackers


On Mon, May 9, 2016 at 6:35 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

Initial eyeball review comments below...

On Fri, Apr 22, 2016 at 11:57 AM, Sandeep Thakkar <sandeep.thakkar@enterprisedb.com> wrote:
Hi Team, Dave,

Attached herewith are two patches. 

pgadmin4-rpm.patch - This is the main patch that includes scripts, makefiles and spec to create RPMs for RHEL6/RHEL7/F-22/F-23/F-24. 


Can we keep the directory names in lower case?
 
Sure. Will do that. 

It will create two RPMs i.e pgadmin4 and pgadmin4-web. The  pgadmin4 tpm depends on web and the web rpm depends on the python packages. I have commented the list of packages which are not available on some systems so that Devrim can build them.

The installation path for pgadmin4 is "/usr/pgadmin4-<major>.<minor>" and pgadmin4-web is the site-packages/pgadmin4-web

Shouldn't the -web package also have the major.minor version number in the path, to allow side-by-side installation?
Right. Now that we don't have major/minor, so, will it be /usr/pgadmin4-v1 and pgadmin4-web-v1 ? Or? 
 

pgadmin4-server-ini.patch - This is the patch for runtime/Server.cpp. As said pgadmin4-web and runtime installation directories are different and that means web does not exists in parallel to runtime like in sources. 

I observed that the location of application settings was not defined in Server.cpp. As per QSettings doc, the default location on Unix is the $HOME/.config/<companyname>/<appname>.conf. Here, $HOME depends on the user that runs the application. So, I thought why not to define the application settings in application directory itself. RPM then knows where to define the ApplicationPath. I tested it and it worked fine with me. I haven't done this change for platform dependent. 

Doesn't that prevent non-root users from changing the settings? Or (if you widen the permissions on the ini file), allow one user to mis-configure the app for others? I think what is needed here is a search path change, much like you added for the Mac app bundle. 

Right. Will use python command to find the site-packages path and then concatenate pgadmin4-web directory name. 
Other thoughts:

- Please rename the README to README.txt

- The code to build the RPMs should be entirely confined to pkg/rpm. A Makefile target should be added to /Makefile to build/clean the targets (this mistake was made with the Mac package too, but was one of the original requirements).

Please resolve these issues and I'll take another look.

Sure. Will share it with you soon.
 
Thanks.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



--
Sandeep Thakkar

pgadmin-hackers by date:

Previous
From: Surinder Kumar
Date:
Subject: Re: [pgAdmin4][Patch]: RM#1243 - Columns on the Query Tool should be sizeable
Next
From: Surinder Kumar
Date:
Subject: Re: [pgAdmin4][Patch]: RM#1243 - Columns on the Query Tool should be sizeable