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

Assign the correct activity for Tape_Test RSEs #11067

Merged
merged 1 commit into from
Apr 12, 2022

Conversation

jhonatanamado
Copy link
Contributor

Fixes #11046

Status

Tested with a replay

Description

Add the proper activity for the T0-Agent when is subscribing data to Tape_Test rses. Small changes on how this code recognize a Tape_Test RSE. With this changes, the rule replica subscribed to T0_CH_CERN_Tape_Test gives the following:

rucio rule-info 0035c69b588045769195f241df939406
Id:                         0035c69b588045769195f241df939406
Account:                    tier0_replay
Scope:                      cms
Name:                       /ExpressCosmics/Tier0_REPLAY_2022-v98/RAW
RSE Expression:             T0_CH_CERN_Tape_Test
Copies:                     1
State:                      REPLICATING
Locks OK/REPLICATING/STUCK: 0/1/0
Grouping:                   ALL
Expires at:                 None
Locked:                     True
Weight:                     None
Created at:                 2022-04-05 03:33:03
Updated at:                 2022-04-05 03:33:55
Error:                      None
Subscription Id:            None
Source replica expression:  None
Activity:                   T0 Tape
Comment:                    T0 WMAgent automatic container rule
Ignore Quota:               False
Ignore Availability:        False
Purge replicas:             False
Notification:               NO
End of life:                None
Child Rule Id:              None

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

Maybe

Related PRs

None

External dependencies / deployment changes

None

@cmsdmwmbot
Copy link

Jenkins results:

  • Python3 Unit tests: succeeded
    • 7 tests added
    • 3 changes in unstable tests
  • Python3 Pylint check: failed
    • 12 warnings and errors that must be fixed
    • 6 warnings
    • 11 comments to review
  • Pylint py3k check: succeeded
  • Pycodestyle check: succeeded

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/12986/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.

Thanks for providing this patch, Jhonatan. Please see a couple of comments along the code.

@@ -465,7 +465,7 @@ def insertContainerRules(self):
grouping="ALL",
comment=ruleComment,
meta=self.metaData)
if not rseName.endswith("_Tape"):
if "Tape" not in rseName:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a risky check, and given that we never produce data at a Tape endpoint, I would suggest to revert this change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, the way how this check has been implemented originally is better.

@@ -529,7 +529,7 @@ def _activityMap(self, rseName):
"""
if not self.isT0agent and not rseName.endswith("_Tape"):
return "Production Output"
elif self.isT0agent and rseName.endswith("_Tape"):
elif self.isT0agent and "Tape" in rseName:
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest a check like:

elif self.isT0agent and (rseName.endswith("_Tape") or rseName.endswith("_Tape_Test"):

People are very creative and I wouldn't be surprised to, one day, see a Disk endpoint with Tape in its name :-D

Copy link
Contributor

Choose a reason for hiding this comment

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

I can second Alan's suggestion here.

Copy link
Contributor

@todor-ivanov todor-ivanov left a comment

Choose a reason for hiding this comment

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

Thanks Jhonatan
I have basically agreed with Alan's suggestions here too. Please take a look the comments inline.

@@ -529,7 +529,7 @@ def _activityMap(self, rseName):
"""
if not self.isT0agent and not rseName.endswith("_Tape"):
return "Production Output"
elif self.isT0agent and rseName.endswith("_Tape"):
elif self.isT0agent and "Tape" in rseName:
Copy link
Contributor

Choose a reason for hiding this comment

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

I can second Alan's suggestion here.

@@ -465,7 +465,7 @@ def insertContainerRules(self):
grouping="ALL",
comment=ruleComment,
meta=self.metaData)
if not rseName.endswith("_Tape"):
if "Tape" not in rseName:
Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, the way how this check has been implemented originally is better.

@cmsdmwmbot
Copy link

Jenkins results:

  • Python3 Unit tests: succeeded
    • 7 tests added
  • Python3 Pylint check: failed
    • 12 warnings and errors that must be fixed
    • 6 warnings
    • 11 comments to review
  • Pylint py3k check: succeeded
  • Pycodestyle check: succeeded

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

@cmsdmwmbot
Copy link

Jenkins results:

  • Python3 Unit tests: failed
    • 1 new failures
    • 7 tests added
    • 1 changes in unstable tests
  • Python3 Pylint check: failed
    • 12 warnings and errors that must be fixed
    • 6 warnings
    • 11 comments to review
  • Pylint py3k check: succeeded
  • Pycodestyle check: succeeded

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

Add the proper activity for the T0-Agent when is subscribing data to Tape_Test rses.
@cmsdmwmbot
Copy link

Jenkins results:

  • Python3 Unit tests: succeeded
    • 7 tests added
    • 1 changes in unstable tests
  • Python3 Pylint check: failed
    • 12 warnings and errors that must be fixed
    • 6 warnings
    • 11 comments to review
  • Pylint py3k check: succeeded
  • Pycodestyle check: succeeded

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

Copy link
Contributor

@todor-ivanov todor-ivanov left a comment

Choose a reason for hiding this comment

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

Thanks @jhonatanamado
looks good to me

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.

@jhonatanamado this is even a better implementation than the one suggested. I wasn't aware we could check it against a tuple! Thanks

@amaltaro amaltaro merged commit ce7fd8b into dmwm:master Apr 12, 2022
@jhonatanamado jhonatanamado deleted the fix_T0Activity branch April 12, 2022 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong activity for subscriptions to Tape_Test sites
4 participants