cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Santhosh Edukulla <santhosh.eduku...@citrix.com>
Subject RE: Review Request 15021: Fixed Bug: 4899 : Added Configuration Support to Marvin.
Date Wed, 30 Oct 2013 10:42:48 GMT
ConfigManager is as synonymous to any other configuration  facility provided for products.
Currently, we provide configuration as part of some file say json\cfg and provide the command
line option to use that.  Now, there are places where the hard coding specific to setup\data
is done inside of test data.  I have given one simple example related to ldap search and replace.
 In order to replace these issues, we can use the ConfigManager facility added.  We can now
extend it to some more usecases if required. It has now some provision to read the config,
abstract to users and obtain to test features as easy to use dictionary.

The new facility added handles all that mentioned in doc strings. It has a simple interface
to retrieve the configuration provided and users can use it easily inside the test features
as a dictionary.  With this, all configuration pertaining to run will be at one single place
 and we provide a simple usage of this through dictionary.  It doesn't have any setters for
that matter. With this, all hard coding will be moved to configuration and users use the dynamic
dictionary created based upon config and is more driven through config.  We can remove current
not a cleaner way of search and replace with this.   

Currently, testclient provides apiclient,dbconnection and added to it will provide configuration
interface as well. Using this, test features will get configuration facility for his feature
for use.

Users mention all parameters related to their test in config file, and they have a dictionary
of the parsed config inside of their tests to use this data. The user don't need to hard code
this. Of a test as we are doing currently. On a given setup they can use a different config
and it works to work with those ips set up infrastructure dynamically. If I have a ldap server
ip different from other, the code still works, we just need to change the config.

The features, where this hard coding is done will now be replaced with this usage of moving
those hard coded strings to configuration file and replace those with config dictionary values,
which will be filled in dynamically.

What else you see is missing? Let me know and I can enhance it to add.


Santhosh

-----Original Message-----
From: Prasanna Santhanam [mailto:tsp@apache.org] 
Sent: Wednesday, October 30, 2013 3:42 PM
To: Santhosh Edukulla
Cc: Prasanna Santhanam; cloudstack
Subject: Re: Review Request 15021: Fixed Bug: 4899 : Added Configuration Support to Marvin.

I've given some inline comments in the patch. The ConfigManager looks too simple perhaps because
it's a WIP? At least the goals as you've written in your docstring seem to talk about more
than what's provided in the module.

On Tue, Oct 29, 2013 at 06:00:14PM -0000, Santhosh Edukulla wrote:
> -------
> 
>  Added Configuration Support to Marvin.
>     
>     1. It provides the basic configuration facilities to marvin.
>     
How will this be different from the current configGenerator module?
The configGenerator could read and write a structured deployment configuration. Is the goal
of this configGenerator different?

>     2. User can just add configuration files for his tests, deployment
>                   etc, under one config folder before running their tests.
>                   cs/tools/marvin/marvin/config.

This seems restrictive because previously one could have their configs placed anywhere. what
would be the advantage of placing all configs in tools/marvin/marvin/config? When packaged
and deployed this path is going to change to /usr/local/lib/python/site-packages/marvin/config.
Do we want that?

>                   They can remove all hard coded values from code and separate
>                   it out as config at this location.

How would the ConfigParser know where to find a specific section and at what depth? Is there
a structure? Or do you leave that up to the test author? I'd rather not do the latter as things
go wild with too many different ways to do the same thing.

>                   Either add this to the existing setup.cfg as separate section
>                   or add new configuration.
>     3. This will thus removes hard coded tests and separate
>                   data from tests.

Really? I think a lot of data is not specific to deployment configuration today in tests.
If we unify all data into the deployment json it's going to become a mess to manage, edit
and control when running tests.

>     
>     4. This API is provided as an additional facility under
>                   cloudstackTestClient and users can get the
>                   configuration object as similar to apiclient,dbconnection
>                   etc to drive their test.

marvinPlugin.py,L#158 - the config is already injected into each test.
I like it that it's part of the testClient, wil you be providing setter properties to it if
it's part of the testClient via ConfigManager. Test case authors might be tempted to change
configuration on the fly which might affect downstream tests in the same suite? 

>     
>     5. They just add their configuration for a test,
>                   setup etc,at one single place under configuration dir
>                   and use "getConfigParser" API of cloudstackTestClient
>                   It will give them "configObj".They can either pass their own
>                   config file for parsing to "getConfig" or it will use
>                   default config file @ config/setup.cfg.
>     6. They will then get the dictionary of parsed
>                   configuration and can use it further to drive their tests or
>                   config drive
>     7. Test features, can  drive their setups thus removing hard coded
>               values. Configuration default file will be under config and as
>                   setup.cfg.
>     8. Users can use their own configuration file passed to
>                   "getConfig" API,once configObj is returned.
>     
> Another such case where we are using sed or bash script is  in between a build run for
replacing hard coded ldap ip for region\setup specific. We can now change all parameters before
run under configuration, the test features will use configuration object and thus values,
rather hard coded strings which avoids replacement through shell script.
> 
>  Also, did few naming convention changes. Its better to follow uniform naming conventions.
Currently, wherever iam seeing a specific notation not followed. We are incorporating those
changes.
> 
> ToDo:
> clean up of current config at places, making configuration required for marvin\tests
unified and available at one place and clean up of files\code related to it. 
> 
> 
> Diffs
> -----
> 
>   tools/marvin/marvin/cloudstackTestClient.py be93f35 
>   tools/marvin/marvin/config/setup.cfg PRE-CREATION 
>   tools/marvin/marvin/configGenerator.py 0cfad30 
>   tools/marvin/marvin/integration/lib/utils.py 7d662af
> 
> Diff: https://reviews.apache.org/r/15021/diff/
> 
> 
> Testing
> -------
> 
> Basic check to retrieve configuration values is done.
> 
> 
> Thanks,
> 
> Santhosh Edukulla
> 

--
Prasanna.,

------------------------
Powered by BigRock.com


Mime
View raw message