dmwm / WMCore

Core workflow management components for CMS.
Apache License 2.0
45 stars 106 forks source link

Different dict information displayed in the ReqMgr and the injected one [taskChain] #3891

Closed amaltaro closed 11 years ago

amaltaro commented 12 years ago

Dear Developers,

it seems there is an inconsistency between the dictionary injected into ReqMgr to create a workflow/request and the information that is shown in the ReqMgr page.

Here [1] is the dictionary used in one injection and here [2] is the dictionary that I see in the ReqMgr page. As we can see, Task1 and Task2 has no value in the webpage.

I'm using testbed and here is the requestName: amaltaro_RVRunMinBias2011A_120618_111224_5981

Thanks, Alan.

[1] {'AcquisitionEra': 'ReleaseValidation', 'CMSSWVersion': 'CMSSW_5_3_2', 'CouchDBName': 'reqmgr_config_cache', 'CouchURL': 'https://cmsweb-testbed.cern.ch/couchdb', 'GlobalTag': u'GR_R_53_V2::All', 'Group': 'DATAOPS', 'ProcessingVersion': 'v1', 'RequestString': 'RVRunMinBias2011A', 'RequestType': 'TaskChain', 'Requestor': 'amaltaro', 'ScramArch': 'slc5_amd64_gcc462', 'SiteWhitelist': ['T1_CH_CERN', 'T1_US_FNAL'], 'Task1': {'ConfigCacheID': '2a9ee3f68cf5c0f9f153db9c9455f560', 'InputDataset': '/MinimumBias/Run2011A-v1/RAW', 'RunWhitelist': [165121], 'SplittingAlgorithm': 'LumiBased', 'SplittingArguments': {'lumis_per_job': 10}, 'TaskName': 'HLTD'}, 'Task2': {'ConfigCacheID': '2a9ee3f68cf5c0f9f153db9c9456038b', 'InputFromOutputModule': u'RAWoutput', 'InputTask': 'HLTD', 'SplittingAlgorithm': 'LumiBased', 'SplittingArguments': {'lumis_per_job': 10}, 'TaskName': 'RECODst3'}, 'TaskChain': 2}

[2] https://cmsweb-testbed.cern.ch/reqmgr/reqMgr/request?requestName=amaltaro_RVRunMinBias2011A_120618_111224_5981

stuartw commented 12 years ago

swakef: The information is there - it simply isn't being shown properly.

If you view the source of that page you get: {{{ 'Task1': <WMCore.Configuration.ConfigSection object at 0x104ccb50>, 'Task2': <WMCore.Configuration.ConfigSection object at 0x104ccc10>, }}}

The task configs need to be expanded.

zdenekmaxa commented 12 years ago

maxa: Investigating the problem:

{{{ https://cmsweb-testbed.cern.ch/reqmgr/reqMgr/request?requestName=amaltaro_RVRunMinBias2011A_120618_111224_5981 -> downloading the workload from couch -> alan-schema.pkl import cPickle from WMCore.WMSpec.WMWorkload import WMWorkloadHelper handle = open("alan-schema.pkl") data = cPickle.load(handle) helper = WMWorkloadHelper(data) schema = helper.data.request.schema schema.dictionary_() # ConfigSection instance

-> does not dictionarise internal ConfigSection children

}}}

The patch fixes the problem by implementing ConfigSection.dictionary_wholetree() method. The old method ConfigSection.dictionary_() which does not bother about internal ConfigSection children remained since esp. WebTools.Root.py enjoys iterating over items and doing isinstance(x, ConfigSection) itself ...

Steve, please review.

stuartw commented 12 years ago

swakef: I'd be tempted to add a recurse = True argument to dictionary_() instead of adding a new method, as a fully resolved dict is what i would imagine we would want in most cases.

stuartw commented 12 years ago

swakef: Replying to [comment:4 swakef]:

I'd be tempted to add a recurse = True argument to dictionary_() instead of adding a new method, as a fully resolved dict is what i would imagine we would want in most cases.

You could have this value as False by default - but to be honest i'd rather see it recurse by default and change any client code which breaks as a result.

sfoulkes commented 12 years ago

sfoulkes: Replying to [comment:5 swakef]:

Replying to [comment:4 swakef]:

I'd be tempted to add a recurse = True argument to dictionary_() instead of adding a new method, as a fully resolved dict is what i would imagine we would want in most cases.

You could have this value as False by default - but to be honest i'd rather see it recurse by default and change any client code which breaks as a result.

It really should recurse by default. The lack of recursive encoding of dictionaries in the ConfigSection stuff has caused a number of people lots of problems. It should just be fixed.

sfoulkes commented 12 years ago

sfoulkes: I should have read the ticket closer because I thought this was about the inability to use nested dictionaries in a ConfigSection attribute. Anyway, Zdenek's patch looks OK, I think expanding the method to do this by default would be best. New patch combines what I did with what Zdenek did. Stu has the final call.

zdenekmaxa commented 12 years ago

maxa: I was about to do just a single dictionary_(recurse = True) as suggested but it seems that Steve has already been working on this, though I think a wrong patch has been uploaded (mine and Steve's don't really differ).

btw, indeed I was surprised to see that client code (Root.py) does the inspection of the configuration instance itself ...

sfoulkes commented 12 years ago

sfoulkes: (In c41d019bb03dc7f45f90b0967e9695fff5a8704a) Handle some issue with the ConfigSection stuff with nested containers and nested ConfigSections. Fixes #3891.

From: Zdenek Maxa maxa@cern.ch Signed-off-by: Steve Foulkes sfoulkes@fnal.gov

sfoulkes commented 12 years ago

sfoulkes: Sorry, didn't mean to commit this.

zdenekmaxa commented 12 years ago

maxa: no problem. I am confused here. I meant to implement what you and Stu had suggested but you uploaded a patch containing only my modifications ... Let me know if I am supposed to take any action ...