Skip to content

A possible issue with not deepcopying job kwargs#860

Open
vineetbansal wants to merge 2 commits intomaterialsproject:mainfrom
vineetbansal:vb/job_kwargs_ref
Open

A possible issue with not deepcopying job kwargs#860
vineetbansal wants to merge 2 commits intomaterialsproject:mainfrom
vineetbansal:vb/job_kwargs_ref

Conversation

@vineetbansal
Copy link
Contributor

As we discovered when working towards another PR, mutating one job's config mutates all others created by that decorator. This is because the @job decorator keeps a reference to job_kwargs instead of deep-copying it. This may or may not be an issue, and may just need explicit documentation.

This PR introduces an xfail test to demonstrate this behavior. The solution suggested by @gpetretto which should solve this is:

--- a/src/jobflow/core/job.py
+++ b/src/jobflow/core/job.py
@@ -184,6 +184,7 @@ def job(
     --------
     Job, .Flow, .Response
     """
+    from copy import deepcopy

     def decorator(func):
         from functools import wraps
@@ -214,7 +215,7 @@ def job(
                         args = args[1:]

             return Job(
-                function=f, function_args=args, function_kwargs=kwargs, **job_kwargs
+                function=f, function_args=args, function_kwargs=kwargs, **deepcopy(job_kwargs)
             )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant