Open Stack

On Wed, Jul 8, 2015 at 12:27 PM, Ken'ichi Ohmichi <ken1ohmichi at gmail.com> wrote:
> Hi tempest team,
>
> We are working for refactoring service clients in tempest for
> preparing to migrate them to tempest-lib, and I found an inconsistency
> between them.
> So I'd like to ask opinions for considering which is the best as
> library methods.
>
> On some methods, a caller can specify whole body for sending a request
> to http POST/PUT methods.
> But on the others a caller can specify only part of body.
>
> For example, a caller can specify any parameters to
> update_quota_class_set() like:
>
> tempest/services/compute/json/quota_classes_client.py
>  34     def update_quota_class_set(self, quota_class_id, **kwargs):
>  35         """
>  36         Updates the quota class's limits for one or more resources.
>  37         """
>  38         post_body = json.dumps({'quota_class_set': kwargs})
>  39
>  40         resp, body = self.put('os-quota-class-sets/%s' % quota_class_id,
>  41                               post_body)
>
> but update_quota_set method has each argument and a caller cannot
> specify the other parameters like:
>
> tempest/services/compute/json/quotas_client.py
>  44     def update_quota_set(self, tenant_id, user_id=None,
>  45                          force=None, injected_file_content_bytes=None,
>  46                          metadata_items=None, ram=None, floating_ips=None,
>  47                          fixed_ips=None, key_pairs=None, instances=None,
>  48                          security_group_rules=None, injected_files=None,
>  49                          cores=None, injected_file_path_bytes=None,
>  50                          security_groups=None):
>  51         """
>  52         Updates the tenant's quota limits for one or more resources
>  53         """
>  54         post_body = {}
>  55
>  56         if force is not None:
>  57             post_body['force'] = force
>  58
>  59         if injected_file_content_bytes is not None:
>  60             post_body['injected_file_content_bytes'] = \
>  61                 injected_file_content_bytes
>  [..]
>  96         post_body = json.dumps({'quota_set': post_body})
>  97
>  98         if user_id:
>  99             resp, body = self.put('os-quota-sets/%s?user_id=%s' %
> 100                                   (tenant_id, user_id), post_body)
> 101         else:
> 102             resp, body = self.put('os-quota-sets/%s' % tenant_id,
> 103                                   post_body)
>
> By defining all parameters on each method like update_quota_set(), it
> is easy to know what parameters are available from caller/programer
> viewpoint.

I think this can be achieved with former approach also by defining all
expected param in doc string properly.

> So I feel the later seems easy to use them as library methods.
> But as the demerit, a caller cannot specify *undefined" parameters for
> fuzzing tests.
> Nova v2.1 API should deny a request which includes undefined
> parameters, and Tempest will be able to test the behavior if the
> former style.
> So this is my concern point now.
>
> I tend to prefer the later(all parameters need to be defined on each
> method) because that will be useful for callers as the above.
> In addition, tempest-lib should be a library for implementing
> integration tests, then fuzzing tests can be out of scope.
> If fuzzing tests are necessary, we will be able to implement them with
> RestClient of tempest-lib without service clients.

Second approach also looks good but I prefer the first one. As you
mentioned that fuzzy testing will not be possible with second
approach, we should not restrict our lib functions for the same. Any
project/test suits should be able to do API/Fuzzy testing with
tempest-lib.
Good example is nova v2.1 as you mentioned. Using service clients of
lib, project (Nova) or test suits (Tempest) should be able to perform
fuzzy testing.

IMO, lib's service clients should not have any restriction on param
list (type or number etc) to be passed from its users. Whatever
(random param etc) is being passed that should be passed to
corresponding projects APIs and let APIs through error or success.

This can be useful when project adds new request attributes (with
current API change guidelines like with microversion), in that case we
do not need to change service clients. But microversion testing is not
yet decided so this should/may not be considered aa strong point?

>
> I am on between both now, and I'd like to get feedback about this
> before changing them.

Thanks a lot for driving service clients towards good interface :)

> Any comments are welcome :)
>
> Thanks
> Ken ohmichi
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



-- 
Thanks & Regards
Ghanshyam Mann