Re: [pgAdmin4][Patch] - PostgreSQL deployment on Amazon RDS

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch] - PostgreSQL deployment on Amazon RDS
Date: 2022-02-04 09:22:53
Message-ID: CANxoLDfUPPZOdxyL6FBdyGETVo96G5hoUVm2oBeP0KSkUtcvsw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Khushboo

Following are the review comments:

*GUI:*

- Can we rename the menu 'Deploy and Register a Cloud Instance...' to
only 'Deploy Cloud Instance...' as we already have a Register menu?
- Change the title of the server dialog from 'Create Server' to
'Register Server'. Change the documentation and respective screenshots as
well.
- Every time the wizard is open, I get "vendor.others.js?ver=60400:2
Uncaught (in promise) TypeError: Cannot read properties of undefined
(reading 'protocol')" in the browser console.
- Can we use the AWS standard icon, we had a discussion but I forgot.
- Rename the label from 'Default Region' to 'Region' as it is not the
default one.
- Region name should be displayed in full like (Africa (Cape town)
af-south1, Asia Pacific (Mumbai) ap-south1.
- The label should be corrected 'AWS secret access Key' to 'AWS secret
access key'.
- When validating the credentials the message 'Validating
Credentials...' is Info not an error it should be displayed in Blue color.
- Fetching instance type taking some time can we show spinner until the
response comes?
- For *Burstable classes,* instance type not coming.
- Instance type should have more info like CPU, RAM etc.
- If there is no change in the port field or type 5432, we get 'Error
while saving cloud wizard data: 'aws_db_port'.
- Public IP Range is optional, as per implementation, but when it is not
provided then we get 'Error while saving cloud wizard data:
'aws_public_ip'. The error message should be proper and add validation at
the wizard page itself if Public IP Range is not optional.
- Process logs come after the completion of the process. Is it possible
to display ongoing logs if any?
- After completion of the deployment process, the server deployment icon
is not changed until we refresh the browser tree.
- On refreshing the browser tree if try to connect to the newly deployed
server, no action will be performed, try double click or the 'Connect
Server' menu.

*Code:*

- Fixed PEP8.
- pgacloud folder should be inside the pgadmin folder. Any specific
reason why it is outside of the code hierarchy?
- Similarly why *cloud* folder placed inside the *misc* folder, can't
this be a part of pgacloud? pgacloud is a complete module itself. Files
belonging to Cloud deployment will be used by only that module.
- Some new files inside *misc/cloud* and '1586db67b98e_.py' still have
copyright headers as of 2013-2021 please correct those.
- Fix SonarQube issues (*newly introduced only*) in
cloud.js, cloud_db_details_schema.ui.js, CloudWizard.jsx, processes.py.
- There are two files *rds.py* with the same name, if possible/feasible
change the name of one of the files.
- "*_cloud.scss*", and "cloud.js " files contain references of grant
wizard. Please correct those.
- The size of the screenshots in the documentation is not consistent and
the background color should be other than white while taking a screenshot.
- In "pgacloud/providers/rds.py" file
'/Users/khushboovashi/.aws/credentials'' sting should not be hardcoaded.

*Note*: Not tested each and every scenario.

On Thu, Feb 3, 2022 at 12:57 PM Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

> Hi Akshay,
>
> Please find the attached updated patch. Please check, if it is working
> then I will send it to the hackers.
>
> Thanks,
> Khushboo
>
> On Wed, Feb 2, 2022 at 5:53 PM Khushboo Vashi <
> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>> Hi,
>>
>> Please find the attached updated patch. Fixes for Python-3.10.
>>
>> Thanks,
>> Khushboo
>>
>> On Mon, Jan 31, 2022 at 12:39 PM Khushboo Vashi <
>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi,
>>>
>>> Please find the attached updated patch with some UI changes.
>>>
>>> Thanks,
>>> Khushboo
>>>
>>> On Tue, Jan 25, 2022 at 12:28 PM Khushboo Vashi <
>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hello,
>>>>
>>>> Please find the attached patch for the RM #6717 - PostgreSQL deployment
>>>> on Cloud (RDS, Azure, Starlight).
>>>>
>>>> This patch includes only RDS cloud deployment.
>>>>
>>>> Thanks,
>>>> Khushboo
>>>>
>>>>

--
*Thanks & Regards*
*Akshay Joshi*
*pgAdmin Hacker | Principal Software Architect*
*EDB Postgres <http://edbpostgres.com>*

*Mobile: +91 976-788-8246*

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2022-02-04 09:51:22 pgAdmin 4 commit: Ensure that layout should not be reset if a query too
Previous Message Aditya Toshniwal 2022-02-04 09:02:36 [pgAdmin][RM7154] Layout change is reset on refresh