Hi Pradip
Following are the review comments:
- 'show user defined templates' in preferences should be ''show user
defined template databases'.
- Help message for 'Template?' should be changed to "Note: When the
preferences setting 'show user defined templates' is set to false, then
user-defined template databases won't be displayed in the browser tree."
instead of the old one "Based on the preferences setting, this field
determines whether the database is visible or hidden"
- As per my previous review comment, extra space after "IS_TEMPLATE" is
still there
- ALTER DATABASE {{ conn|qtIdent(data.name) }} WITH IS_TEMPLATE = {{
data.is_template }};
- IS_TEMPLATE = {{ data.is_template }}{% endif %};
- In the SQL file, you added a new condition "{% if
show_user_defined_templates is defined %}" but I found that you have not
passed the '*show_user_defined_templates*' parameter from the 'py' file
then there is no use of the above condition. Either remove the condition or
send the parameter. Check for all such files.
- Update the docs and screenshots again after the first 2 fixes.
- RESQL failing on EPAS, check once.
On Thu, Jun 9, 2022 at 7:37 AM Pradip Parkale <
pradip.park...@enterprisedb.com> wrote:
> Hi Akshay,
>
> Please find the updated patch.I have fixed all the review comments.
>
>
> *Thanks & Regards,*
>
> *Pradip ParkaleSoftware Engineer | EnterpriseDB Corporation*
>
>
> On Mon, Jun 6, 2022 at 3:15 PM Dave Page <dp...@pgadmin.org> wrote:
>
>>
>>
>> On Mon, 6 Jun 2022 at 10:30, Aditya Toshniwal <
>> aditya.toshni...@enterprisedb.com> wrote:
>>
>>>
>>>
>>> On Mon, Jun 6, 2022 at 2:20 PM Khushboo Vashi <
>>> khushboo.va...@enterprisedb.com> wrote:
>>>
>>>>
>>>>
>>>> On Mon, Jun 6, 2022 at 2:17 PM Dave Page <dp...@pgadmin.org> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Fri, 3 Jun 2022 at 08:26, Akshay Joshi <
>>>>> akshay.jo...@enterprisedb.com> wrote:
>>>>>
>>>>>> Hi Pradip
>>>>>>
>>>>>> Following are the review comments:
>>>>>>
>>>>>> - Fixed pep8 issues.
>>>>>> - Documentation updates are missing.
>>>>>> - In the 'alter_online.sql' file correct the comment above the
>>>>>> ALTER statement. Also, remove one extra space after "IS_TEMPLATE". Do
>>>>>> this
>>>>>> in both the files.
>>>>>> - Remove extra spaces from all the 'properties.sql' files before
>>>>>> the "ORDER BY" clause.
>>>>>> -
>>>>>>
>>>>>> {% if show_user_defined_templates is defined %}
>>>>>> AND db.datistemplate = {{show_user_defined_templates}}
>>>>>> {% endif %} Code is duplicated in the "9.1_plus/properties.sql" file
>>>>>> please check.
>>>>>>
>>>>>> -
>>>>>>
>>>>>> 'IS_TEMPLATE' is available from 9.4 onwards, so change the SQL files
>>>>>> accordingly.
>>>>>>
>>>>>>
>>>>>> We shouldn't be messing around with pre-v10 templates any more.
>>>>>
>>>>> I wonder if we should rethink the decision we made ages ago to not
>>>>> purge old template versions. It seems to me we have a *lot* of templates
>>>>> for now unsupported versions of PostgreSQL, and maybe we should work to
>>>>> bring the default level up to v10 and get rid of older variants.
>>>>>
>>>>>
>>>> +1
>>>>
>>> I was/am always in favor of this. :)
>>> And a decision on browser version support also.
>>>
>>
>> Start a thread with a proposed policy then :-p
>>
>>
>>>
>>>>>> -
>>>>>>
>>>>>> Database creation is missing IS_TEMPLATE command, please add that
>>>>>> control into the same patch.
>>>>>>
>>>>>> -
>>>>>>
>>>>>> Found one issue where SQL tab is thorwing an error when selecting
>>>>>> 'template0' database.
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Wed, Jun 1, 2022 at 3:51 PM Pradip Parkale <
>>>>>> pradip.park...@enterprisedb.com> wrote:
>>>>>>
>>>>>>> Hi Hackers,
>>>>>>>
>>>>>>> Please find the attached patch for #7351.
>>>>>>> I have made all the necessary changes which were discussed.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> *Thanks & Regards,*
>>>>>>>
>>>>>>> *Pradip ParkaleSoftware Engineer | EnterpriseDB Corporation*
>>>>>>>
>>>>>>>
>>>>>>> On Wed, May 18, 2022 at 7:11 PM Dave Page <dp...@pgadmin.org> wrote:
>>>>>>>
>>>>>>>> Hi
>>>>>>>>
>>>>>>>> On Wed, 18 May 2022 at 14:02, Pradip Parkale <
>>>>>>>> pradip.park...@enterprisedb.com> wrote:
>>>>>>>>
>>>>>>>>> Hi Dave/Team,
>>>>>>>>>
>>>>>>>>> I have come up with a plan to implement this. Please give your
>>>>>>>>> suggestions
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 1. Users will be able to hide user-defined templates in the
>>>>>>>>> preferences setting. By default, the value is 'Yes', and all
>>>>>>>>> user-defined
>>>>>>>>> templates will be hidden.
>>>>>>>>> 2. The icon will be different for user-defined templates.
>>>>>>>>> 3. Users can connect to templete DB and properies are also
>>>>>>>>> same, so no need to change the properties dialog options/design.
>>>>>>>>> 4. We are giving options to hide the templates and by default,
>>>>>>>>> the option will be true, so the collection node for template DBs
>>>>>>>>> will be
>>>>>>>>> the same as for other databases.
>>>>>>>>>
>>>>>>>>> Works for me. Thanks.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Tue, May 17, 2022 at 1:50 PM Dave Page <dp...@pgadmin.org>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Tue, 17 May 2022 at 08:08, Pradip Parkale <
>>>>>>>>>> pradip.park...@enterprisedb.com> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi Dave/Team,
>>>>>>>>>>>
>>>>>>>>>>> On Wed, May 11, 2022 at 4:37 PM Aditya Toshniwal <
>>>>>>>>>>> aditya.toshni...@enterprisedb.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> On Wed, May 11, 2022 at 4:03 PM Dave Page <dp...@pgadmin.org>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Hi
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Wed, 11 May 2022 at 09:40, Aditya Toshniwal <
>>>>>>>>>>>>> aditya.toshni...@enterprisedb.com> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi Dave,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Wed, May 11, 2022 at 2:00 PM Dave Page <dp...@pgadmin.org>
>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Wed, 11 May 2022 at 09:24, Aditya Toshniwal <
>>>>>>>>>>>>>>> aditya.toshni...@enterprisedb.com> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On Wed, May 11, 2022 at 1:24 PM Dave Page <
>>>>>>>>>>>>>>>> dp...@pgadmin.org> wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Hi
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I think this change needs some thought and discussion on
>>>>>>>>>>>>>>>>> how it should be implemented. pgAdmin has worked this way for
>>>>>>>>>>>>>>>>> over 20 years
>>>>>>>>>>>>>>>>> with only this one suggestion to change afaicr - and I for
>>>>>>>>>>>>>>>>> one don't
>>>>>>>>>>>>>>>>> suddenly want templates showing up amongst my other databases.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> User defined template DBs cannot come under "Show system
>>>>>>>>>>>>>>>> objects". I mean, they're not system objects.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> No they're not. But we have over 20 years of them being
>>>>>>>>>>>>>>> classed that way, and users may not suddenly want to see
>>>>>>>>>>>>>>> template1 (for
>>>>>>>>>>>>>>> example) listed amongst their databases. There are various
>>>>>>>>>>>>>>> things to think
>>>>>>>>>>>>>>> about here, for example:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>> No template1, template0 are system objects. They won't be
>>>>>>>>>>>>>> visible. Only new databases created manually and marked as
>>>>>>>>>>>>>> template are
>>>>>>>>>>>>>> excluded from "Show system objects" .
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> template0 is. template1 is *by default*, but not if you drop
>>>>>>>>>>>>> and recreate it.
>>>>>>>>>>>>>
>>>>>>>>>>>> Didn't think in that way :)
>>>>>>>>>>>> In that case, we can add a new preference - "Show
>>>>>>>>>>>> non-system/user defined template databases?" explicitly. By default
>>>>>>>>>>>> "yes", show them.
>>>>>>>>>>>> Along with a new icon.
>>>>>>>>>>>>
>>>>>>>>>>> Has this been finalized?
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> No - I haven't seen any proposals for what will be done, except
>>>>>>>>>> for what you've written below which no one has commented on yet.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> Could I add a new preference setting to show/hide user-defined
>>>>>>>>>>> template databases? The new icon for that may look like a 'T' on
>>>>>>>>>>> top of the
>>>>>>>>>>> current DB icon.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I think that's fine.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> - Should there be a separate option to show/hide template
>>>>>>>>>>>>>>> databases?
>>>>>>>>>>>>>>> - If so, what should the default be (hint: I think yes, and
>>>>>>>>>>>>>>> off)
>>>>>>>>>>>>>>> - Should template databases have a different icon?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>> We can have this. Good to differentiate.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> - Should they have their own collection node?
>>>>>>>>>>>>>>> - They can't be connected to, so much of what's on the
>>>>>>>>>>>>>>> database properties dialog won't work. Should they have their
>>>>>>>>>>>>>>> own
>>>>>>>>>>>>>>> properties dialogue design?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I can connect to template DBs in pgAdmin.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Yeah, disregard that. My brain was mixing up datistemplate and
>>>>>>>>>>>>> datallowconn.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On Wed, 11 May 2022 at 07:12, Pradip Parkale <
>>>>>>>>>>>>>>>>> pradip.park...@enterprisedb.com> wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Hi Hackers,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Please find the attached patch for # 7351:Templates not
>>>>>>>>>>>>>>>>>> displayed.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> I have fixed below issues
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> 1. Databases which are marked as template manually by
>>>>>>>>>>>>>>>>>> the user should be visible independent of - "Show System
>>>>>>>>>>>>>>>>>> Objects".
>>>>>>>>>>>>>>>>>> 2. DB properties dialogs should allow you to change
>>>>>>>>>>>>>>>>>> template flag in edit mode.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>>>> Thanks & Regards,
>>>>>>>>>>>>>>>>>> Pradip Parkale
>>>>>>>>>>>>>>>>>> Software Engineer | EnterpriseDB Corporation
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>>> Dave Page
>>>>>>>>>>>>>>>>> Blog: https://pgsnake.blogspot.com
>>>>>>>>>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> EDB: https://www.enterprisedb.com
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>> Aditya Toshniwal
>>>>>>>>>>>>>>>> pgAdmin Hacker | Software Architect | *edbpostgres.com*
>>>>>>>>>>>>>>>> <http://edbpostgres.com>
>>>>>>>>>>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>> Dave Page
>>>>>>>>>>>>>>> Blog: https://pgsnake.blogspot.com
>>>>>>>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> EDB: https://www.enterprisedb.com
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> --
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> Aditya Toshniwal
>>>>>>>>>>>>>> pgAdmin Hacker | Software Architect | *edbpostgres.com*
>>>>>>>>>>>>>> <http://edbpostgres.com>
>>>>>>>>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> --
>>>>>>>>>>>>> Dave Page
>>>>>>>>>>>>> Blog: https://pgsnake.blogspot.com
>>>>>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>>>>>
>>>>>>>>>>>>> EDB: https://www.enterprisedb.com
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> --
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Aditya Toshniwal
>>>>>>>>>>>> pgAdmin Hacker | Software Architect | *edbpostgres.com*
>>>>>>>>>>>> <http://edbpostgres.com>
>>>>>>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>> Thanks & Regards,
>>>>>>>>>>> Pradip Parkale
>>>>>>>>>>> Software Engineer | EnterpriseDB Corporation
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> Dave Page
>>>>>>>>>> Blog: https://pgsnake.blogspot.com
>>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>>
>>>>>>>>>> EDB: https://www.enterprisedb.com
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Thanks & Regards,
>>>>>>>>> Pradip Parkale
>>>>>>>>> Software Engineer | EnterpriseDB Corporation
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Dave Page
>>>>>>>> Blog: https://pgsnake.blogspot.com
>>>>>>>> Twitter: @pgsnake
>>>>>>>>
>>>>>>>> EDB: https://www.enterprisedb.com
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>> --
>>>>>>
>>>>>> <http://www.enterprisedb.com>
>>>>>>
>>>>>> Akshay Joshi
>>>>>>
>>>>>> Principal Software Architect
>>>>>>
>>>>>> +91 9767888246
>>>>>>
>>>>>> www.enterprisedb.com
>>>>>>
>>>>>> <https://www.linkedin.com/company/edbpostgres>
>>>>>> <https://twitter.com/edbpostgres?lang=en>
>>>>>> <https://www.facebook.com/EDBpostgres>
>>>>>> <https://www.instagram.com/EDBpostgres/>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Dave Page
>>>>> Blog: https://pgsnake.blogspot.com
>>>>> Twitter: @pgsnake
>>>>>
>>>>> EDB: https://www.enterprisedb.com
>>>>>
>>>>>
>>>
>>> --
>>> Thanks,
>>> Aditya Toshniwal
>>> pgAdmin Hacker | Software Architect | *edbpostgres.com*
>>> <http://edbpostgres.com>
>>> "Don't Complain about Heat, Plant a TREE"
>>>
>>
>>
>> --
>> Dave Page
>> Blog: https://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EDB: https://www.enterprisedb.com
>>
>>
--
<http://www.enterprisedb.com>
Akshay Joshi
Principal Software Architect
+91 9767888246
www.enterprisedb.com
<https://www.linkedin.com/company/edbpostgres>
<https://twitter.com/edbpostgres?lang=en>
<https://www.facebook.com/EDBpostgres>
<https://www.instagram.com/EDBpostgres/>
