Re: [openstack-dev] [glance] Deprecating osprofiler option 'enabled' in favour of 'profiler_enabled'

2014-12-03 Thread Nikhil Komawar
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'

2014-12-02 Thread Flavio Percoco

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'

2014-12-02 Thread Zhi Yan Liu
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'

2014-12-01 Thread Louis Taylor
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