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

[WIP][8.0]refactor gfal2 and storageElement #6127

Closed
wants to merge 11 commits into from

Conversation

chaen
Copy link
Contributor

@chaen chaen commented May 30, 2022

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
  • 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

@andresailer I guess you are the most appropriate to review that. This change would be best suited for v8.0, but given that it risk of not coming out anytime soon, I am almost tempting to tag it v7r4...

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 #6127 for details

ENDRELEASENOTES

@DIRACGridBot DIRACGridBot added the alsoTargeting:integration Cherry pick this PR to integration after merge label May 30, 2022
@chaen chaen force-pushed the rel-v7r3_FEAT_refactorGfal2 branch from 3854d7e to 898a174 Compare May 31, 2022 13:07
@chaen chaen force-pushed the rel-v7r3_FEAT_refactorGfal2 branch 3 times, most recently from 93a523d to 38496cd Compare June 2, 2022 14:19
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.

The release notes need to be filled still.

Maybe this is also a chance to update the Resources/StorageElement documentation section.

src/DIRAC/Resources/Storage/StorageElement.py Outdated Show resolved Hide resolved
src/DIRAC/Resources/Storage/GFAL2_SRM2Storage.py Outdated Show resolved Hide resolved
src/DIRAC/Resources/Storage/StorageFactory.py Outdated Show resolved Hide resolved
src/DIRAC/Resources/Storage/StorageFactory.py Outdated Show resolved Hide resolved
@chaen chaen force-pushed the rel-v7r3_FEAT_refactorGfal2 branch from c4d7699 to a7ed196 Compare June 3, 2022 13:23
@@ -458,6 +458,7 @@ def __loadCFGFiles(self):
if "DIRACSYSCONFIG" in os.environ:
diracSysConfigFiles = os.environ["DIRACSYSCONFIG"].replace(" ", "").split(",")
for diracSysConfigFile in reversed(diracSysConfigFiles):
gLogger.debug("Loading file from DIRACSYSCONFIG %s" % diracSysConfigFile)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
gLogger.debug("Loading file from DIRACSYSCONFIG %s" % diracSysConfigFile)
gLogger.debug(f"Loading file from DIRACSYSCONFIG {diracSysConfigFile}")

@@ -116,8 +116,6 @@ def __init__(self, storageName, parameters):
self.stageTimeout = gConfig.getValue("/Resources/StorageElements/StageTimeout", 12 * 60 * 60)
# gfal2Timeout, amount of time it takes until an operation times out
self.gfal2Timeout = gConfig.getValue("/Resources/StorageElements/GFAL_Timeout", 100)
# set the gfal2 default protocols, e.g. used when trying to retrieve transport url
self.defaultLocalProtocols = gConfig.getValue("/Resources/StorageElements/DefaultProtocols", [])
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 referenced also in the documentation, it should be removed from there too.

@@ -51,16 +56,10 @@ def __init__(self, storageName, parameters):

self.gfal2requestLifetime = gConfig.getValue("/Resources/StorageElements/RequestLifeTime", 100)

self.__setSRMOptionsToDefault()
self.protocolsList = self.protocolParameters["OutputProtocols"]
self.log.debug("GFAL2_SRM2Storage: protocolsList = %s" % self.protocolsList)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
self.log.debug("GFAL2_SRM2Storage: protocolsList = %s" % self.protocolsList)
self.log.debug(f"GFAL2_SRM2Storage: protocolsList = {self.protocolsList}")

@chaen chaen force-pushed the rel-v7r3_FEAT_refactorGfal2 branch from 861242f to c411829 Compare August 2, 2022 09:54
@chaen chaen closed this Aug 3, 2022
@chaen chaen deleted the rel-v7r3_FEAT_refactorGfal2 branch June 11, 2024 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alsoTargeting:integration Cherry pick this PR to integration after merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants