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

I'm suggesting that it be pushed

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