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

Pass-through source to onFilePublish() #3284

Merged
merged 5 commits into from
Nov 9, 2022
Merged

Pass-through source to onFilePublish() #3284

merged 5 commits into from
Nov 9, 2022

Conversation

BrunoGrandePhD
Copy link
Contributor

@BrunoGrandePhD BrunoGrandePhD commented Oct 11, 2022

During the nf-core hackathon, I started working on a Nextflow plugin called nf-prov, whose goal is to generate a file manifest of published files. In this initial PR, I was able to create a simple list of published files. In a follow-up PR, I switched to JSON output. Going forward, I would like to include the information about the task doing the publishing as well as input sample provenance.

However, the onFilePublish() observer interface doesn't allow published files to be linked with the task that did the publishing. There are several ways to address this, but the least obtrusive one that I identified is to pass-through the source path, which will enable the plugin to cross-reference with the task output files.

I've included a question below.

Signed-off-by: Bruno Grande <bruno.grande@sagebase.org>
@BrunoGrandePhD BrunoGrandePhD marked this pull request as ready for review October 11, 2022 14:45
BrunoGrandePhD pushed a commit to BrunoGrandePhD/nf-prov that referenced this pull request Oct 11, 2022
@sonatype-lift
Copy link

sonatype-lift bot commented Oct 11, 2022

⚠️ 7 God Classes were detected by Lift in this project. Visit the Lift web console for more details.

Bruno Grande added 2 commits October 27, 2022 08:35
Signed-off-by: Bruno Grande <bruno.grande@sagebase.org>
Signed-off-by: Bruno Grande <bruno.grande@sagebase.org>
@BrunoGrandePhD
Copy link
Contributor Author

@bentsherman: Any chance for this to get a final review and possible merge? Thanks!

Signed-off-by: Bruno Grande <bruno.grande@sagebase.org>
@pditommaso
Copy link
Member

Ok, but the destination, source should be applied consistently. I've made some comments to address that

Co-authored-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Signed-off-by: Bruno Grande <bruno.grande@sagebase.org>
@BrunoGrandePhD
Copy link
Contributor Author

@pditommaso: It does start off as Path source, Path destination here, so I wasn't sure where to make the switch. I've accepted your suggested edits.

@pditommaso
Copy link
Member

Awesome, thanks

@pditommaso pditommaso merged commit 81acc3e into nextflow-io:master Nov 9, 2022
@BrunoGrandePhD BrunoGrandePhD deleted the bgrande/onfilepublish-source branch November 9, 2022 23:00
@pditommaso pditommaso added this to the 23.04.0 milestone Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants