Re: Pgadmin4 System Stats Extension Design

From: Sahil Harpal <sahilharpal1234(at)gmail(dot)com>
To: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: Pgadmin4 System Stats Extension Design
Date: 2023-09-07 19:43:06
Message-ID: CAKi=nneajJeG3n5HC=ne7MrcudtZyOccVo0WkbWmE=2XpWjyaQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Aditya,

Sorry for the delay; I've been a bit busy lately. I have made all the
requested changes. Could you please review it?

Thanks,
Sahil

On Mon, 4 Sept 2023 at 11:17, Aditya Toshniwal <
aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:

> Hi Sahil,
>
> I have replied to the PR.
>
> On Sun, Sep 3, 2023 at 2:31 AM Sahil Harpal <sahilharpal1234(at)gmail(dot)com>
> wrote:
>
>> Hi Aditya,
>>
>> I have made almost all of the requested changes and pushed the latest
>> code. I just need a bit of clarification for a couple of suggestions that I
>> have posted in the reviews.
>>
>> Thank you,
>> Sahil
>>
>>
>> On Thu, 31 Aug 2023 at 17:20, Aditya Toshniwal <
>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Sahil,
>>>
>>> OK fine. We will check it later. Not priority. Please also fix the
>>> review raised on PR.
>>>
>>> On Thu, Aug 31, 2023, 16:17 Sahil Harpal <sahilharpal1234(at)gmail(dot)com>
>>> wrote:
>>>
>>>> Hi Aditya,
>>>> I have made all these changes except the StreamingChart issue. I tried
>>>> filling an array with null values inside the StreamingChart component while
>>>> initializing initialState but still the issue is not resolved.
>>>> I have made following changes:
>>>>
>>>>> const initialState = [
>>>>> Array.from(new Array(xRange).keys()),
>>>>> ...(data.datasets?.map((d)=>{
>>>>> let nullValues = new Array(xRange - d.data.length).fill(null);
>>>>> let ret = [...nullValues, ...d.data];
>>>>> ret.reverse();
>>>>> return ret;
>>>>> })??{}),
>>>>> ];
>>>>
>>>>
>>>> It works fine if we initialize the data array with the null values but
>>>> I'm not getting why this is not working.
>>>>
>>>> Thank you,
>>>> Sahil
>>>>
>>>>
>>>> [image: Mailtrack]
>>>> <https://mailtrack.io?utm_source=gmail&utm_medium=signature&utm_campaign=signaturevirality11&> Sender
>>>> notified by
>>>> Mailtrack
>>>> <https://mailtrack.io?utm_source=gmail&utm_medium=signature&utm_campaign=signaturevirality11&> 31/08/23,
>>>> 16:14:27
>>>>
>>>> On Mon, 28 Aug 2023 at 10:44, Aditya Toshniwal <
>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi Sahil,
>>>>>
>>>>> I have few observations. You have added separate titles for graphs and
>>>>> other tabular data. This is inconsistent with existing UI.
>>>>> For example,
>>>>> [image: image.png]
>>>>> like here:
>>>>> [image: image.png]
>>>>>
>>>>> [image: Screenshot 2023-08-28 at 10.28.59 AM.png]
>>>>> like here:
>>>>> [image: image.png]
>>>>>
>>>>> [image: image.png]
>>>>>
>>>>> [image: image.png]
>>>>>
>>>>> The dashboard goes blank when I change refresh rates.
>>>>>
>>>>> [image: image.png]
>>>>>
>>>>> And regarding filling with nulls to fix the reversing issue of graph -
>>>>> you can do it in StreamingChart itself when setting initialState var, as it
>>>>> is StreamingChart's responsibility to do it.
>>>>> Next review will be on the PR directly.
>>>>>
>>>>>
>>>>> On Sun, Aug 27, 2023 at 5:58 PM Sahil Harpal <
>>>>> sahilharpal1234(at)gmail(dot)com> wrote:
>>>>>
>>>>>> Hello everyone,
>>>>>>
>>>>>> I have raised the PR
>>>>>> <https://github.com/pgadmin-org/pgadmin4/pull/6721>.
>>>>>> I would like to request you all to review the changes and provide
>>>>>> your valuable feedback. Your insights and suggestions would be invaluable
>>>>>> in ensuring the quality and accuracy of the codebase.
>>>>>>
>>>>>> Thank you,
>>>>>> Sahil
>>>>>>
>>>>>> On Sun, 27 Aug 2023 at 07:13, Khushboo Vashi <
>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Sat, 26 Aug 2023, 11:36 Sahil Harpal, <sahilharpal1234(at)gmail(dot)com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hi Khushboo,
>>>>>>>>
>>>>>>>> On Mon, 21 Aug 2023 at 10:03, Khushboo Vashi <
>>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> Sahil, once the issues get resolved, please raise the PR and we
>>>>>>>>> will do the final review there.
>>>>>>>>>
>>>>>>>> Could you please tell me to which branch I should raise the PR?
>>>>>>>>
>>>>>>> Master branch
>>>>>>>
>>>>>>>> Also, should I remove the code responsible for the static
>>>>>>>> DonutChart of process information?
>>>>>>>>
>>>>>>>> Thank you,
>>>>>>>> Sahil
>>>>>>>>
>>>>>>>>>
>>>>>
>>>>> --
>>>>> Thanks,
>>>>> Aditya Toshniwal
>>>>> pgAdmin Hacker | Sr. Software Architect | *enterprisedb.com*
>>>>> <https://www.enterprisedb.com/>
>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>
>>>>
>
> --
> Thanks,
> Aditya Toshniwal
> pgAdmin Hacker | Sr. Software Architect | *enterprisedb.com*
> <https://www.enterprisedb.com/>
> "Don't Complain about Heat, Plant a TREE"
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Aditya Toshniwal 2023-09-08 03:33:51 Re: Pgadmin4 System Stats Extension Design
Previous Message Anil Sahoo 2023-09-07 06:24:15 [pgadmin-org/pgadmin4] 324039: Enhancement of properties tab for column's collect...