From b3263223779d45377aa13906b8d4422d71e60c1b Mon Sep 17 00:00:00 2001 From: Zhao Chao Date: Thu, 15 Mar 2018 22:50:47 +0800 Subject: [PATCH] Avoid using hasattr() on troveclient resources for py3 porting troveclient resources use the lazy-loading feature, which causes problems for calling hasttr() on the object. copy.deepcopy() also use hasattr(), and results in infinite loops. hasattr() will ignore any exceptions under Python 2.x, but reraise them under Python 3, this is reason why the related test cases passed under Python 2.x and keep failing under Python 3. This patch fixes and should be the last one to fix the py35 gate jobs. Partial-Bug: #1755413 Change-Id: I97492605047a986d3075a8b5f22ecbfdb3af8aca Signed-off-by: Zhao Chao (cherry picked from commit 721da6ac3e34f98606672c0f3f52a240e96cb860) --- .../content/database_clusters/forms.py | 13 +++++++- .../content/database_configurations/tests.py | 33 +++++++++++++++++-- .../databases/workflows/create_instance.py | 7 +++- 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/trove_dashboard/content/database_clusters/forms.py b/trove_dashboard/content/database_clusters/forms.py index 5cbb87d..c7149b4 100644 --- a/trove_dashboard/content/database_clusters/forms.py +++ b/trove_dashboard/content/database_clusters/forms.py @@ -238,7 +238,18 @@ class LaunchForm(forms.SelfHandlingForm): # only add to choices if datastore has at least one version version_choices = () for v in versions: - if hasattr(v, 'active') and not v.active: + # NOTE(zhaochao): troveclient API resources are lazy + # loading objects. When an attribute is not found, the + # get() method of the Manager object will be called + # with the ID of the resource. However for + # datastore_versions, the get() method is expecting two + # arguments: datastore and datastore_version(name), so + # TypeError will be raised as not enough arguments are + # passed. In Python 2.x, hasattr() won't reraise the + # exception(which is not correct), but reraise under + # Python 3(which should be correct). + # Use v.to_dict() to verify the 'active' info instead. + if not v.to_dict().get('active', True): continue selection_text = self._build_datastore_display_text( ds.name, v.name) diff --git a/trove_dashboard/content/database_configurations/tests.py b/trove_dashboard/content/database_configurations/tests.py index 20fba8f..ea0c291 100644 --- a/trove_dashboard/content/database_configurations/tests.py +++ b/trove_dashboard/content/database_configurations/tests.py @@ -12,7 +12,6 @@ # License for the specific language governing permissions and limitations # under the License. -import copy import logging import six @@ -378,7 +377,26 @@ class DatabaseConfigurationsTests(test.TestCase): 'configuration_update',), config_param_manager: ('get',)}) def test_values_tab_apply_action(self): - config = copy.deepcopy(self.database_configurations.first()) + # NOTE(zhaochao): we cannot use copy.deepcopy() under Python 3, + # because of the lazy-loading feature of the troveclient Resource + # objects. copy.deepcopy will use hasattr to search for the + # '__setstate__' attribute of the resource object. As the resource + # object is lazy loading, searching attributes relys on the 'is_load' + # property, unfortunately this property is also not loaded at the + # moment, then we're getting in an infinite loop there. Python will + # raise RuntimeError saying "maximum recursion depth exceeded", this is + # ignored under Python 2.x, but reraised under Python 3 by hasattr(). + # + # Temporarily importing troveclient and reconstructing a configuration + # object from the original config object's dict info will make this + # case (and the next) working under Python 3. + original_config = self.database_configurations.first() + from troveclient.v1 import configurations + config = configurations.Configuration( + configurations.Configurations(None), original_config.to_dict()) + # Making sure the newly constructed config object is the same as + # the original one. + self.assertEqual(config, original_config) # setup the configuration parameter manager config_param_mgr = config_param_manager.ConfigParamManager( @@ -412,7 +430,16 @@ class DatabaseConfigurationsTests(test.TestCase): 'configuration_update',), config_param_manager: ('get',)}) def test_values_tab_apply_action_exception(self): - config = copy.deepcopy(self.database_configurations.first()) + # NOTE(zhaochao) Please refer to the comment at the beginning of the + # 'test_values_tab_apply_action' about not using copy.deepcopy() for + # details. + original_config = self.database_configurations.first() + from troveclient.v1 import configurations + config = configurations.Configuration( + configurations.Configurations(None), original_config.to_dict()) + # Making sure the newly constructed config object is the same as + # the original one. + self.assertEqual(config, original_config) # setup the configuration parameter manager config_param_mgr = config_param_manager.ConfigParamManager( diff --git a/trove_dashboard/content/databases/workflows/create_instance.py b/trove_dashboard/content/databases/workflows/create_instance.py index 281dfde..4f120ba 100644 --- a/trove_dashboard/content/databases/workflows/create_instance.py +++ b/trove_dashboard/content/databases/workflows/create_instance.py @@ -211,7 +211,12 @@ class SetInstanceDetailsAction(workflows.Action): # only add to choices if datastore has at least one version version_choices = () for v in versions: - if hasattr(v, 'active') and not v.active: + # NOTE(zhaochao): please refer to the comment about + # the same change for 'populate_datastore_choices' + # of 'LaunchForm' in + # trove_dashboard/content/database_clusters/forms.py + # for details. + if not v.to_dict().get('active', True): continue if self.backup_id: if v.id != backup.datastore['version_id']: