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

[8.0] Refactor gfal2 and storageElement #6283

Merged
merged 13 commits into from
Aug 15, 2022

Conversation

chaen
Copy link
Contributor

@chaen chaen commented Aug 3, 2022

(Replacement of #6127)

Although this PR has the potential to break things, I do not think it will. If you were in a situation that would break, it means:

  • you are doing very advanced things with StorageElements, and you would recognize it
  • your very advanced things work by chance, and it was just waiting to break. So this will make it deterministic.

It contains 4 main changes:

  • simple migration to pytest for the Test_StorageFactory and Test_Resources_GFAL2
  • When two storage plugins provide the same protocol, give priority to the local one
  • Make an explicit distinction between the pluginName (which is the effectively used object to perform operations) and the protocolSectionName (which is the name of the section in the CS used to configure a given protocol).
  • The DefaultProtocols option in the StorageElements section of the CS is unused anymore by GFAL2_SRM2 as redundant with outputProtocols

Historically, we never really had to deal with such cases, but now we have situations where the same storage plugin can be used for different protocols, and hence corresponds to two different protocol sections. We thus need to be able to make the difference.

It contains an API change for the StorageElement object (that will go in the wiki):

  • se.storages was a list, it is now a dict. The keys are the protocol section name
  • se.localPlugins becomes se.localProtocolSections
  • se.remotePlugins becomes se.remoteProtocolSections
  • se.getPlugins() becomes se.getProtocolSections
  • se.getRemotePlugins() becomes se.getRemoteProtocolSections()
  • se.getLocalPlugins() becomes se.getLocalProtocolSections()
  • se.getStorageParameters: plugin parameter is replaced with protocolSection

In a second stage, I will tackle #4744 once for all

BEGINRELEASENOTES

*Resources

CHANGE: GFA2_SRM2 does not use anymore StorageElements/DefaultProtocols option in the CS (relies on specific OutputProtocols config)
CHANGE: explicit distinction between PluginName and ProtocolSection name in all the StorageElement components
CHANGE: API change for the StorageElement. See #6283 for details
CHANGE: StorageElement favors local plugins

ENDRELEASENOTES

@chrisburr chrisburr changed the title [WIP][8.0]refactor gfal2 and storageElement [8.0] Refactor gfal2 and storageElement Aug 4, 2022
@chrisburr chrisburr marked this pull request as draft August 4, 2022 12:40
@chaen chaen marked this pull request as ready for review August 10, 2022 09:17
@andresailer
Copy link
Contributor

Maybe this is also a chance to update the Resources/StorageElement documentation section.
Also this comment #6127 (comment) ?

Copy link
Contributor

@andresailer andresailer left a comment

Choose a reason for hiding this comment

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

Looks good to me, maybe some documentation updates are needed though

@chaen
Copy link
Contributor Author

chaen commented Aug 10, 2022

Thanks, I'll check Federico's review that I missed. And I will go over the StorageElement documentation. I will need another PR anyway

@chaen
Copy link
Contributor Author

chaen commented Aug 11, 2022

OK, I've updated the documentation and applied Federico's comment.
I think this is ready to go as is. I will most probably come up with another PR for continuing that revamp work

@fstagni fstagni merged commit a24d431 into DIRACGrid:integration Aug 15, 2022
@DIRACGridBot DIRACGridBot added the sweep:ignore Prevent sweeping from being ran for this PR label Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sweep:ignore Prevent sweeping from being ran for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants