Keyword args are defaulted to CONF values

[Peter Stachowski]

The Trove code is full of numerous cases where kwargs are defaulted to CONF values. Unfortunately this is an error-prone way of setting up a default value, as python sets these one time only. If the module is loaded before the CONF values are taken from the config files, then the default value will be incorrect.

For example, the following code will use the default (i.e. the value in cfg.py) for default_password_length, regardless of the value set in the trove.conf file:

    def generate_random_password(password_length=CONF.default_password_length):
        # generate password here
        ...

The correct way of handling this is to use the following:

    def generate_random_password(password_length=None):
        password_length = password_length or CONF.default_password_length
        # generate password here
        ...

A quick grep turns up 85 instances in the code where this occurs.

[Amrith Kumar (amrith)]

Good catch Peter!

[Brandon Iz (iz-brandon)]

I can go ahead an get that changed.

[Peter Stachowski (peterstac)]

Amrith - Actually Doug (dougshelley66) ran into this. I just volunteered to enter the bug for it. :)

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/306786

[Brandon Iz (iz-brandon)]

When looking into this bug I found that the statement of this happening 85 times in the code was a bit misleading. It is true that when you run ' grep -r "=CONF" ' it produces 85 results, but many of these results do not deal with a kwarg.

By refining the grep to ' grep -r -A 1 -B 5 "\def\b" | grep "=CONF" ' you can see when this happens to a kwarg value being defined as a parameter in a def.

run_tests.py- datastore_id=CONFIG.dbaas_datastore_id,
trove/taskmanager/manager.py- @periodic_task.periodic_task(spacing=CONF.quota_notification_interval)
trove/extensions/mgmt/instances/models.py- seconds=CONF.exists_notification_interval),
trove/tests/api/instances.py- @test(enabled=CONFIG.test_mgmt)
trove/tests/api/instances.py- enabled=CONFIG.test_mgmt)
trove/common/debug_utils.py- pydev_path=CONF.pydev_path)
trove/common/remote.py- endpoint_region=CONF.os_region_name,
trove/common/utils.py:def generate_random_password(password_length=CONF.default_password_length):
trove/common/notification.py: def notify(self, event_type, publisher_id=CONF.host):
trove/guestagent/datastore/galera_common/service.py- self._bootstrap_cluster(timeout=CONF.restore_usage_timeout)
trove/guestagent/datastore/galera_common/service.py- self.start_mysql(timeout=CONF.restore_usage_timeout)
trove/guestagent/datastore/experimental/redis/service.py- timeout=CONF.usage_timeout):

Not all of these need to be changed either, but I applied the changes where they followed the example stated. I am still on the fence about the changes in 'debug_utils.py' , but with those changes I was passing local tests.

[Amrith Kumar (amrith)]

Brandon, would you like to fix this in Mitaka as well?

[Brandon Iz (iz-brandon)]

Just wanted to note this was discussed on IRC, and I'd be happy to.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/306786
Committed: https://git.openstack.org/cgit/openstack/trove/commit/?id=64a869dbaf6ee583d667b4a6fd5fbbcd8902a646
Submitter: Jenkins
Branch: master

commit 64a869dbaf6ee583d667b4a6fd5fbbcd8902a646
Author: Brandon Irizarry <email address hidden>
Date: Sat Apr 16 22:55:50 2016 +0000

    Fixed kwargs being defaulted to CONF values

    The Trove code is full of numerous cases where kwargs are
    defaulted to CONF values. This is an error-prone
    way of setting up a default value, as python sets these one
    time only. This fix accounts for that.

    Change-Id: Icc0858ccb2d3e2584bf6f3d1542a7d631b251ac8
    Closes-Bug: 1571076

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/mitaka
Review: https://review.openstack.org/307501

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/307501
Committed: https://git.openstack.org/cgit/openstack/trove/commit/?id=afb2e0fddc9df5b3c9bc7ffda267ae143a0abc66
Submitter: Jenkins
Branch: stable/mitaka

commit afb2e0fddc9df5b3c9bc7ffda267ae143a0abc66
Author: Brandon Irizarry <email address hidden>
Date: Sat Apr 16 22:55:50 2016 +0000

    Fixed kwargs being defaulted to CONF values

    The Trove code is full of numerous cases where kwargs are
    defaulted to CONF values. This is an error-prone
    way of setting up a default value, as python sets these one
    time only. This fix accounts for that.

    Change-Id: Icc0858ccb2d3e2584bf6f3d1542a7d631b251ac8
    Closes-Bug: 1571076
    (cherry picked from commit 64a869dbaf6ee583d667b4a6fd5fbbcd8902a646)

[Doug Hellmann (doug-hellmann)]

This issue was fixed in the openstack/trove 5.0.1 release.

[Doug Hellmann (doug-hellmann)]

This issue was fixed in the openstack/trove 6.0.0.0b2 development milestone.