Skip to content
This repository has been archived by the owner on Jan 20, 2024. It is now read-only.

implement SelfTargetExecutor #68

Merged
merged 2 commits into from
Oct 4, 2021
Merged

Conversation

AlfinST
Copy link
Contributor

@AlfinST AlfinST commented Sep 16, 2021

Summary

@yahoo/ychaos-dev

Implement SelfTargetExecutor

Fixes: #12

Checklist

Checklist (Developer)

Prerequisites

  • I have read the contribution guidelines

Code Analysis

  • Covered by Unittests
  • Security warnings ignored (Why?!)

Project related

  • Schema changed and is backward compatible.
  • Introduces a new sub-command for ychaos CLI

Autogenerated Files

  • Auto generated schema

Pre-Merge Checklist

  • All the build jobs are passing

Checklist (Reviewer 1)

  • Reviewed Source Code
  • Reviewed Tests (If applicable)
  • Reviewed Documentation (If applicable)

Checklist (Reviewer 2)

  • Reviewed Source Code
  • Reviewed Tests (If applicable)
  • Reviewed Documentation (If applicable)

@shashankrnr32
Copy link
Member

Please Rebase commits and associate the commits with an Email Address.

src/ychaos/core/executor/SelfTargetExecutor.py Outdated Show resolved Hide resolved
src/ychaos/core/executor/SelfTargetExecutor.py Outdated Show resolved Hide resolved
src/ychaos/core/executor/SelfTargetExecutor.py Outdated Show resolved Hide resolved
else:
raise NotImplementedError()

self._register_machine_target_hooks()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be part of the individual branch

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could this be changed to _register_target_hooks() and handle both self and machine target hooks there

src/ychaos/core/executor/SelfTargetExecutor.py Outdated Show resolved Hide resolved
src/ychaos/core/executor/SelfTargetExecutor.py Outdated Show resolved Hide resolved
src/ychaos/core/executor/SelfTargetExecutor.py Outdated Show resolved Hide resolved
@shashankrnr32 shashankrnr32 added enhancement New feature or request ychaos-executor This issue or pull request is regarding the YChaos Executor labels Sep 18, 2021
@shashankrnr32 shashankrnr32 added this to the Version 1.0.0 milestone Sep 18, 2021
@AlfinST AlfinST force-pushed the add_SelfTargetExecutor branch 2 times, most recently from 2527327 to e4152a2 Compare September 22, 2021 18:46
@AlfinST AlfinST marked this pull request as ready for review September 22, 2021 18:46
@AlfinST AlfinST requested a review from a team as a code owner September 22, 2021 18:46
@AlfinST AlfinST force-pushed the add_SelfTargetExecutor branch 2 times, most recently from 058f1de to d4724bd Compare September 23, 2021 05:39
@AlfinST AlfinST changed the title implement SelfTargetExecutor[WIP] implement SelfTargetExecutor Sep 23, 2021
src/ychaos/core/executor/MachineTargetExecutor.py Outdated Show resolved Hide resolved
src/ychaos/core/executor/SelfTargetExecutor.py Outdated Show resolved Hide resolved

if CallbackBase: # pragma: no cover

class YChaosAnsibleResultCallback(CallbackBase, EventHook):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add docstrings with some documentation as to what this class does

src/ychaos/core/executor/SelfTargetExecutor.py Outdated Show resolved Hide resolved
src/ychaos/core/executor/SelfTargetExecutor.py Outdated Show resolved Hide resolved
tests/core/executor/test_SelfTargetExecutor.py Outdated Show resolved Hide resolved
tests/core/executor/test_SelfTargetExecutor.py Outdated Show resolved Hide resolved
tests/cli/test_execute.py Outdated Show resolved Hide resolved
@AlfinST AlfinST force-pushed the add_SelfTargetExecutor branch 3 times, most recently from a716e85 to 77cc96c Compare September 27, 2021 05:46
Copy link
Member

@shashankrnr32 shashankrnr32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor comments. Looks good overall

tests/core/executor/test_SelfTargetExecutor.py Outdated Show resolved Hide resolved
tests/core/executor/test_SelfTargetExecutor.py Outdated Show resolved Hide resolved
tests/core/executor/test_SelfTargetExecutor.py Outdated Show resolved Hide resolved
tests/cli/test_execute.py Show resolved Hide resolved
tests/cli/test_execute.py Show resolved Hide resolved
@yahoo yahoo deleted a comment from codecov bot Sep 27, 2021
@codecov
Copy link

codecov bot commented Sep 27, 2021

Codecov Report

Merging #68 (e609040) into main (9d8bbf5) will increase coverage by 0.15%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #68      +/-   ##
==========================================
+ Coverage   98.52%   98.67%   +0.15%     
==========================================
  Files          55       56       +1     
  Lines        2367     2410      +43     
  Branches      238      240       +2     
==========================================
+ Hits         2332     2378      +46     
+ Misses         22       20       -2     
+ Partials       13       12       -1     
Impacted Files Coverage Δ
src/ychaos/cli/execute.py 91.80% <100.00%> (+5.83%) ⬆️
src/ychaos/core/executor/MachineTargetExecutor.py 98.03% <100.00%> (-0.11%) ⬇️
src/ychaos/core/executor/SelfTargetExecutor.py 100.00% <100.00%> (ø)
src/ychaos/core/executor/common.py 100.00% <100.00%> (ø)
src/ychaos/testplan/attack.py 100.00% <100.00%> (ø)

shashankrnr32
shashankrnr32 previously approved these changes Sep 27, 2021
nafri-irfan96
nafri-irfan96 previously approved these changes Sep 27, 2021
@shashankrnr32
Copy link
Member

Please hold on to merging this.

@shashankrnr32
Copy link
Member

Please rebase. Lets merge this on Monday. Thanks for the PR

@AlfinST AlfinST merged commit 41abce4 into yahoo:main Oct 4, 2021
@AlfinST AlfinST deleted the add_SelfTargetExecutor branch October 4, 2021 11:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request ychaos-executor This issue or pull request is regarding the YChaos Executor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants