project-machine / disko

Disk Operations API in Go
Apache License 2.0
13 stars 9 forks source link

fix CreatePV to use name and path #51

Closed rchamarthy closed 4 years ago

rchamarthy commented 4 years ago

CreatePV accepts a path but thinks it can accept a name and uses the param as a name to later find the PV it just created. The scan fails to match and CreatePV always returns an error saying it did not find the PV it just successfully created.

Fix: accept name or path, if name is given make a path and if path is given make a name and then use both appropriately

Signed-off-by: Ravi Chamarthy ravchama@cisco.com

codecov[bot] commented 4 years ago

Codecov Report

Merging #51 into master will decrease coverage by 0.35%. The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #51      +/-   ##
==========================================
- Coverage   57.37%   57.01%   -0.36%     
==========================================
  Files          15       15              
  Lines        1607     1617      +10     
==========================================
  Hits          922      922              
- Misses        618      628      +10     
  Partials       67       67              
Impacted Files Coverage Δ
linux/lvm.go 9.01% <0.00%> (-0.41%) :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 4124c14...cdbeb48. Read the comment docs.

smoser commented 4 years ago

I do want to get this fix in, because it is clearly broken. so... please respond and lets get that done.

rchamarthy commented 4 years ago

@smoser that diff looks good to me, I have to change my code in storaged and storagectl to make these names. I had to work around and make it a path. I took the route to keep it compatible with my codebase but this is fine too. Its a small change on my side.

smoser commented 4 years ago

https://github.com/anuvu/disko/pull/52

closing this one for that.