chaostoolkit-kubernetes
chaostoolkit-kubernetes copied to clipboard
Terminate pod return value
Hi,
this PR would add a return value to the terminate_pods
action. I found it quite helpful to get this kind of information persisted in the journal.json document for future reference and further analysis. It would also address #60 .
Since it seems only the return value (or exception) of an action will be stored, I would suggest to store the already available pod names in an array and return it.
Tests have been enhanced to include checks of the return value.
Looking forward to get some thoughts on this suggestion.
Cheers, Hendrik
Hello @hendrikKahl, this has been so long and I haven't paid attention enough to this PR. I apologise.
I see you had made a merge from master, I usually prefer a rebase. Would you be able to join me here so we can discuss if you still have bandwith to look at this PR again?
Hi @Lawouach no worries, thanks for looking into the PR :)
I'd be happy to discuss the next steps. Please let me know, what you have in mind.
Thanks for tolerating my slowness here :)
The best, if you could, start from master and only make a commit for your changes? Either you still have this branch somewhere and try a rebase, or feel free to create a new PR altogether. Whatever is simpler .
Please excuse my lack of knowledge and understanding here, but I might need a hint or two to proceed.
I went back to my fork, branched off before the merge commit, rebased to incorporate latest changes from upstream and then cherry-picked my changes into this local branch.
While the commit tree look ok now, rebasing seems to alter the authors section of each commit while re-applying them on my fork. Not sure, if this would be kept, when I open a new PR...
The other option I could try, would be to create a branch in my fork without the merge commit and without any upstream changes. Again I could cherry-pick my change and reference this branch in a separate PR to bring in the single commit.
Any suggestion would be highly appreciated. Thanks for your understanding.
Hi @Lawouach,
with a bit of time I managed to get a clean commit tree, but in a different branch. So I created #104 and would close this PR. Hope that's ok with you.
Cheers, Hendrik
Codecov Report
Merging #89 (73e1899) into master (9b56dff) will increase coverage by
0.07%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #89 +/- ##
==========================================
+ Coverage 85.61% 85.69% +0.07%
==========================================
Files 10 10
Lines 598 601 +3
==========================================
+ Hits 512 515 +3
Misses 86 86
Impacted Files | Coverage Δ | |
---|---|---|
chaosk8s/pod/actions.py | 95.91% <100.00%> (+0.12%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 9b56dff...73e1899. Read the comment docs.