Re: Pgadmin4 System Stats Extension Design

From: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>
To: Sahil Harpal <sahilharpal1234(at)gmail(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>, Dave Page <dpage(at)pgadmin(dot)org>, Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
Subject: Re: Pgadmin4 System Stats Extension Design
Date: 2023-09-04 05:47:18
Message-ID: CAM9w-_=LrLS1G48noXv+Oa9vd-9sYCoP-enOX_DUcxF2eitiFQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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 sprayzcs 2023-09-04 07:36:29 [pgadmin-org/pgadmin4] 5cf9de: Change grep regex in the docker's entrypoint to fi...
Previous Message Sahil Harpal 2023-09-02 21:00:42 Re: Pgadmin4 System Stats Extension Design