Thread: [pgAdmin4][RM#3072] Make pgagent job history rows configurable
Hi,
PFA patch which allow user to configure how many rows they wish to display for any pgagent jobs on statistics panel.
--
Regards,
Attachment
Hi
--
On Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,PFA patch which allow user to configure how many rows they wish to display for any pgagent jobs on statistics panel.
I think this is essentially good, however, I'm really not happy with the preference name and category. In general, I'd suggest that before creating patches in the future we should confirm naming etc on the mailing list, as I often end up changing wording and then requiring new screenshots etc.
In this case, I really don't like that we've added another category, and quite a specific one at that. I would suggest we move it to Browser -> Properties and name it "Maximum job history rows" with a description of "The maximum number of history rows to show on the Statistics tab for pgAgent jobs."
Thoughts?
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Thu, Apr 5, 2018 at 4:43 PM, Dave Page <dpage@pgadmin.org> wrote:
HiOn Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: Hi,PFA patch which allow user to configure how many rows they wish to display for any pgagent jobs on statistics panel.I think this is essentially good, however, I'm really not happy with the preference name and category. In general, I'd suggest that before creating patches in the future we should confirm naming etc on the mailing list, as I often end up changing wording and then requiring new screenshots etc.
+1
In this case, I really don't like that we've added another category, and quite a specific one at that. I would suggest we move it to Browser -> Properties and name it "Maximum job history rows" with a description of "The maximum number of history rows to show on the Statistics tab for pgAgent jobs."Thoughts?--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
HiOn Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: Hi,PFA patch which allow user to configure how many rows they wish to display for any pgagent jobs on statistics panel.I think this is essentially good, however, I'm really not happy with the preference name and category. In general, I'd suggest that before creating patches in the future we should confirm naming etc on the mailing list, as I often end up changing wording and then requiring new screenshots etc.
Ok
In this case, I really don't like that we've added another category, and quite a specific one at that. I would suggest we move it to Browser -> Properties and name it "Maximum job history rows" with a description of "The maximum number of history rows to show on the Statistics tab for pgAgent jobs."
I'll change it and resend the patch.
Thoughts?--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Hi Dave,
Please find update patch.
--
Regards,
On Thu, Apr 5, 2018 at 4:52 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
HiOn Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: Hi,PFA patch which allow user to configure how many rows they wish to display for any pgagent jobs on statistics panel.I think this is essentially good, however, I'm really not happy with the preference name and category. In general, I'd suggest that before creating patches in the future we should confirm naming etc on the mailing list, as I often end up changing wording and then requiring new screenshots etc.OkIn this case, I really don't like that we've added another category, and quite a specific one at that. I would suggest we move it to Browser -> Properties and name it "Maximum job history rows" with a description of "The maximum number of history rows to show on the Statistics tab for pgAgent jobs."I'll change it and resend the patch.Thoughts?--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachment
Re: [pgAdmin4][RM#3072] Make pgagent job history rows configurable
From
Joao De Almeida Pereira
Date:
Hi Murtuza,
Generally the patch looks good passes all CI but the linter fails:
/tmp/build/4a5630c2/pivotal-rm-3072/web /tmp/build/4a5630c2./pgadmin/browser/register_browser_preferences.py:11: [E302] expected 2 blank lines, found 11 E302 expected 2 blank lines, found 11
Did not test the functionality itself ....
Thanks
Joao
On Thu, Apr 5, 2018 at 8:09 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,Please find update patch.--Regards,On Thu, Apr 5, 2018 at 4:52 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:HiOn Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:Hi,PFA patch which allow user to configure how many rows they wish to display for any pgagent jobs on statistics panel.I think this is essentially good, however, I'm really not happy with the preference name and category. In general, I'd suggest that before creating patches in the future we should confirm naming etc on the mailing list, as I often end up changing wording and then requiring new screenshots etc.OkIn this case, I really don't like that we've added another category, and quite a specific one at that. I would suggest we move it to Browser -> Properties and name it "Maximum job history rows" with a description of "The maximum number of history rows to show on the Statistics tab for pgAgent jobs."I'll change it and resend the patch.Thoughts?--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Murtuza, please send me the screenshot as a PNG. I've been tweaking the code and will recreate the patch with my changes.
On Thu, Apr 5, 2018 at 2:53 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi Murtuza,Generally the patch looks good passes all CI but the linter fails:/tmp/build/4a5630c2/pivotal-rm-3072/web /tmp/build/4a5630c2 ./pgadmin/browser/register_browser_preferences.py:11: [E302] expected 2 blank lines, found 1 1 E302 expected 2 blank lines, found 11Did not test the functionality itself ....ThanksJoaoOn Thu, Apr 5, 2018 at 8:09 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: Hi Dave,Please find update patch.--Regards,On Thu, Apr 5, 2018 at 4:52 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: HiOn Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: Hi,PFA patch which allow user to configure how many rows they wish to display for any pgagent jobs on statistics panel.I think this is essentially good, however, I'm really not happy with the preference name and category. In general, I'd suggest that before creating patches in the future we should confirm naming etc on the mailing list, as I often end up changing wording and then requiring new screenshots etc.OkIn this case, I really don't like that we've added another category, and quite a specific one at that. I would suggest we move it to Browser -> Properties and name it "Maximum job history rows" with a description of "The maximum number of history rows to show on the Statistics tab for pgAgent jobs."I'll change it and resend the patch.Thoughts?--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Hi Dave,
Please find attached PNG.
On Thu, Apr 5, 2018 at 7:25 PM, Dave Page <dpage@pgadmin.org> wrote:
Murtuza, please send me the screenshot as a PNG. I've been tweaking the code and will recreate the patch with my changes.On Thu, Apr 5, 2018 at 2:53 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:Hi Murtuza,Generally the patch looks good passes all CI but the linter fails:/tmp/build/4a5630c2/pivotal-rm-3072/web /tmp/build/4a5630c2 ./pgadmin/browser/register_browser_preferences.py:11: [E302] expected 2 blank lines, found 1 1 E302 expected 2 blank lines, found 11Did not test the functionality itself ....
@Joao,
I have included the test for Stats call which will cover the newly added functionality.
ThanksJoaoOn Thu, Apr 5, 2018 at 8:09 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: Hi Dave,Please find update patch.--Regards,On Thu, Apr 5, 2018 at 4:52 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: HiOn Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: Hi,PFA patch which allow user to configure how many rows they wish to display for any pgagent jobs on statistics panel.I think this is essentially good, however, I'm really not happy with the preference name and category. In general, I'd suggest that before creating patches in the future we should confirm naming etc on the mailing list, as I often end up changing wording and then requiring new screenshots etc.OkIn this case, I really don't like that we've added another category, and quite a specific one at that. I would suggest we move it to Browser -> Properties and name it "Maximum job history rows" with a description of "The maximum number of history rows to show on the Statistics tab for pgAgent jobs."I'll change it and resend the patch.Thoughts?--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachment
Sorry Murtuza - I'm mixing patches up here. It's the sort/filter one I need the image for please.
Which means this patch needs re-creating as well please.
On Thu, Apr 5, 2018 at 3:03 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,Please find attached PNG.On Thu, Apr 5, 2018 at 7:25 PM, Dave Page <dpage@pgadmin.org> wrote:Murtuza, please send me the screenshot as a PNG. I've been tweaking the code and will recreate the patch with my changes.On Thu, Apr 5, 2018 at 2:53 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:Hi Murtuza,Generally the patch looks good passes all CI but the linter fails:/tmp/build/4a5630c2/pivotal-rm-3072/web /tmp/build/4a5630c2 ./pgadmin/browser/register_browser_preferences.py:11: [E302] expected 2 blank lines, found 1 1 E302 expected 2 blank lines, found 11Did not test the functionality itself ....@Joao,I have included the test for Stats call which will cover the newly added functionality.ThanksJoaoOn Thu, Apr 5, 2018 at 8:09 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: Hi Dave,Please find update patch.--Regards,On Thu, Apr 5, 2018 at 4:52 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: HiOn Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: Hi,PFA patch which allow user to configure how many rows they wish to display for any pgagent jobs on statistics panel.I think this is essentially good, however, I'm really not happy with the preference name and category. In general, I'd suggest that before creating patches in the future we should confirm naming etc on the mailing list, as I often end up changing wording and then requiring new screenshots etc.OkIn this case, I really don't like that we've added another category, and quite a specific one at that. I would suggest we move it to Browser -> Properties and name it "Maximum job history rows" with a description of "The maximum number of history rows to show on the Statistics tab for pgAgent jobs."I'll change it and resend the patch.Thoughts?--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Hi
The description for the Preferences field doesn't seem to be there in the code or the screenshot. I suggested below using: "The maximum number of history rows to show on the Statistics tab for pgAgent jobs."
Thanks.
--
On Thu, Apr 5, 2018 at 1:09 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,Please find update patch.--Regards,On Thu, Apr 5, 2018 at 4:52 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: HiOn Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: Hi,PFA patch which allow user to configure how many rows they wish to display for any pgagent jobs on statistics panel.I think this is essentially good, however, I'm really not happy with the preference name and category. In general, I'd suggest that before creating patches in the future we should confirm naming etc on the mailing list, as I often end up changing wording and then requiring new screenshots etc.OkIn this case, I really don't like that we've added another category, and quite a specific one at that. I would suggest we move it to Browser -> Properties and name it "Maximum job history rows" with a description of "The maximum number of history rows to show on the Statistics tab for pgAgent jobs."I'll change it and resend the patch.Thoughts?--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Hi,
Please find updated patch which fixes PEP8 issue, added help string for preference dialog option & updated screenshot.
--
Regards,
On Thu, Apr 5, 2018 at 7:56 PM, Dave Page <dpage@pgadmin.org> wrote:
Sorry Murtuza - I'm mixing patches up here. It's the sort/filter one I need the image for please.Which means this patch needs re-creating as well please.On Thu, Apr 5, 2018 at 3:03 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: Hi Dave,Please find attached PNG.On Thu, Apr 5, 2018 at 7:25 PM, Dave Page <dpage@pgadmin.org> wrote:Murtuza, please send me the screenshot as a PNG. I've been tweaking the code and will recreate the patch with my changes.On Thu, Apr 5, 2018 at 2:53 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:Hi Murtuza,Generally the patch looks good passes all CI but the linter fails:/tmp/build/4a5630c2/pivotal-rm-3072/web /tmp/build/4a5630c2 ./pgadmin/browser/register_browser_preferences.py:11: [E302] expected 2 blank lines, found 1 1 E302 expected 2 blank lines, found 11Did not test the functionality itself ....@Joao,I have included the test for Stats call which will cover the newly added functionality.ThanksJoaoOn Thu, Apr 5, 2018 at 8:09 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: Hi Dave,Please find update patch.--Regards,On Thu, Apr 5, 2018 at 4:52 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: HiOn Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: Hi,PFA patch which allow user to configure how many rows they wish to display for any pgagent jobs on statistics panel.I think this is essentially good, however, I'm really not happy with the preference name and category. In general, I'd suggest that before creating patches in the future we should confirm naming etc on the mailing list, as I often end up changing wording and then requiring new screenshots etc.OkIn this case, I really don't like that we've added another category, and quite a specific one at that. I would suggest we move it to Browser -> Properties and name it "Maximum job history rows" with a description of "The maximum number of history rows to show on the Statistics tab for pgAgent jobs."I'll change it and resend the patch.Thoughts?--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachment
Thanks, applied.
On Fri, Apr 6, 2018 at 5:53 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,Please find updated patch which fixes PEP8 issue, added help string for preference dialog option & updated screenshot.--Regards,On Thu, Apr 5, 2018 at 7:56 PM, Dave Page <dpage@pgadmin.org> wrote:Sorry Murtuza - I'm mixing patches up here. It's the sort/filter one I need the image for please.Which means this patch needs re-creating as well please.On Thu, Apr 5, 2018 at 3:03 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: Hi Dave,Please find attached PNG.On Thu, Apr 5, 2018 at 7:25 PM, Dave Page <dpage@pgadmin.org> wrote:Murtuza, please send me the screenshot as a PNG. I've been tweaking the code and will recreate the patch with my changes.On Thu, Apr 5, 2018 at 2:53 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:Hi Murtuza,Generally the patch looks good passes all CI but the linter fails:/tmp/build/4a5630c2/pivotal-rm-3072/web /tmp/build/4a5630c2 ./pgadmin/browser/register_browser_preferences.py:11: [E302] expected 2 blank lines, found 1 1 E302 expected 2 blank lines, found 11Did not test the functionality itself ....@Joao,I have included the test for Stats call which will cover the newly added functionality.ThanksJoaoOn Thu, Apr 5, 2018 at 8:09 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: Hi Dave,Please find update patch.--Regards,On Thu, Apr 5, 2018 at 4:52 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: HiOn Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote: Hi,PFA patch which allow user to configure how many rows they wish to display for any pgagent jobs on statistics panel.I think this is essentially good, however, I'm really not happy with the preference name and category. In general, I'd suggest that before creating patches in the future we should confirm naming etc on the mailing list, as I often end up changing wording and then requiring new screenshots etc.OkIn this case, I really don't like that we've added another category, and quite a specific one at that. I would suggest we move it to Browser -> Properties and name it "Maximum job history rows" with a description of "The maximum number of history rows to show on the Statistics tab for pgAgent jobs."I'll change it and resend the patch.Thoughts?--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company