Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow numbers in data tier names #11930

Merged
merged 1 commit into from
Mar 18, 2024
Merged

Conversation

germanfgv
Copy link
Contributor

@germanfgv germanfgv commented Mar 14, 2024

Fixes #11931

Status

ready

Description

In coming weeks Tier-0 will introduce L1SCOUT data tier. This requieres numbers in data tier names

Is it backward compatible (if not, which system it affects?)

YES

External dependencies / deployment changes

@cmsdmwmbot
Copy link

Jenkins results:

  • Python3 Unit tests: failed
    • 2 new failures
    • 1 changes in unstable tests
  • Python3 Pylint check: failed
    • 4 warnings and errors that must be fixed
    • 94 comments to review
  • Pylint py3k check: succeeded
  • Pycodestyle check: succeeded
    • 84 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/14972/artifact/artifacts/PullRequestReport.html

@vkuznet
Copy link
Contributor

vkuznet commented Mar 14, 2024

I hope that everybody understand that it will not be sufficient to patch the Lexicon.py since it is not used in DBS Go-based code (https://github.com/dmwm/dbs2go). Therefore, in order to make this data-tier acceptable in DBS someone should patch the Go-based code and update dbs2go in production. Current maintainer of DBS Go code is @todor-ivanov

@vkuznet
Copy link
Contributor

vkuznet commented Mar 14, 2024

For the record in dbs2go codebase I ported Lexicon.py rules into independent JSON files. They are located at https://github.com/dmwm/dbs2go/tree/master/static and you can find tier patterns in the following lines:

static/lexicon_writer.json
71:    "name": "data_tier_name",

static/lexicon_writer_negative.json
27:    "data_tier_name": [

static/lexicon_writer_positive.json
14:    "data_tier_name": [

static/lexicon_reader_positive.json
14:    "data_tier_name": [

static/lexicon_reader.json
81:    "name": "data_tier_name",

static/lexicon_reader_negative.json
27:    "data_tier_name": [

All patterns should be properly adjusted. For the record, WM has long standing issue (#10614) about porting Lexicon rules from python code to independent format. Since we didn't address it now we must be careful with Lexicon changes as for python based code it is Lexicon.py file while DBS can't use it and therefore it relies on independent JSON format I pointed out above.

@todor-ivanov
Copy link
Contributor

Hi @vkuznet

I'll look into that ASAP

@amaltaro
Copy link
Contributor

amaltaro commented Mar 15, 2024

I think we should take this opportunity and make the dataset regex consistent with the datatier field as well, both in WMCore and in DBS.

For instance, DBS defines (I guess it's the same as in the Lexicon) the dataset regex with a datatier field of up to 50 chars:
https://github.com/dmwm/dbs2go/blob/master/static/lexicon_writer.json#L29

while the datatier field is defined with up to 99 chars:
https://github.com/dmwm/dbs2go/blob/master/static/lexicon_writer.json#L73

The same situation applies to WMCore, which defines the datatier field in the dataset regex to be smaller than 50 chars:
https://github.com/dmwm/WMCore/blob/master/src/python/WMCore/Lexicon.py#L205

@germanfgv I would suggest to update the line above to up to 99 chars as well.
@todor-ivanov can you please check the length of the dataset name in the DBS table? I guess it will be >= 301, but we better confirm it before updating this in WMCore.

@germanfgv
Copy link
Contributor Author

@germanfgv I would suggest to update the line above to up to 99 chars as well. @todor-ivanov can you please check the length of the dataset name in the DBS table? I guess it will be >= 301, but we better confirm it before updating this in WMCore.

I updated both SEARCHDATASET_RE and DATASET_RE

@cmsdmwmbot
Copy link

Jenkins results:

  • Python3 Unit tests: succeeded
    • 2 changes in unstable tests
  • Python3 Pylint check: failed
    • 4 warnings and errors that must be fixed
    • 94 comments to review
  • Pylint py3k check: succeeded
  • Pycodestyle check: succeeded
    • 84 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/14975/artifact/artifacts/PullRequestReport.html

Copy link
Contributor

@amaltaro amaltaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, German. It looks good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for L1SCOUT data tier
5 participants