-
Notifications
You must be signed in to change notification settings - Fork 190
[8.0] Interpret outputPath with LFN: prefix as an absolute one #8603
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
base: rel-v8r0
Are you sure you want to change the base?
Changes from all commits
a01c892
90f19a6
f625e50
cb1a21f
f850de6
0b048ad
f3a0c39
686ce76
011839b
e567fe8
623c71b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -951,9 +951,24 @@ def __transferOutputDataFiles(self, outputData, outputSE, outputPath): | |
| else: | ||
| nonlfnList.append(out) | ||
|
|
||
| # Check whether list of outputData has a globbable pattern | ||
| # Check whether the list of LFNs has globbable patterns | ||
| globbedLfnList = [] | ||
| for lfn in lfnList: | ||
| lfnPath = os.path.dirname(lfn) | ||
| lfnLocal = os.path.basename(lfn) | ||
| globbedLfnList += [os.path.join(lfnPath, gLfn) for gLfn in getGlobbedFiles(lfnLocal)] | ||
|
|
||
| if globbedLfnList: | ||
| globbedLfnList = List.uniqueElements(globbedLfnList) | ||
| if globbedLfnList != lfnList: | ||
| self.log.info( | ||
| "Found a pattern in the output data LFN list, LFNs to upload are:", ", ".join(globbedLfnList) | ||
| ) | ||
| lfnList = globbedLfnList | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My llm sees the following issues:
Better approach: globbedLfnList = []
for lfn in lfnList:
lfnPath = os.path.dirname(lfn)
lfnLocal = os.path.basename(lfn)
globbedLfnList += [os.path.join(lfnPath, gLfn) for gLfn in getGlobbedFiles(lfnLocal)]
if globbedLfnList:
globbedLfnList = List.uniqueElements(globbedLfnList)
if globbedLfnList != lfnList:
self.log.info("Found a pattern in the output data LFN list, LFNs to upload are:", ", ".join(globbedLfnList))
lfnList = globbedLfnList |
||
| # Check whether the list of outputData has a globbable pattern | ||
|
fstagni marked this conversation as resolved.
|
||
| globbedOutputList = List.uniqueElements(getGlobbedFiles(nonlfnList)) | ||
| if globbedOutputList != nonlfnList and globbedOutputList: | ||
| if globbedOutputList and globbedOutputList != nonlfnList: | ||
| self.log.info( | ||
| "Found a pattern in the output data file list, files to upload are:", ", ".join(globbedOutputList) | ||
| ) | ||
|
|
@@ -1113,6 +1128,10 @@ def __getLFNfromOutputFile(self, outputFile, outputPath=""): | |
| # If output path is given, append it to the user path and put output files in this directory | ||
| if outputPath.startswith("/"): | ||
| outputPath = outputPath[1:] | ||
| # If output path is given with the LFN: prefix, take it as an absolute path | ||
| elif outputPath.startswith("LFN:"): | ||
| outputPath = outputPath[4:] | ||
| basePath = "" | ||
|
Comment on lines
+1131
to
+1134
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this block be before the previous 2 lines? For the case when |
||
| else: | ||
| # By default the output path is constructed from the job id | ||
| subdir = str(int(self.jobID / 1000)) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to double check this. What if the user is indeed "not prevented" to upload to such location, but there are no effective policies preventing it? For example, is a simple user prevented from specifying
"LFN:/lhcb/user/a/anotheruser"?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does prevent it now, if anything ? We've been getting around this restriction for ever by using dirac-dms-add-file directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not trying to fix the whole loose security system of the grid, because as we know "the tokens will solve that" ™️
But at least we can try to fix one such use case server side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you are feeling ambitious ;-). But I have never seen an incident like that.