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

Hi Akshay,

On Fri, Feb 4, 2022 at 2:53 PM Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
wrote:

> 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?
>
> Done

>
> - Change the title of the server dialog from 'Create Server' to
> 'Register Server'. Change the documentation and respective screenshots as
> well.
>
> Done

>
> - 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.
>
> Fixed

>
> - Can we use the AWS standard icon, we had a discussion but I forgot.
>
> We had a discussion about if I could find the font awesome icon for the
same but I couldn't.

>
> - Rename the label from 'Default Region' to 'Region' as it is not the
> default one.
>
> Done

>
> - Region name should be displayed in full like (Africa (Cape town)
> af-south1, Asia Pacific (Mumbai) ap-south1.
>
> I couldn't find any API for that, so haven't done this, but I can make a
hard code list for the same, if everyone agrees.

>
> - The label should be corrected 'AWS secret access Key' to 'AWS secret
> access key'.
>
> Fixed

>
> - When validating the credentials the message 'Validating
> Credentials...' is Info not an error it should be displayed in Blue color.
>
> done

>
> - Fetching instance type taking some time can we show spinner until
> the response comes?
>
> I tried but couldn't do it

>
> - For *Burstable classes,* instance type not coming.
>
> Fixed

>
> - Instance type should have more info like CPU, RAM etc.
>
> I wanted the same way you suggested but couldn't find any API which
provides these details.

>
> - If there is no change in the port field or type 5432, we get 'Error
> while saving cloud wizard data: 'aws_db_port'.
>
> Fixed

>
> - 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.
>
> Fixed

>
> - Process logs come after the completion of the process. Is it
> possible to display ongoing logs if any?
>
> I tried a lot but couldn't fix it recently but will try again.

>
> - After completion of the deployment process, the server deployment
> icon is not changed until we refresh the browser tree.
>
> Fixed

>
> - 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.
>
> Fixed

> *Code:*
>
> - Fixed PEP8.
>
> Fixed

>
> - 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.
>
> The outside folder is holding the code which will deploy the instance on
the cloud and we run that as a separate process, so it is outside the
pgAdmin module but inside the web directory.

The misc folder contains the code which is used to get the cloud details
on the UI and nothing to do with the deployment.

>
> - Some new files inside *misc/cloud* and '1586db67b98e_.py' still have
> copyright headers as of 2013-2021 please correct those.
>
> Fixed

>
> - Fix SonarQube issues (*newly introduced only*) in
> cloud.js, cloud_db_details_schema.ui.js, CloudWizard.jsx, processes.py.
>
> Fixed

>
> - 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.
>
> Fixed

>
> -
> - The size of the screenshots in the documentation is not consistent
> and the background color should be other than white while taking a
> screenshot.
>
> Fixed

>
> - In "pgacloud/providers/rds.py" file
> '/Users/khushboovashi/.aws/credentials'' sting should not be
> hardcoaded.
>
> Fixed, I left it by mistake .

Thanks,
Khushboo

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