nextflow-io / nextflow

A DSL for data-driven computational pipelines
http://nextflow.io
Apache License 2.0
2.61k stars 605 forks source link

Fix the cleanup of local secret file #5061

Closed pditommaso closed 1 week ago

pditommaso commented 2 weeks ago

This PR fixes the cleanup of secret files created by the local provider

netlify[bot] commented 2 weeks ago

Deploy Preview for nextflow-docs-staging canceled.

Name Link
Latest commit 014b4425cc0ed06020defc712706d4d16383a37a
Latest deploy log https://app.netlify.com/sites/nextflow-docs-staging/deploys/667050cd3592ec00081e5b13
pditommaso commented 2 weeks ago

Umm, not sure anymore it should be deleted 😄

marcodelapierre commented 2 weeks ago

Why are you not convinced?
I mean, fair enough from my perspective, mostly curious. The source of truth should be store.json anyway.

If not deleted, I expect the impact on users still to be quite limited: the only ones experiencing inconsistencies would be those using a literal $ symbol in the secret. And the workaround for them is simply a one-off manual deletion of those temporary files.

pditommaso commented 2 weeks ago

The point that file could be deleted during a by a run while a concurrent execution is running.

I think the temp file is removed, then the file name should be unique i.e. using a random uuid

marcodelapierre commented 1 week ago

See now - added a random UUID to the filename.
I did not use the session uniqueID, as it seemed an overshooting to expose the session object all the way to this class.

(oops closed by accident - reopened!)

pditommaso commented 1 week ago

Ouch got messed with unrelated commits

marcodelapierre commented 1 week ago

unrelated commits should be fine now, see changelog

marcodelapierre commented 1 week ago

(still showing extra commits in commit list, as I rebased to sign off one commit)