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>, 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-02 21:00:42
Message-ID: CAKi=nnc+L62NPnxZV_YKwB_GC5GXyC8w4XGLbNTq+HduaXkumA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Aditya Toshniwal 2023-09-04 05:47:18 Re: Pgadmin4 System Stats Extension Design
Previous Message Aditya Toshniwal 2023-08-31 11:49:45 Re: Pgadmin4 System Stats Extension Design