Added exception for any symbol apart from . in names #529
Conversation
Codecov Report
@@ Coverage Diff @@
## devel #529 +/- ##
==========================================
+ Coverage 61.52% 61.80% +0.28%
==========================================
Files 20 21 +1
Lines 2277 2299 +22
==========================================
+ Hits 1401 1421 +20
- Misses 876 878 +2
Continue to review full report at Codecov.
|
|
@iparask: What is the performance overhead? |
|
I measured the added overhead and the times in seconds are in the following table. Each row shows the number of Tasks. Column
|
| actual_value=d['name'], | ||
| expected_value="Valid object names can " + | ||
| "contains letters, numbers and '.'. Any " | ||
| "other character is not allowed") |
There was a problem hiding this comment.
Would it be worth to store these messages in a dedicated data structure? See that you repeated the same message twice, and now we have to fix the same error in both instances of the same message. More in general, should we template exceptions? Repeating attribute and other parameters all the time might be undesirable.
There was a problem hiding this comment.
I created a constants file to keep constants in general. I placed the message there. Some exceptions are already templated, but I am not a fan because it confuses me a bit. I can do it if you think it is important.
There was a problem hiding this comment.
This should only be done for messages we use a lot IMHO.
There was a problem hiding this comment.
This message is used ~8 times and each instance had the same typo. Happy for you to have the last word.
There was a problem hiding this comment.
@iparask, about the templating, happy to follow your preference as you are the main developer of EnTK. I would probably stress the need of consistency so that we do not end up with a mix of (non)templated exceptions?
This PR fixes #392.
It raises an exception if any symbol other than '.' is used in the name of a task, stage, or pipeline.