Validate StdBase ValidStatus parameter and use it in DBS3Upload #11237
Conversation
|
Jenkins results:
|
|
test this please |
|
Jenkins results:
|
|
test this please |
|
Jenkins results:
|
|
Jenkins results:
|
|
test this please |
|
Jenkins results:
|
|
Jenkins results:
|
|
Jenkins results:
|
|
Jenkins results:
|
|
Jenkins results:
|
There was a problem hiding this comment.
Thanks for the change @amaltaro
I have put some minor comments inline. You may want to take a look.
| if not result: | ||
| # this should never happen, but just in case... | ||
| return {} | ||
| return result[0] |
There was a problem hiding this comment.
If possible, I'd say we better avoid this result[0] return value. It seems to me this is due to the difference between the return type from the DAO and the current method, one being a list the other a dictionary. But maybe this would require a change in the DAO itself.
There was a problem hiding this comment.
Yes, changing the DAO would be another option. But I decided to make this change just in case this DAO is also used in other parts of the code, thus avoid extra changes. Still, there is a check for a non-empty list in the lines above it.
| datasetType=dsetInfo['valid_status'], | ||
| prep_id=dsetInfo['prep_id']) | ||
| except Exception as ex: | ||
| msg = f"Unhandled exception while loading dataset for block: {blockObj.getName()}. " |
There was a problem hiding this comment.
Could it actually be an exception not only from loading the the dataset information, but also from setting it. It seems this block of code covers those both steps.
| msg = "Invalid dataset name %s" % datasetName | ||
| logging.error(msg) | ||
| raise DBSBufferBlockException(msg) | ||
| _, primary, processed, tier = datasetName.split('/') |
There was a problem hiding this comment.
Isn't the check for leading / still needed here, even though we remove the datasetType check?
There was a problem hiding this comment.
It shouldn't, since it's a dataset name. If it's broken, then there is a serious error upstream and it will fail as we try to inject it into DBS. So, I don't think we gain anything from that extra check.
|
Jenkins results:
|
Set dataset from the dbsbuffer_dataset table fix data structs and key names do not set dataset when adding files; remove unneeded checks Call setDataset for new blocks as well Fix getting datasetPath and splitting its name update error message
amaltaro
changed the title
|
Jenkins results:
|
|
test this please |
|
Jenkins results:
|
|
unit test failure is the unstable and known couchdb one. Thanks for the review, Todor. |
Fixes #11236
Status
ready
Description
This PR provides two changes to this
ValidStatusspec parameter:PRODUCTIONorVALID, the former being still the default value.dbsbuffer_datasettable, whenever a block is loaded or freshly created.NOTE that this functionality is mostly useful for workflows with growing dataset, where PPD wants to have output datasets set to VALID as soon as data is injected into DBS.
Is it backward compatible (if not, which system it affects?)
YES
Related PRs
None
External dependencies / deployment changes
None