Skip to content

[8.0] Interpret outputPath with LFN: prefix as an absolute one - #8603

Open
atsareg wants to merge 11 commits into
DIRACGrid:rel-v8r0from
atsareg:fix-outputpath
Open

[8.0] Interpret outputPath with LFN: prefix as an absolute one#8603
atsareg wants to merge 11 commits into
DIRACGrid:rel-v8r0from
atsareg:fix-outputpath

Conversation

@atsareg

@atsareg atsareg commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

This PR allows to define absolute outputPath by specifying it with the LFN: prefix.
It also allows using wild cards in outputData file names specified as LFNs

BEGINRELEASENOTES

*WorkloadManagement
NEW: JobWrapper - interpret outputPath with LFN: prefix as an absolute one
NEW: JobWrapper - allow wild cards in output LFNs

For examples look into release.notes

ENDRELEASENOTES

Comment thread src/DIRAC/WorkloadManagementSystem/JobWrapper/JobWrapper.py Outdated
@arrabito

Copy link
Copy Markdown
Contributor

Thank you. All seems fine to me.

@fstagni fstagni left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@aldbr has added a lot of tests for JobWrapper in https://github.com/DIRACGrid/DIRAC/blob/integration/src/DIRAC/WorkloadManagementSystem/JobWrapper/test/Test_JobWrapper.py.

Can you see if (some of) those can be used here? I am always somewhat scared of changing the JobWrapper.

Comment thread src/DIRAC/WorkloadManagementSystem/JobWrapper/JobWrapper.py Outdated
Comment thread src/DIRAC/WorkloadManagementSystem/JobWrapper/JobWrapper.py
Comment on lines +1126 to +1129
# If output path is given with the LFN: prefix, take it as an absolute path
elif outputPath.startswith("LFN:"):
outputPath = outputPath[4:]
basePath = ""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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 outputPath == "LFN:/some/where/some/thing.xyz"

@atsareg

atsareg commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Can you see if (some of) those can be used here? I am always somewhat scared of changing the JobWrapper.

Yes, there are many tests to JobWrapper added to the integration/9.0 . In 8.0 there are just few. Will see if I can add something here

Comment thread docs/source/UserGuide/GettingStarted/UserJobs/JDLReference/index.rst Outdated
Comment thread docs/source/UserGuide/GettingStarted/UserJobs/JDLReference/index.rst Outdated
Comment thread docs/source/UserGuide/GettingStarted/UserJobs/JDLReference/index.rst Outdated
@aldbr
aldbr requested a review from fstagni August 6, 2026 08:14
Comment on lines +111 to +113
- if given as ``"LFN:/output/path"``, it will be taken as an absolute path for
output files in the logical namespace. It is the responsibility of the user to make
sure that this path is accessible for writing for the user's data.

Copy link
Copy Markdown
Contributor

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" ?

@marianne013 marianne013 Aug 7, 2026

Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Contributor

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.

Comment thread docs/source/UserGuide/GettingStarted/UserJobs/JDLReference/index.rst Outdated
if globbedLfnList and globbedLfnList != lfnList:
self.log.info("Found a pattern in the output data LFN list, LFNs to upload are:", ", ".join(globbedLfnList))
lfnList = globbedLfnList

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My llm sees the following issues:

  1. No global deduplication: List.uniqueElements is applied per-LFN, but if multiple LFNs have overlapping glob patterns, globbedLfnList can still contain duplicates.
  2. Comparison can fail due to duplicates: globbedLfnList != lfnList might always be True if duplicates inflate the list, even when no useful expansion occurred.
  3. Order not preserved: The result order depends on input order + glob expansion order, which may be inconsistent. [we might ignore this]

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

@fstagni fstagni added the sweep:ignore Prevent sweeping from being ran for this PR label Aug 26, 2026
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.

5 participants