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

From: Dave Cramer <davecramer(at)gmail(dot)com>
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>
Subject: Re: [pgadmin4][Patch] Greenplum specific DDL and Dashboard display
Date: 2017-08-22 20:56:49
Message-ID: CADK3HHJa+xPfAwuV4uKjnP-5gf+Bw8iF33jnxBrtWssGN_pAuA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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
>>>>>
>>>>
>>>>
>>>
>>
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Surinder Kumar 2017-08-23 04:38:25 [Patch][pgAdmin4][pgAgent]: RM_2656 - Unable to select start and end time for scheduler from calendar
Previous Message Dave Cramer 2017-08-22 20:41:29 Re: [pgadmin4][Patch] Greenplum specific DDL and Dashboard display