[openstack-dev] [nova] Can we remove the monkey_patch_modules config option?

[Matt Riedemann]

I'm having a hard time tracing what this is necessary for. It's related
to the notify_decorator which is around for legacy notifications but I
don't actually see that decorator used anywhere. Given there are other
options related to the notify_decorator, like "default_publisher_id" if
we can start unwinding and removing this legacy stuff it would make the
config (.005%) simpler.

It also just looks like we have a monkey_patch option that is run at the
beginning of every service, uses monkey_patch_modules and if loaded,
monkey patches whatever is configured for modules.

I mean, if we thought hooks were bad, this is pretty terrible.

[Alex Schultz]

I'm having a hard time tracing what this is necessary for. It's related to
the notify_decorator which is around for legacy notifications but I don't
actually see that decorator used anywhere. Given there are other options
related to the notify_decorator, like "default_publisher_id" if we can start
unwinding and removing this legacy stuff it would make the config (.005%)
simpler.
It also just looks like we have a monkey_patch option that is run at the
beginning of every service, uses monkey_patch_modules and if loaded, monkey
patches whatever is configured for modules.
I mean, if we thought hooks were bad, this is pretty terrible.

JFYI, https://review.openstack.org/#/c/494305/

Since this was just added, someone is looking to use it or is using it.

Thanks,
-Alex

--
Thanks,
Matt
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-***@lists.openstack.org?subject:unsubscribe
http://lists.openstack

[Matt Riedemann]

JFYI,https://review.openstack.org/#/c/494305/
Since this was just added, someone is looking to use it or is using it.

Thanks for pointing this out. I've asked in the puppet review that the
submitter of that change explain if/why they are using this config
option and reply to this thread, since the change to deprecate the
option in nova just merged:

https://review.openstack.org/498113

[Lokesh Jain]

Hi Matt,

Thank you for pointing this out and bringing this to my attention.

We are using the monkey_patch options to add more nova capabilities.
Specifically we are using it to replace Nova’s

nova/network/neutronv2/api.get_port_vnic_info

function for extending the vnic type + phys network lookup for given
neutron port id logic in case of multi-segments + multi phys network
SRIOV deployments.

We are using this functionality since Mitaka. I am assuming that the
removal of nova parameters is proposed for Pike and onwards. Or is the
plan to backport it to previous releases as well? In that case will
need to figure out the plan for it.

Please let me know if further information or clarification is required.

Thanks and Regards,
Lokesh Jain

[Matt Riedemann]

Post by Lokesh Jain
Hi Matt,
Thank you for pointing this out and bringing this to my attention.
We are using the monkey_patch options to add more nova capabilities.
Specifically we are using it to replace Nova’s
nova/network/neutronv2/api.get_port_vnic_info
function for extending the vnic type + phys network lookup for given
neutron port id logic in case of multi-segments + multi phys network
SRIOV deployments.
We are using this functionality since Mitaka. I am assuming that the
removal of nova parameters is proposed for Pike and onwards. Or is the
plan to backport it to previous releases as well? In that case will
need to figure out the plan for it.
Please let me know if further information or clarification is required.
Thanks and Regards,
Lokesh Jain
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

The change to deprecate the monkey_patch and monkey_patch_modules option
was deprecated in Queens, so they wouldn't be removed until the Rocky
release at the earliest. We don't backport changes which deprecate things.

My recommendation to you is upstream your changes. If it's a new
feature, propose a blueprint [1].

Chances are someone else needs or wants the same thing, or is already
doing something similar but without the monkey patch config options.

[1] https://docs.openstack.org/nova/latest/contributor/blueprints.html

[Matt Riedemann]

Post by Matt Riedemann
The change to deprecate the monkey_patch and monkey_patch_modules option
was deprecated in Queens, so they wouldn't be removed until the Rocky
release at the earliest. We don't backport changes which deprecate things.
My recommendation to you is upstream your changes. If it's a new
feature, propose a blueprint [1].
Chances are someone else needs or wants the same thing, or is already
doing something similar but without the monkey patch config options.
[1] https://docs.openstack.org/nova/latest/contributor/blueprints.html

Following up on this, I have submitted a patch to remove the deprecated
monkey_patch-related config options:

https://review.openstack.org/#/c/568880/