[bugfix] Fix AttributeError in flexible node allocation from the Slurm backend
#1259
Conversation
* Fix `AttributeError` raised when trying to mutate a `SequenceView` object in the `filternodes` method of the slurm scheduler. * Add a corresponding unittest.
ekouts
left a comment
ekouts
left a comment
There was a problem hiding this comment.
lgtm in general. I only added a comment to change to f-strings, if possible.
|
@teojgo I can't understand where this |
Codecov Report
@@ Coverage Diff @@
## master #1259 +/- ##
=======================================
Coverage 91.39% 91.39%
=======================================
Files 84 84
Lines 12409 12416 +7
=======================================
+ Hits 11341 11348 +7
Misses 1068 1068
Continue to review full report at Codecov.
|
vkarak
left a comment
vkarak
left a comment
There was a problem hiding this comment.
I would just do the fix here and not any conversion to f-strings. it's two lines only + the comment. In your initial version, it made sense to change to f-strings the lines you were touching, but now you need not touch them for your fix.
|
It seems that this unit test is not enough. It does not break if you revert the fix. Since I'm fixing the PR, I will have a look at it as well. |
@varak yes, that's why I had the separate unit test because you need both |
|
@teojgo Yes, just realized that. I will cherry pick that commit. |
vkarak
force-pushed
the
bugfix/slurm_filter_nodes
branch
from
933f2d5 to
873d100
Compare
vkarak
changed the title
AttributeError in flexible node allocation from the Slurm backend
4 participants
Fix
AttributeErrorraised when trying to mutate aSequenceViewobject in the
filternodesmethod of the slurm scheduler.Add a corresponding unittest.
Fixes #1258