mprove CRAB API. Add config api which act as a proxy and do the upload to CouchDB about crabserver HOT 9 CLOSED

spiga: Please Review

from crabserver.

metson: The change to Requests.py looks fine to me - Rick do you want to look over the two RequestManager changes (they also look fine but I don't know the code).

from crabserver.

metson: Dammit wrong ticket!

from crabserver.

metson: This looks fine, few comments:

• line 102 (and a few other places) has {{{return json.dumps(result)}}} - that shouldn't be necessary, just return the dictionary and the framework will encode it appropriately (you might need to set your accept header in your client, or modify the server's Formatter to only return json). https://twiki.cern.ch/twiki/bin/view/CMS/DMWTTutorialDatabaseREST#RESTFormatter might be useful.
• I think {{{postUserConfig}}} should only return the {{{DocID}}} and not the database URL/name, because you'll have a one to one crab server:couchdb. At best it's useless information (the server will always use the same database) and at worse it's information leaking.
• I think the server should just set the team as 'analysis' (maybe one day base it on VOMS role? e.g. for physics group accounting) and not expose that to the client (in a similar way as the production system ignores the user)
• the imports are a bit funny, why do

{{{import WMCore.RequestManager.RequestMaker.CheckIn as CheckIn}}}

when you can do

{{{from WMCore.RequestManager.RequestMaker import CheckIn}}}
?

• I think the second {{{HTTPError}}} in {{{postRequest}}} should probably raise a 500 error, it's a problem in the server not in the request from the client
• There's no unit tests for this, before going much further the test coverage needs to increase a lot. REST_t.py in https://svnweb.cern.ch/trac/CMSDMWM/browser/WMCore/trunk/test/python/WMCore_t/WebTools_t is probably a good place to start

I think the first three issues should be addressed before you commit, and tests need to appear ASAP.

from crabserver.

spiga: Replying to [comment:5 metson]:

This looks fine, few comments:

• line 102 (and a few other places) has {{{return json.dumps(result)}}} - that shouldn't be necessary, just return the dictionary and the framework will encode it appropriately (you might need to set your accept header in your client, or modify the server's Formatter to only return json). https://twiki.cern.ch/twiki/bin/view/CMS/DMWTTutorialDatabaseREST#RESTFormatter might be useful.

ok. fine.

• I think {{{postUserConfig}}} should only return the {{{DocID}}} and not the database URL/name, because you'll have a one to one crab server:couchdb. At best it's useless information (the server will always use the same database) and at worse it's information leaking.

Ok.you are right... I'm not sure user doesn't want to access the pset into couch.. but we'll change this later only if needed.

• I think the server should just set the team as 'analysis' (maybe one day base it on VOMS role? e.g. for physics group accounting) and not expose that to the client (in a similar way as the production system ignores the user)

ok. this answer to one of my questions. For sure we cannot ignore since the team is a key point to have all the pieces talking each other.

• the imports are a bit funny, why do

{{{import WMCore.RequestManager.RequestMaker.CheckIn as CheckIn}}}

when you can do

{{{from WMCore.RequestManager.RequestMaker import CheckIn}}}
?

mistake.

• I think the second {{{HTTPError}}} in {{{postRequest}}} should probably raise a 500 error, it's a problem in the server not in the request from the client

I'll check and then change.

• There's no unit tests for this, before going much further the test coverage needs to increase a lot. REST_t.py in https://svnweb.cern.ch/trac/CMSDMWM/browser/WMCore/trunk/test/python/WMCore_t/WebTools_t is probably a good place to start

I think the first three issues should be addressed before you commit, and tests need to appear ASAP.

Ok. a patch without tests will be attached soon.

from crabserver.

metson: Replying to [comment:6 spiga]:

• I think the server should just set the team as 'analysis' (maybe one day base it on VOMS role? e.g. for physics group accounting) and not expose that to the client (in a similar way as the production system ignores the user)

ok. this answer to one of my questions. For sure we cannot ignore since the team is a key point to have all the pieces talking each other.

Ignore might not be the right word, but having some simple sane default for all CRAB servers as the baseline seems reasonable to me.

from crabserver.

spiga: Replying to [comment:7 metson]:

Replying to [comment:6 spiga]:

• I think the server should just set the team as 'analysis' (maybe one day base it on VOMS role? e.g. for physics group accounting) and not expose that to the client (in a similar way as the production system ignores the user)

ok. this answer to one of my questions. For sure we cannot ignore since the team is a key point to have all the pieces talking each other.

Ignore might not be the right word, but having some simple sane default for all CRAB servers as the baseline seems reasonable to me.

absolutely agree!

from crabserver.

spiga: Please Review

from crabserver.

metson: (In b88f053) Improve CRAB API. Add config api which act as a proxy and do the upload to CouchDB. Fixes #1254

From: spiga [email protected]

from crabserver.