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

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

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