sdb icon indicating copy to clipboard operation
sdb copied to clipboard

Add input type-checking to Commands

Open shartse opened this issue 5 years ago • 2 comments

Pull request checklist

Please check if your PR fulfills the following requirements:

  • [X] Tests for the changes have been added (for bug fixes / features)
  • [ ] Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • [X] Build was run locally and any changes were pushed
  • [X] Lint has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

  • [ ] Bugfix
  • [x] Feature
  • [ ] Code style update (formatting, renaming)
  • [ ] Refactoring (no functional changes, no api changes)
  • [ ] Build related changes
  • [ ] Documentation content changes
  • [ ] Other (please describe):

What is the current behavior?

Issue Number: https://github.com/delphix/sdb/issues/159

What is the new behavior?

Refactored the input-type checking to be a generator and added it to the generic Command implementation.

Note - because we actually support multiple input and output types for certain commands (spa and vdev), I added extra logic to check for InputHandler functionality. This might not be the cleanest solution - maybe exact command should have a Set for input and output types instead of a single string? Maybe we want to revisit the idea of multiple input/output types at all?

Does this introduce a breaking change?

  • [ ] Yes
  • [ ] No

Other information

shartse avatar Feb 26 '20 23:02 shartse

Codecov Report

Merging #190 into master will increase coverage by 0.26%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #190      +/-   ##
==========================================
+ Coverage   85.36%   85.62%   +0.26%     
==========================================
  Files          56       56              
  Lines        2309     2324      +15     
==========================================
+ Hits         1971     1990      +19     
+ Misses        338      334       -4     
Impacted Files Coverage Δ
sdb/commands/pretty_print.py 65.21% <ø> (ø)
sdb/command.py 76.45% <100.00%> (+3.06%) :arrow_up:
sdb/commands/linux/per_cpu.py 96.96% <0.00%> (-3.04%) :arrow_down:

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 0ea73b6...f304dc8. Read the comment docs.

prachetaa avatar Mar 10 '20 00:03 prachetaa

I think that Walker._call() also needs to be updated to do the input type checking correctly. It's currently using type equality, which doesn't work at all. We'll at least need to use the canonicalized type names.

ahrens avatar Apr 15 '20 22:04 ahrens