rucio icon indicating copy to clipboard operation
rucio copied to clipboard

Transmogrifier improvements : Closes #5624, #5600, #5599, #5564

Open cserf opened this issue 3 years ago • 6 comments

This PR is fixing multiple issues :

  • Transmogrifier code restructuring. Cut the monolithic main function into smaller bits. Closes : https://github.com/rucio/rucio/issues/5624
  • Support injection of delayed rules. Closes : https://github.com/rucio/rucio/issues/5600
  • Overriding the subscription account if an account is specified into replication rules : Closes https://github.com/rucio/rucio/issues/5599
  • Support of wildcard for argument copies to create as many copies of RSEs under a specific RSE expression : Closes : https://github.com/rucio/rucio/issues/5564
  • Introduce tests that were missing for some functionalities (e.g. chained subscriptions)

cserf avatar Jun 14 '22 11:06 cserf

The first commit is extremely difficult to review. It mixes new functionality; with code move; with refactoring; and code formatter visual changes.

rcarpa avatar Jun 15 '22 11:06 rcarpa

I have to agree with Radu, this is very hard to review since it is really unclear what changed and what was just hit by the code formatter. Although we don't have a guideline/rule here, I find this "irregular" application of code formatters bothersome - this is a discussion we need to have. In any way, the application of the formatter should be broken into a separate commit.

bari12 avatar Jul 03 '22 19:07 bari12

Just to clarify. There is no additional code formatting in the 1st commit. The code was already formatted before (see https://github.com/rucio/rucio/commit/cbb5a31d1ad9dcb82aad65d4a2cbb628b9f7d779). This is just a massive restructuring of the code and there is nothing that is coming from black. Now I get the argument from Radu and next time I will try to do smaller changes.

cserf avatar Jul 06 '22 09:07 cserf

Thanks Cedric, I will add it for 1.29.1

bari12 avatar Jul 06 '22 14:07 bari12

Please rebase this

bari12 avatar Jul 29 '22 12:07 bari12

@bari12 Code was rebased

cserf avatar Aug 12 '22 14:08 cserf