Mailing-List: contact dev-help@cloudstack.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cloudstack.apache.org
Received-SPF: pass (nike.apache.org: domain of santhosh.edukulla@citrix.com
 designates 103.14.252.240 as permitted sender)
From: Santhosh Edukulla <santhosh.edukulla@citrix.com>
To: Prasanna Santhanam <tsp@apache.org>
CC: cloudstack <dev@cloudstack.apache.org>
Subject: RE: Review Request 15021: Fixed Bug: 4899 : Added Configuration
 Support to Marvin.
Thread-Topic: Review Request 15021: Fixed Bug: 4899 : Added Configuration
 Support to Marvin.
Thread-Index: AQHO1KMlznRQU/hipE+btwnPItQ/q5oLcgwAgAEPmgCAAIkHMA==
Date: Wed, 30 Oct 2013 10:42:48 +0000
Message-ID: 
 <CB688D716959224C95995F52DE5E82420EBA654C@SINPEX01CL02.citrite.net>
References: <20131029123348.7399.29557@reviews.apache.org>
 <20131029180014.7402.69626@reviews.apache.org>
 <20131030101219.GC6243@cloud-2.local>
In-Reply-To: <20131030101219.GC6243@cloud-2.local>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

ConfigManager is as synonymous to any other configuration  facility provide=
d for products. Currently, we provide configuration as part of some file sa=
y 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 a=
dded.  We can now extend it to some more usecases if required. It has now s=
ome provision to read the config, abstract to users and obtain to test feat=
ures 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 i=
t easily inside the test features as a dictionary.  With this, all configur=
ation pertaining to run will be at one single place  and we provide a simpl=
e usage of this through dictionary.  It doesn't have any setters for that m=
atter. With this, all hard coding will be moved to configuration and users =
use the dynamic dictionary created based upon config and is more driven thr=
ough config.  We can remove current not a cleaner way of search and replace=
 with this.  =20

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 d=
ata. The user don't need to hard code this. Of a test as we are doing curre=
ntly. 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 c=
onfig.

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]=20
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 Su=
pport to Marvin.

I've given some inline comments in the patch. The ConfigManager looks too s=
imple perhaps because it's a WIP? At least the goals as you've written in y=
our 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:
> -------
>=20
>  Added Configuration Support to Marvin.
>    =20
>     1. It provides the basic configuration facilities to marvin.
>    =20
How will this be different from the current configGenerator module?
The configGenerator could read and write a structured deployment configurat=
ion. 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 plac=
ed anywhere. what would be the advantage of placing all configs in tools/ma=
rvin/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 sep=
arate
>                   it out as config at this location.

How would the ConfigParser know where to find a specific section and at wha=
t 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 s=
ection
>                   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 t=
oday 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.

>    =20
>     4. This API is provided as an additional facility under
>                   cloudstackTestClient and users can get the
>                   configuration object as similar to apiclient,dbconnecti=
on
>                   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 pro=
perties to it if it's part of the testClient via ConfigManager. Test case a=
uthors might be tempted to change configuration on the fly which might affe=
ct downstream tests in the same suite?=20

>    =20
>     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 thei=
r 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 tes=
ts 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.
>    =20
> 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 ca=
n now change all parameters before run under configuration, the test featur=
es will use configuration object and thus values, rather hard coded strings=
 which avoids replacement through shell script.
>=20
>  Also, did few naming convention changes. Its better to follow uniform na=
ming conventions. Currently, wherever iam seeing a specific notation not fo=
llowed. We are incorporating those changes.
>=20
> ToDo:
> clean up of current config at places, making configuration required for m=
arvin\tests unified and available at one place and clean up of files\code r=
elated to it.=20
>=20
>=20
> Diffs
> -----
>=20
>   tools/marvin/marvin/cloudstackTestClient.py be93f35=20
>   tools/marvin/marvin/config/setup.cfg PRE-CREATION=20
>   tools/marvin/marvin/configGenerator.py 0cfad30=20
>   tools/marvin/marvin/integration/lib/utils.py 7d662af
>=20
> Diff: https://reviews.apache.org/r/15021/diff/
>=20
>=20
> Testing
> -------
>=20
> Basic check to retrieve configuration values is done.
>=20
>=20
> Thanks,
>=20
> Santhosh Edukulla
>=20

--
Prasanna.,

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