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-08 09:39:08
Message-ID: CAKi=nnfBp-PqW84-CHsw6h37Fmc5h2s4b5xh3c_c7-0qyNBdsA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Aditya,

I have fixed this.

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&>
08/09/23,
15:08:19

On Fri, 8 Sept 2023 at 10:50, Sahil Harpal <sahilharpal1234(at)gmail(dot)com>
wrote:

> Oh yeah! Maybe because I rebased the branch. I'll try to fix this.
>
> On Fri, Sep 8, 2023, 9:04 AM Aditya Toshniwal <
> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>
>> Hi Sahil,
>>
>> Your PR is showing 135 file changes and a lot of commits which shouldn't
>> have appeared on your PR.
>> It is very difficult to identify your changes. Can you please check once?
>>
>> On Fri, Sep 8, 2023 at 1:13 AM Sahil Harpal <sahilharpal1234(at)gmail(dot)com>
>> wrote:
>>
>>> 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"
>>>>
>>>
>>
>> --
>> 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 dependabot[bot] 2023-09-11 10:16:18 [pgadmin-org/pgadmin4] 01e4ed: Python dependency: Bump eslint from 8.47.0 to 8.49...
Previous Message Sahil Harpal 2023-09-08 05:20:12 Re: Pgadmin4 System Stats Extension Design