Re: [pgadmin4][Patch] Greenplum specific DDL and Dashboard display

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
Cc: Dave Cramer <davecramer(at)gmail(dot)com>, Teng Zhang <tezhang(at)pivotal(dot)io>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Dave Page <dpage(at)pgadmin(dot)org>, Jing Li <jingli(at)pivotal(dot)io>
Subject: Re: [pgadmin4][Patch] Greenplum specific DDL and Dashboard display
Date: 2017-08-25 12:27:09
Message-ID: CANxoLDftNP9v2yPF0j+=4-whqiqbqFz2iwxvOiG+esfCZArVug@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Thanks patch applied. I haven't tested it on GPDB.

On Fri, Aug 25, 2017 at 4:55 PM, Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com
> wrote:

> On Fri, Aug 25, 2017 at 4:53 PM, Dave Cramer <davecramer(at)gmail(dot)com> wrote:
>
>> I'm suggesting that it be pushed
>>
> Akshay is already on to it, and currently reviewing it.
> Will push it once get the confirmation.
>
> -- Thanks, Ashesh
>
>>
>> Dave Cramer
>>
>> On 24 August 2017 at 23:00, Teng Zhang <tezhang(at)pivotal(dot)io> wrote:
>>
>>> Sure, you can get as much as you like.
>>> Thanks
>>>
>>> ---------- Forwarded message ----------
>>> From: Dave Cramer <davecramer(at)gmail(dot)com>
>>> Date: Thu, Aug 24, 2017 at 8:34 PM
>>> Subject: Re: [pgadmin4][Patch] Greenplum specific DDL and Dashboard
>>> display
>>> To: Teng Zhang <tezhang(at)pivotal(dot)io>
>>> Cc: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>, pgadmin-hackers <
>>> pgadmin-hackers(at)postgresql(dot)org>, Dave Page <dpage(at)pgadmin(dot)org>, Jing Li
>>> <jingli(at)pivotal(dot)io>
>>>
>>>
>>> Can we get some movement on this patch? This seems like a more sane way
>>> to go to support different "products"
>>>
>>> Dave Cramer
>>>
>>> On 22 August 2017 at 16:56, Dave Cramer <davecramer(at)gmail(dot)com> wrote:
>>>
>>>>
>>>> On 22 August 2017 at 16:41, Dave Cramer <davecramer(at)gmail(dot)com> wrote:
>>>>
>>>>> Ok,
>>>>>
>>>>> Surely this :
>>>>>
>>>>> self.table_template_path = 'table/sql/' + (
>>>>> + '#{0}#{1}#'.format(server_type, ver)
>>>>> + if server_type == 'gpdb' else
>>>>> + '#{0}#'.format(ver)
>>>>> + )
>>>>>
>>>>> could be written in a more readable manner ??
>>>>>
>>>>>
>>>>>
>>>> Apologies, after reading a bit, this is apparently idiomatic python.
>>>>
>>>> Please ignore
>>>>
>>>>
>>>>> Dave Cramer
>>>>>
>>>>> On 22 August 2017 at 14:25, Dave Cramer <davecramer(at)gmail(dot)com> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> I've been able to get back to this and test it. So far so good. It
>>>>>> applies more or less cleanly against 1.6 and everything I've tried so far
>>>>>> works
>>>>>>
>>>>>> I'll update more as I test it.
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>> Dave Cramer
>>>>>>
>>>>>> On 21 August 2017 at 05:29, Teng Zhang <tezhang(at)pivotal(dot)io> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Thanks for the review, here is a fixed patch working for GBDP which
>>>>>>> shows the appropriate graphs.
>>>>>>> In this fix, we toke out the changes to diver/psycopg2 and
>>>>>>> implemented the greenplum version checking process in the ppas way
>>>>>>> mentioned by Dave Cramer.
>>>>>>>
>>>>>>> Regards,
>>>>>>> Teng Zhang & Hao Wang
>>>>>>>
>>>>>>> On Mon, Aug 21, 2017 at 3:55 PM, Ashesh Vashi <
>>>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> On Mon, Aug 21, 2017 at 1:23 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Ashesh, do you have a recommended way to do this?
>>>>>>>>>
>>>>>>>>> I haven't looked at the patch, but I assume it adds a database
>>>>>>>>> driver module for GPDB?
>>>>>>>>>
>>>>>>>> I have not looked at the patch yet.
>>>>>>>> I will take a look at it.
>>>>>>>>
>>>>>>>> --
>>>>>>>>
>>>>>>>> Thanks & Regards,
>>>>>>>>
>>>>>>>> Ashesh Vashi
>>>>>>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>>>>>>>> <http://www.enterprisedb.com/>
>>>>>>>>
>>>>>>>>
>>>>>>>> *http://www.linkedin.com/in/asheshvashi*
>>>>>>>> <http://www.linkedin.com/in/asheshvashi>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Mon, Aug 21, 2017 at 8:50 AM, Jing Li <jingli(at)pivotal(dot)io>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> Hi Dave,
>>>>>>>>>>
>>>>>>>>>> Since we're hoping to get this change working for GPDB we've
>>>>>>>>>> currently using this method to detect if it's gpdb and show the appropriate
>>>>>>>>>> graphs. Right now it displays errors on the dashboard if it's connected to
>>>>>>>>>> a gpdb server.
>>>>>>>>>> For this patch specifically, the goal is to improve the
>>>>>>>>>> experience for greenplum users so they can get the same information as
>>>>>>>>>> someone connected to a postgres server.
>>>>>>>>>>
>>>>>>>>>> I do agree that this is a bigger discussion about how we handle
>>>>>>>>>> behavior change overall if it's regular postgres or something else. Let's
>>>>>>>>>> talk about how we can restructure this behavior in a wider context. Are you
>>>>>>>>>> open to meeting about it?
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> ~Jing
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Fri, Aug 18, 2017 5:37 AM, Dave Cramer davecramer(at)gmail(dot)com
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi Violet.
>>>>>>>>>>>
>>>>>>>>>>> I don't really like the way this has been implemented. It adds a
>>>>>>>>>>> variable which is only used for gpdb.
>>>>>>>>>>>
>>>>>>>>>>> There are other places in the code where the behaviour is
>>>>>>>>>>> changed if the server is ppas or regular postgres.
>>>>>>>>>>>
>>>>>>>>>>> Candidly I think all of this needs restructuring.
>>>>>>>>>>>
>>>>>>>>>>> Dave Cramer
>>>>>>>>>>>
>>>>>>>>>>> On 15 August 2017 at 23:29, Violet Cheng <vcheng(at)pivotal(dot)io>
>>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> Any comment on this patch? If no, will it be committed soon?
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Violet
>>>>>>>>>>>
>>>>>>>>>>> On Wed, Aug 9, 2017 at 12:05 PM, Sarah McAlear <
>>>>>>>>>>> smcalear(at)pivotal(dot)io> wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi Hackers!
>>>>>>>>>>>
>>>>>>>>>>> This patch enables Greenplum users to see the same charts on the
>>>>>>>>>>> dashboard as postgres users. It also adds some additional information to
>>>>>>>>>>> the DDL that is Greenplum specific and necessary to create a new table.
>>>>>>>>>>>
>>>>>>>>>>> Thanks!
>>>>>>>>>>> Sarah
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Dave Page
>>>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>
>>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>
>

--
*Akshay Joshi*
*Principal Software Engineer *

*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Robert Eckhardt 2017-08-25 12:39:32 Re: [pgadmin4][Patch] Greenplum specific DDL and Dashboard display
Previous Message Akshay Joshi 2017-08-25 12:25:09 pgAdmin 4 commit: Greenplum specific DDL and Dashboard display changes.