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

Fix hash when CmdEvalParam as output #5517

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jorgee
Copy link
Contributor

@jorgee jorgee commented Nov 18, 2024

close #5470

Adds the CmdEvalParams in the task outputs as a key to compute the hash

@jorgee jorgee linked an issue Nov 18, 2024 that may be closed by this pull request
Copy link

netlify bot commented Nov 18, 2024

Deploy Preview for nextflow-docs-staging canceled.

Name Link
🔨 Latest commit 7971fc1
🔍 Latest deploy log https://app.netlify.com/sites/nextflow-docs-staging/deploys/679bd25216efe30008d8374f

@jorgee jorgee force-pushed the 5470-pipeline-uses-cached-process-despite-edited-eval-script branch from 40ecdc4 to d605b2b Compare November 18, 2024 13:44
@bentsherman bentsherman added the cache-breaking Changes that will break everyone's task cache label Dec 13, 2024
@jorgee jorgee requested a review from pditommaso January 30, 2025 19:29
@pditommaso
Copy link
Member

This alter the assumption the task is idempotent. The task hash should only rely on the inputs not on the outputs

@pditommaso
Copy link
Member

I see the point it could be considered as part of the execution script, but still in principle it should not be needed to re-run the task

@bentsherman
Copy link
Member

Considering that eval is a shorthand for export an output env var in the script, it is essentially part of the script and therefore part of the task identity.

I think it is possible to evaluate the task outputs without "resolving" or "collecting" them, before computing the task hash. Then you can see if any output evals were declared and include them as if they were part of the script

@pditommaso pditommaso force-pushed the master branch 2 times, most recently from 5a93547 to 27345a6 Compare February 10, 2025 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cache-breaking Changes that will break everyone's task cache
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pipeline uses cached process despite edited eval script
3 participants