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 1evbin-0008Ae-Mq for pgadmin-hackers@arkaria.postgresql.org; Tue, 13 Mar 2018 04:39:33 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1evbim-0005T9-OZ for pgadmin-hackers@arkaria.postgresql.org; Tue, 13 Mar 2018 04:39:32 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1evbfg-0003vO-SJ for pgadmin-hackers@lists.postgresql.org; Tue, 13 Mar 2018 04:36:20 +0000 Received: from mail-ot0-x22b.google.com ([2607:f8b0:4003:c0f::22b]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1evbfW-0001U9-RQ for pgadmin-hackers@postgresql.org; Tue, 13 Mar 2018 04:36:19 +0000 Received: by mail-ot0-x22b.google.com with SMTP id g97so17647647otg.13 for ; Mon, 12 Mar 2018 21:36:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=SqAi37St5KAFdIAsDtEJ2iCXyFUQLmpQH3NdNXw4wF4=; b=1+8GIfqj9TRv+0rdVD9Z+XTh790k6tz8+rLgNLQdn6wobVIwqcrp7iw0OKaOSAbtaA aCucJcqkrRidPIDDzsdNUduu1AHK73Gx5jRNkNVeVkwVLKxWvAgos49x6BZCPXqNjqfM Nud5cQVGLnN1/BFM9exlRefZ6f/3sQeI2VL7MLBEpOTxYgGkrmpXV/UVL2nm7RG9I++f a4w9/QYIvXUn26EwvbSggJnxq2sTDfS6i3eBzZGtUDKxjNHyqbFGn876PfACyZUK7I1p YOQ3YyQNQZao1cbyuRLXz+qnJwlKybtRq0Peok+ZQZag9YcHJu30HdQ8UoNfUd15zc2p tQgg== 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=SqAi37St5KAFdIAsDtEJ2iCXyFUQLmpQH3NdNXw4wF4=; b=bXjpggB65x0+oX4e5A6HvFbZM8r8D7lawgXoFC6i9zvrui1/GLKfhV3jKxNGBnHMb9 axsMIF3WePJS/Hyhpn+pnrdRDua2aYiG/dLIJ3DUROmjBcqjxQl1knXpDwyF5C3GotU4 Eea6oWuzuJ+lF9RfpdGsui0h8hIgFcoysZaTtfEAP5xnRbQw39bgNeVXIqp7iVueZBcS uEzM6ECDcZLe508OTWcM2XFJvvsFZVi7LDy551Ry9JIcT+ygV1hCJP5ZRVz/i0LNXHiw K5nRzBK4tnel6qv510Ew/alIm3/DrcdX5oey9UjGgo230jFYj9fKCWoQvZDrO7TYmN1/ SVng== X-Gm-Message-State: AElRT7EWIeEoZQ/8LD6xuHGGbvj1Wu4K6D1gWyCqgMi4Y+r7EzPnyYLB ycL+3I5CAs2ZvPaei+5s5nazBEgOk1F70/Ko8v+Aog== X-Google-Smtp-Source: AG47ELucSExgh6bIyqhyrqwk0pnqACPvZeonFX2ch2caTDF9suYrZkfpQOp66GahBHSoThmZx579yq7KZA6EJdxPP8M= X-Received: by 10.157.49.47 with SMTP id e44mr7542495otc.46.1520915768569; Mon, 12 Mar 2018 21:36:08 -0700 (PDT) MIME-Version: 1.0 Received: by 10.74.8.150 with HTTP; Mon, 12 Mar 2018 21:35:48 -0700 (PDT) In-Reply-To: References: From: Murtuza Zabuawala Date: Tue, 13 Mar 2018 10:05:48 +0530 Message-ID: Subject: Re: [pgAdmin4][RM#3140] Add service parameter To: Ashesh Vashi Cc: Dave Page , Joao De Almeida Pereira , pgadmin-hackers Content-Type: multipart/alternative; boundary="001a113daf621e8c4c056743ccdd" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --001a113daf621e8c4c056743ccdd Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Mar 13, 2018 at 9:45 AM, Ashesh Vashi wrote: > > On Tue, Mar 13, 2018 at 9:42 AM, Murtuza Zabuawala enterprisedb.com> wrote: > >> Hi Ashesh, >> >> I haven't implemented that intentionally because Khushboo is working on >> the same for SSL and our code will conflict, So once Khushboo's patch ge= ts >> committed, I'll make changes for Service file as well. >> > No - that's a bad practice. > You need to work on full feature set, not partial. > > If you have intentionally skipped that, you should have informed the list= . > =E2=80=8BI forgot to create sub-task, will keep that in mind next time onwa= rds.=E2=80=8B https://redmine.postgresql.org/issues/3195 > > -- Thanks, Ashesh > >> >> >> -- >> Regards, >> Murtuza Zabuawala >> EnterpriseDB: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company >> >> >> On Tue, Mar 13, 2018 at 9:01 AM, Ashesh Vashi < >> ashesh.vashi@enterprisedb.com> wrote: >> >>> Murtuza/Dave, >>> >>> I have to reviewed/seen the patch yet. >>> But - I have a question. >>> Have we used the service file in the external tools for backup, restore= , >>> and import/export functionalities? >>> If not - we should fix that asap. >>> >>> We had missed that during SSL support, and now - we're fixing that. >>> >>> -- >>> >>> Thanks & Regards, >>> >>> Ashesh Vashi >>> EnterpriseDB INDIA: Enterprise PostgreSQL Company >>> >>> >>> >>> *http://www.linkedin.com/in/asheshvashi* >>> >>> >>> On Tue, Mar 13, 2018 at 6:18 AM, Dave Page wrote: >>> >>>> Hi >>>> >>>> On Mon, Mar 12, 2018 at 5:18 PM, Joao De Almeida Pereira < >>>> jdealmeidapereira@pivotal.io> wrote: >>>> >>>>> Hi Dave and Murtuza, >>>>> >>>>> Regarding this patch we refactored the Javascript code so that is >>>>> lives in a different file and added some tests. >>>>> >>>>> Also we found an issue with karma-jasmine that does not allow us to >>>>> use jasmine 3.1 yet. You can find attached a patch that reverts that = commit. >>>>> >>>> >>>> Sounds good, but neither patch will apply (in fact, the Jasmine one >>>> looks entirely backwards). One of the error messages was changed in >>>> Murtuza's patch, and wasn't reflected in your update for example. >>>> >>>> Can you rebase please? >>>> >>>> Thanks. >>>> >>>> >>>>> >>>>> Thanks >>>>> Victoria && Joao >>>>> >>>>> On Mon, Mar 12, 2018 at 4:46 PM Dave Page wrote: >>>>> >>>>>> Thanks, patch applied! >>>>>> >>>>>> On Mon, Mar 12, 2018 at 3:31 AM, Murtuza Zabuawala < >>>>>> murtuza.zabuawala@enterprisedb.com> wrote: >>>>>> >>>>>>> Hi Dave, >>>>>>> >>>>>>> PFA updated patch. >>>>>>> >>>>>>> -- >>>>>>> Regards, >>>>>>> Murtuza Zabuawala >>>>>>> EnterpriseDB: http://www.enterprisedb.com >>>>>>> The Enterprise PostgreSQL Company >>>>>>> >>>>>>> >>>>>>> On Fri, Mar 9, 2018 at 9:29 PM, Murtuza Zabuawala < >>>>>>> murtuza.zabuawala@enterprisedb.com> wrote: >>>>>>> >>>>>>>> Hi Dave, >>>>>>>> >>>>>>>> I'll change the name and send you updated patch. >>>>>>>> >>>>>>>> >>>>>>>> On Fri, Mar 9, 2018 at 9:25 PM, Dave Page >>>>>>>> wrote: >>>>>>>> >>>>>>>>> HI >>>>>>>>> >>>>>>>>> On Fri, Mar 9, 2018 at 11:47 AM, Murtuza Zabuawala < >>>>>>>>> murtuza.zabuawala@enterprisedb.com> wrote: >>>>>>>>> >>>>>>>>>> Hi, >>>>>>>>>> >>>>>>>>>> PFA patch to add service parameter in server dialog. >>>>>>>>>> - Docs updated >>>>>>>>>> - Test case added for Service ID parameter >>>>>>>>>> >>>>>>>>>> Please note, >>>>>>>>>> I have extracted Connection class and Server manager class from >>>>>>>>>> our own custom Psycopg2 driver module. >>>>>>>>>> >>>>>>>>>> Patch also covers RM#3120 >>>>>>>>>> >>>>>>>>> >>>>>>>>> This patch seems a little confused. The "Service" and "Service >>>>>>>>> ID" fields from pgAdmin 3 are very different things. The Redmine = ticket >>>>>>>>> seems to be asking for the Service field (the pg_service.conf ser= vice >>>>>>>>> name), *not* Service ID (the operating system's service ID, used = to >>>>>>>>> start/stop the database server service). >>>>>>>>> >>>>>>>>> -- >>>>>>>>> Dave Page >>>>>>>>> Blog: http://pgsnake.blogspot.com >>>>>>>>> Twitter: @pgsnake >>>>>>>>> >>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com >>>>>>>>> The Enterprise PostgreSQL Company >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Dave Page >>>>>> Blog: http://pgsnake.blogspot.com >>>>>> Twitter: @pgsnake >>>>>> >>>>>> EnterpriseDB UK: http://www.enterprisedb.com >>>>>> The Enterprise PostgreSQL Company >>>>>> >>>>> >>>> >>>> >>>> -- >>>> Dave Page >>>> Blog: http://pgsnake.blogspot.com >>>> Twitter: @pgsnake >>>> >>>> EnterpriseDB UK: http://www.enterprisedb.com >>>> The Enterprise PostgreSQL Company >>>> >>> >>> >> > --001a113daf621e8c4c056743ccdd Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Tue, Mar 13, 2018 at 9:45 AM, Ashesh Vash= i <ashesh.vashi@enterprisedb.com> wrote:

On Tue, Mar 13, 2018 at 9:42 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

I haven'= ;t implemented that intentionally because Khushboo is working on the same f= or SSL and our code will conflict, So once Khushboo's patch gets commit= ted, I'll make changes for Service file as well.
No - that's a bad practice.
You need to work on full f= eature set, not partial.

If you have intentionally= skipped that, you should have informed the list.

=E2=80=8BI forgot to create s= ub-task, will keep that in mind next time onwards.=E2=80=8B
https://redmine.postgresql.org= /issues/3195
=C2=A0

-- Thanks, Ashesh=C2=A0


--
Regards,
Murtuza Zabuawala
EnterpriseDB:=C2=A0= http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Tue, Mar 13, 2018 at 9:01 AM, Ashesh Vashi <as= hesh.vashi@enterprisedb.com> wrote:
Murtuza/Dave,

I have to reviewed/seen the patch yet.
But - I have a question.
Have we used= the service file in the external tools for backup, restore, and import/exp= ort functionalities?
If not - we should fix that asap.

W= e had missed that during SSL support, and now - we're fixing that.

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company


http://www.linkedin.com/in/asheshvashi


On Tue, Mar 13, 2018 at 6:18 AM, Dave Page <= span dir=3D"ltr"><dpage@pgadmin.org> wrote:
Hi

On Mon, Mar 12, 2018 at 5:18 PM, Joao De Alm= eida Pereira <jdealmeidapereira@pivotal.io> wrote= :
Hi = Dave and Murtuza,

Regarding this patch we refactored the= Javascript code so that is lives in a different file and added some tests.=

Also we found an issue with karma-jasmine that do= es not allow us to use jasmine 3.1 yet. You can find attached a patch that = reverts that commit.

Sou= nds good, but neither patch will apply (in fact, the Jasmine one looks enti= rely backwards). One of the error messages was changed in Murtuza's pat= ch, and wasn't reflected in your update for example.

Can you rebase please?

Thanks.
=C2=A0

Thanks
=
Victoria && Joao

On M= on, Mar 12, 2018 at 4:46 PM Dave Page <dpage@pgadmin.org> wrote:
Thanks, patch appli= ed!

On Mon, = Mar 12, 2018 at 3:31 AM, Murtuza Zabuawala <murtuza.zabua= wala@enterprisedb.com> wrote:
Hi Dave,

PFA updated patch.

<= div dir=3D"ltr">
--=
Regards,
Murtuza Zabuawala
EnterpriseDB:=C2=A0http://www.enter= prisedb.com
The Enterprise PostgreSQL Company

<= /div>

On Fri, Mar 9, 2018 at 9:29 PM, Murtuza Zabu= awala <murtuza.zabuawala@enterprisedb.com> wrote:
= Hi Dave,

I'll change the name and send you updated patch.


On Fri, Mar 9, 2018 at 9:25 PM, Dave Page <= dpage@pgadmin.org> wrote:
HI

On Fri, Mar 9, 2018 at 11:47 AM, Murtuza Zabuawala = <murtuza.zabuawala@enterprisedb.com> w= rote:
Hi,

PFA pat= ch to add=C2=A0service parameter in server dialog.
- Docs updated
- Test case added= for Service ID parameter

Please note,
I have extracted Connection class and Serve= r manager class from our own custom Psycopg2 driver module.

Patch also covers = RM#3120

=C2=A0This patch seems = a little confused. The "Service" and "Service ID" field= s from pgAdmin 3 are very different things. The Redmine ticket seems to be = asking for the Service field (the pg_service.conf service name), *not* Serv= ice ID (the operating system's service ID, used to start/stop the datab= ase server service).

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQ= L Company





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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Co= mpany



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

EnterpriseDB UK: http://www.enterprised= b.com
The Enterprise PostgreSQL Company

--001a113daf621e8c4c056743ccdd--