Re: [openstack-dev] [glance] Deprecating osprofiler option 'enabled' in favour of 'profiler_enabled'
Don't care either way, let's be consistent with other projects and raise this concern in next weekly cross-project meeting [1] to see what all of the projects mutually agree on. If there is no consensus, let's stick to what we have. @Louis: Can you please add that to the agenda of the cross-project meeting? [1] https://wiki.openstack.org/wiki/Meetings/CrossProjectMeeting Thanks, -Nikhil From: Ian Cordasco [ian.corda...@rackspace.com] Sent: Tuesday, December 02, 2014 10:32 AM To: OpenStack Development Mailing List (not for usage questions) Subject: Re: [openstack-dev] [glance] Deprecating osprofiler option 'enabled' in favour of 'profiler_enabled' Except for the fact that the person who implemented this was told to change the option name in other projects because it conflicted with a different option. We can keep this if we’re worried about being too obvious (to the point of becoming the Department of Redundancy Department) with our naming. I don’t think other projects will be very happy having to change their naming especially if the original name was already a problem. On 12/2/14, 06:12, Zhi Yan Liu lzy@gmail.com wrote: I totally agreed to make it to be consistent cross all projects, so I propose to change other projects. But I think keeping it as-it is clear enough for both developer and operator/configuration, for example: [profiler] enable = True instead of: [profiler] profiler_enable = True Tbh, the profiler prefix is redundant to me still from the perspective of operator/configuration. zhiyan On Tue, Dec 2, 2014 at 7:44 PM, Louis Taylor krag...@gmail.com wrote: On Tue, Dec 02, 2014 at 12:16:44PM +0800, Zhi Yan Liu wrote: Why not change other services instead of glance? I see one reason is glance is the only one service use this option name, but to me one reason to keep it as-it in glance is that original name makes more sense due to the option already under profiler group, adding profiler prefix to it is really redundant, imo, and in other existing config group there's no one go this naming way. Then in the code we can just use a clear way: CONF.profiler.enabled instead of: CONF.profiler.profiler_enabled thanks, zhiyan I agree this looks nicer in the code. However, the primary consumer of this option is someone editing it in the configuration files. In this case, I believe having something more verbose and consistent is better than the Glance code being slightly more elegant. One name or the other doesn't make all that much difference, but consistency in how we turn osprofiler on and off across projects would be best. - Louis ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [glance] Deprecating osprofiler option 'enabled' in favour of 'profiler_enabled'
On 02/12/14 12:16 +0800, Zhi Yan Liu wrote: Why not change other services instead of glance? I see one reason is glance is the only one service use this option name, but to me one reason to keep it as-it in glance is that original name makes more sense due to the option already under profiler group, adding profiler prefix to it is really redundant, imo, and in other existing config group there's no one go this naming way. Then in the code we can just use a clear way: CONF.profiler.enabled instead of: CONF.profiler.profiler_enabled I'm with Zhi Yan on this one. Adding profiler sounds redundant. Cheers, Flavio thanks, zhiyan On Mon, Dec 1, 2014 at 11:43 PM, Ian Cordasco ian.corda...@rackspace.com wrote: On 12/1/14, 08:37, Louis Taylor krag...@gmail.com wrote: Hi all, In order to enable or disable osprofiler in Glance, we currently have an option: [profiler] # If False fully disable profiling feature. enabled = False However, all other services with osprofiler integration use a similar option named profiler_enabled. For consistency, I'm proposing we deprecate this option's name in favour of profiler_enabled. This should make it easier for someone to configure osprofiler across projects with less confusion. Does anyone have any thoughts or concerns about this? Thanks, Louis We *just* introduced this if I remember the IRC discussion from last month. I’m not sure how many people will be immediately making use of it. I’m in favor of consistency where possible and while this would require a deprecation, I think it’s a worthwhile change. +1 from me — Ian ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -- @flaper87 Flavio Percoco pgp1egx0cInqT.pgp Description: PGP signature ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [glance] Deprecating osprofiler option 'enabled' in favour of 'profiler_enabled'
I totally agreed to make it to be consistent cross all projects, so I propose to change other projects. But I think keeping it as-it is clear enough for both developer and operator/configuration, for example: [profiler] enable = True instead of: [profiler] profiler_enable = True Tbh, the profiler prefix is redundant to me still from the perspective of operator/configuration. zhiyan On Tue, Dec 2, 2014 at 7:44 PM, Louis Taylor krag...@gmail.com wrote: On Tue, Dec 02, 2014 at 12:16:44PM +0800, Zhi Yan Liu wrote: Why not change other services instead of glance? I see one reason is glance is the only one service use this option name, but to me one reason to keep it as-it in glance is that original name makes more sense due to the option already under profiler group, adding profiler prefix to it is really redundant, imo, and in other existing config group there's no one go this naming way. Then in the code we can just use a clear way: CONF.profiler.enabled instead of: CONF.profiler.profiler_enabled thanks, zhiyan I agree this looks nicer in the code. However, the primary consumer of this option is someone editing it in the configuration files. In this case, I believe having something more verbose and consistent is better than the Glance code being slightly more elegant. One name or the other doesn't make all that much difference, but consistency in how we turn osprofiler on and off across projects would be best. - Louis ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
[openstack-dev] [glance] Deprecating osprofiler option 'enabled' in favour of 'profiler_enabled'
Hi all, In order to enable or disable osprofiler in Glance, we currently have an option: [profiler] # If False fully disable profiling feature. enabled = False However, all other services with osprofiler integration use a similar option named profiler_enabled. For consistency, I'm proposing we deprecate this option's name in favour of profiler_enabled. This should make it easier for someone to configure osprofiler across projects with less confusion. Does anyone have any thoughts or concerns about this? Thanks, Louis signature.asc Description: Digital signature ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev