On Thu, 9 Jun 2022 at 08:16, Akshay Joshi <akshay.jo...@enterprisedb.com>
wrote:
> Hi Pradip
>
> Following are the review comments:
>
>
> - 'show user defined templates' in preferences should be ''show user
> defined template databases'.
>
> Is it just user defined ones, or does it include template0/template1?
>
> -
> - 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/>
>
--
Dave Page
Blog: https://pgsnake.blogspot.com
Twitter: @pgsnake
EDB: https://www.enterprisedb.com
