From: | Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com> |
---|---|
To: | Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com> |
Cc: | pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org> |
Subject: | Re: [pgAdmin4][Patch] - PostgreSQL deployment on Amazon RDS |
Date: | 2022-02-14 11:57:37 |
Message-ID: | CANxoLDcfcaibwDuDuwi1_EpxoJUpVOp0wQw0k+VdLb-=J6SqLQ@mail.gmail.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
Thanks, the patch applied.
On Mon, Feb 14, 2022 at 5:14 PM Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
> Hi,
>
> On Mon, Feb 14, 2022 at 2:17 PM Akshay Joshi <
> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>> Hi Khushboo
>>
>> Jenkins is failing after committing the patch with error:
>>
>> *13:40:05* sed: couldn't edit venv/bin/__pycache__: not a regular file*13:40:05* The command "sed -i 's/#!.*\/python3/#\!\/usr\/pgadmin4\/venv\/bin\/python3/g' venv/bin/*" failed in "_create_python_virtualenv" with exit code 4.
>>
>>
>> API test cases for Utilities are failing.
>>
> Please find the attached patch to fix the API test cases.
>
> Thanks,
> Khushboo
>
>>
>> On Mon, Feb 14, 2022 at 11:02 AM Khushboo Vashi <
>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>> 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
>>>
>>
>> Not fixed.
>>
>>>
>>>> - There are two files *rds.py* with the same name, if
>>>> possible/feasible change the name of one of the files.
>>>>
>>>>
>> Not fixed.
>>
>>>
>>>> - "*_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*
>>>>
>>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>> *pgAdmin Hacker | Principal Software Architect*
>> *EDB Postgres <http://edbpostgres.com>*
>>
>> *Mobile: +91 976-788-8246*
>>
>
--
*Thanks & Regards*
*Akshay Joshi*
*pgAdmin Hacker | Principal Software Architect*
*EDB Postgres <http://edbpostgres.com>*
*Mobile: +91 976-788-8246*
From | Date | Subject | |
---|---|---|---|
Next Message | Akshay Joshi | 2022-02-14 12:54:05 | pgAdmin 4 commit: Remove __pycache__ to fix 'venv/bin/__pycache__: not |
Previous Message | Akshay Joshi | 2022-02-14 11:56:39 | pgAdmin 4 commit: 1) Fixed API test cases for Utilities. |