Include remote path in FileHolder

[rcannood]

Alternative solution to #5907 to implement #5905.

I didn't change any other calls of FileHolder except the one @bentsherman suggested in #5907.

[netlify]

✅ Deploy Preview for nextflow-docs-staging canceled.

Name	Link
🔨 Latest commit	e62f851
🔍 Latest deploy log	https://app.netlify.com/sites/nextflow-docs-staging/deploys/67f7dfd18f8cf90008cd4b01

[rcannood]

Looking at the constructor of FileHolder, it's not straight forward to just add a Path origPath = null to the arguments of the constructor.

    FileHolder( Path inputFile ) {
        assert inputFile
        this.sourceObj = inputFile
        this.storePath = real(inputFile)
        this.stageName = norm(inputFile.getFileName())
    }

    FileHolder( def origin, Path path ) {
        assert origin != null
        assert path != null

        this.sourceObj = origin
        this.storePath = path
        this.stageName = norm(path.getFileName())
    }

    protected FileHolder( def source, Path store, def stageName ) {
        this.sourceObj = source
        this.storePath = store
        this.stageName = norm(stageName)
    }

I just added a class RemoteFileHolder which has a different set of constructors. However there are many more alternative solutions -- e.g. by adding a setter to FileHolder so it doesn't need to get passed to the constructor.

@bentsherman do you have any suggestions on which solution you think would be best?

[bentsherman]

Looks good. Thanks for your help 😄

Do you understand how to use this in your setup? I will also try it with nf-prov. Let me know if you need help with how to test on your end

[rcannood]

I wonder whether something needs to be updated in the constructors:

    FileHolder( Path inputFile ) {
        assert inputFile
        this.sourceObj = inputFile
        this.storePath = real(inputFile)
        this.stageName = norm(inputFile.getFileName())
    }
    FileHolder( def origin, Path path ) {
        assert origin != null
        assert path != null
        this.sourceObj = origin
        this.storePath = path // ← does this need to become `real(path)`?
        this.stageName = norm(path.getFileName())
    }
    protected FileHolder( def source, Path store, def stageName ) {
        this.sourceObj = source
        this.storePath = store // ← does this need to become `real(store)`?
        this.stageName = norm(stageName)
    }

---

Cfr the use case: I'm working on a nf-lamin plugin which allows users to run pre-existing Nextflow workflows and create records in the LaminDB regarding:

• Which workflow was run
• Information about the run itself
• List of input files (e.g. s3://path/to/input/*.fastq.gz)
• List of output files (e.g. s3://path/to/output/*.bam)

This package will use the observer to detect various events and then pass that information on to LaminDB.

[pditommaso]

I think you should have a look at this on-going effort #5715

[bentsherman]

I think this PR will benefit all of us -- the provenance store, nf-prov, and nf-lamin. Fundamentally it's about being able to trace staged files to their remote source, which we currently cannot do

The only snag is that this PR currently will change the task hash. When a task has a foreign input file, the locally staged path is used for the hash, not the remote path. This might actually be better, as it would decouple the task hash from the foreign file staging. But I wonder if the locally staged path was used for a reason

[bentsherman]

I have it working with nf-prov! nextflow-io/nf-prov#45

You can see the before/after in the linked PR. Turns out it doesn't affect the task hash. I think this is because the FilePorter uses the original URL to construct the hash for the stage directory, and since the file is hashed by either base name + size + timestamp (standard) or contents (deep), the remote file and staged file produce the same hash

@rcannood to your point about doing real(path), that function is mainly used to handle inputs that came from an upstream task, so it should not be needed here.

[rcannood]

I see, thanks!

@bentsherman Any updates on the state of this PR? 🙇

[pditommaso]

Checking this, and I'm bit concerned on the different logic of the two constructors

    FileHolder( Path inputFile ) {
        assert inputFile
        this.sourceObj = inputFile
        this.storePath = real(inputFile)
        this.stageName = norm(inputFile.getFileName())
    }

    FileHolder( def origin, Path path ) {
        assert origin != null
        assert path != null

        this.sourceObj = origin
        this.storePath = path
        this.stageName = norm(path.getFileName())
    }

Above all, note the different logic for storePath. I believe this will change the handle on symlink above all.

If the goal is to bring the source path with could try using the first constructor and adding a method to update the source path

[rcannood]

I understand your concern.

Would you rather this get implemented as follows?

                def holder = new FileHolder(target)
                holder.setSourceObj(origin)

[pditommaso]

I've tried adding this

    FileHolder withSourcePath(Path source) {
        new FileHolder(source, this.storePath, this.stageName)
    }

The problem however is that this invalid existing caches for pipelines depending on staged files