-
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
Process
: raise when exposed_outputs
gets non-existing namespace
#5265
Process
: raise when exposed_outputs
gets non-existing namespace
#5265
Conversation
def7426
to
a41716c
Compare
Codecov Report
@@ Coverage Diff @@
## develop #5265 +/- ##
===========================================
+ Coverage 82.02% 82.02% +0.01%
===========================================
Files 533 533
Lines 38255 38257 +2
===========================================
+ Hits 31373 31376 +3
+ Misses 6882 6881 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Up till now, a call to `exposed_outputs` providing a `namespace` as argument that doesn't exist, i.e., no outputs were ever exposed in that namespace, would simply be a no-op. Given that this would often be because the namespace was accidentally misspelled, the error would go unnoticed and the process would finish without error message. To prevent this situation that is difficult to debug, the method will now raise a `KeyError` if the top-level namespace does not exist.
a41716c
to
5329524
Compare
@janssenhenning I think this should fix the problem as now it will throw an exception. Let us know what you think of that solution. |
@sphuber Thank you for the quick fix. Judging from the test this is the behaviour I would like 👍. |
No it does not. This would also be more tricky to do. For example, imagine a process that exposes a process without namespace and a process with a namespace. If the parent process then calls |
Okay I see. The current solution seems fine to me |
@ramirezfranciscof could you give this a look? It is not a big one and would be good to get merged for v2.0 |
@chrisjsewell @ramirezfranciscof let me know if you want to still give this a look. Otherwise I am merging tomorrow |
Fixes #5261
Up till now, a call to
exposed_outputs
providing anamespace
asargument that doesn't exist, i.e., no outputs were ever exposed in that
namespace, would simply be a no-op. Given that this would often be
because the namespace was accidentally misspelled, the error would go
unnoticed and the process would finish without error message.
To prevent this situation that is difficult to debug, the method will
now raise a
KeyError
if the top-level namespace does not exist.