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

Remove eostools dependency from cmsIO #184

Merged
merged 2 commits into from
Jun 20, 2017

Conversation

gpetruc
Copy link
Contributor

@gpetruc gpetruc commented Jun 13, 2017

cmsIO used to be taken from the CAF AFS installation, but it is now deprecated.
This small change to eostools avoids using cmsIO, and in the test case I've run things seem to work fine, but a few more tests would be welcome (especially if other analysis setups make a more complex use of eostools wrt the multilepton cfg I tested)

Many eostools are still littered with multiple translations from lfn to pfn and back again that to me don't seem necessary (maybe they were introduced to make sure the paths had a canonical form?) but I didn't remove them yet.
There's probably also a lot of obsolete legacy API (e.g for castor) that could be cleaned up.
A volunteer would be welcome.

@emanueledimarco
Copy link
Contributor

Ciao Giovanni,

many thanks! It works, but the file used by heppy_batch is the same, but in PhysicsTools:
PhysicsTools/HeppyCore/python/utils/eostools.py.
I just copied your version there and it runs like a charm.

@steggema
Copy link
Member

steggema commented Jun 16, 2017 via email

Copy link
Contributor

@clelange clelange left a comment

Choose a reason for hiding this comment

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

Works for me (VVResonances) as well. Thanks!

@gpetruc gpetruc merged commit 0c406f3 into CERN-PH-CMG:80X Jun 20, 2017
gpetruc added a commit to gpetruc/cmg-cmssw that referenced this pull request Jun 20, 2017
gpetruc added a commit to CERN-PH-CMG/cmg-cmssw that referenced this pull request Jun 20, 2017

def splitPFN(pfn):
"""Split the PFN in to { <protocol>, <host>, <path>, <opaque> }"""
groups = re.match("^(\w\+)://([^/]+)/(/[^?]+)(\?.*)?", pfn)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I missed one problem here, the backslash before the first + should not be there. Instead, it should be:
groups = re.match("^(\w+)://([^/]+)/(/[^?]+)(\?.*)?", pfn)
which for instance works for root://eoscms.cern.ch//eos/cms/store/cmst3/user/jngadiub/ntuple returning
root
eoscms.cern.ch
/eos/cms/store/cmst3/user/jngadiub/ntuple

lfn = eosToLFN(path)
pfn = lfnToPFN(lfn)
tokens = cmsIO.splitPFN(pfn)
tokens = splitPFN(path)
Copy link
Contributor

Choose a reason for hiding this comment

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

There is another issue here (or in general), it calls splitPFN with a /eos/... path and not with the protocol prepended, so the regex evaluation above will fail.

Choose a reason for hiding this comment

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

I think the error is later on at lines 194-196. It works when replacing those lines with:

pfn = lfnToPFN(path)
if not isEOSFile(pfn):
    runEOSCommand(pfn,'mkdir','-p')

emanueledimarco pushed a commit to emanueledimarco/cmgtools-lite that referenced this pull request May 18, 2018
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.

5 participants