-
Notifications
You must be signed in to change notification settings - Fork 192
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
Engine: fix bug when caching from process with nested outputs #5538
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fantastic, thanks a lot @sphuber !
_, node_clone = run.get_node(NestedOutputsProcess, a=Int(1)) | ||
|
||
assert node_clone.is_created_from_cache | ||
assert node_clone.get_cache_source() == node_original.uuid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you maybe want to check here that the structure of the cloned node outputs is as expected?
I guess it's fine, just to be safe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the test
When the execution of a process is being skipped because it has a valid cache source and caching is enabled, the process would except if the process has nested output namespaces. Since the nesting cannot be literally preserved in the database, the nesting is representing by using double underscores between each level of nesting in the link label. The problem occurs when `Process._create_and_setup_db_record` would be called which would create the outputs of the cloned node, however, it would use these "collapsed" link labels containing the double underscores. Passing these to `self.out` would cause the validation to fail because the process port validation expects the actual nesting. The solution is to restore the database nesting representation to the one expected by `Process.out`, which amounts to replacing the namespace separators.
609c4ec
to
b629ef3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks!
When the execution of a process is being skipped because it has a valid cache source and caching is enabled, the process would except if the process has nested output namespaces. Since the nesting cannot be literally preserved in the database, the nesting is representing by using double underscores between each level of nesting in the link label. The problem occurs when `Process._create_and_setup_db_record` would be called which would create the outputs of the cloned node, however, it would use these "collapsed" link labels containing the double underscores. Passing these to `self.out` would cause the validation to fail because the process port validation expects the actual nesting. The solution is to restore the database nesting representation to the one expected by `Process.out`, which amounts to replacing the namespace separators. Cherry-pick: b936da3
Fixes #5536
When the execution of a process is being skipped because it has a valid
cache source and caching is enabled, the process would except if the
process has nested output namespaces. Since the nesting cannot be
literally preserved in the database, the nesting is representing by
using double underscores between each level of nesting in the link
label.
The problem occurs when
Process._create_and_setup_db_record
would becalled which would create the outputs of the cloned node, however, it
would use these "collapsed" link labels containing the double
underscores. Passing these to
self.out
would cause the validation tofail because the process port validation expects the actual nesting.
The solution is to restore the database nesting representation to the
one expected by
Process.out
, which amounts to replacing the namespaceseparators.