feat: Implement frontend and backend for MangoBoost NTT support#6
feat: Implement frontend and backend for MangoBoost NTT support#6han-minhee wants to merge 1 commit intoopiproject:mainfrom
Conversation
han-minhee
force-pushed
the
feat--implement-NTT
branch
2 times, most recently
from
d598022 to
669ece6
Compare
|
I'm looking into the lint error: |
This patch implements MangoBoost NTT support for OPI. Signed-off-by: Minhee Han <minhee.han@mangoboost.io>
han-minhee
force-pushed
the
feat--implement-NTT
branch
from
669ece6 to
46c512c
Compare
artek-koltun
left a comment
artek-koltun
left a comment
There was a problem hiding this comment.
can you pls split for multiple commits? Would it make sense to have a separate commit for backend and for frontend? thanks
| pb "github.com/opiproject/opi-api/storage/v1alpha1/gen/go" | ||
| ) | ||
|
|
||
| func (s *Server) validateCreateNvmeSubsystemRequest(in *pb.CreateNvmeSubsystemRequest) error { |
There was a problem hiding this comment.
this one looks very similar to the one from opi-spdk-bridge. It is embedded into Server, right? Can we try try to re-use it instead of creating the copy?
Pls also check other places where we can re-use the existing methods
There was a problem hiding this comment.
I copied them into separate files for possible future implementations of validation for the custom parameters.
I guess it’s better to remove them as you suggested and maybe add separate validation functions later when needed.
I’ll be making (split) commits again soon.
There was a problem hiding this comment.
BTW, why would Linters give an error?
I can try to modify the .golangci.yml according to the error messages,
but shouldn’t it work right away since the .golangci.yml has almost the same structure as the one in the opi-spdk-bridge repository?
Thanks a lot!
Error: Failed to run: Error: Command failed: /home/runner/golangci-lint-1.64.8-linux-amd64/golangci-lint config verify
jsonschema: "run" does not validate with "/properties/run/type": got null, want object
jsonschema: "output.formats" does not validate with "/properties/output/properties/formats/type": got string, want array
Failed executing command with error: the configuration contains invalid elements
, Error: Command failed: /home/runner/golangci-lint-1.64.8-linux-amd64/golangci-lint config verify
jsonschema: "run" does not validate with "/properties/run/type": got null, want object
jsonschema: "output.formats" does not validate with "/properties/output/properties/formats/type": got string, want array
Failed executing command with error: the configuration contains invalid elements
at genericNodeError (node:internal/errors:984:15)
at wrappedFn (node:internal/errors:538:14)
at ChildProcess.exithandler (node:child_process:422:12)
at ChildProcess.emit (node:events:524:28)
at maybeClose (node:internal/child_process:1104:16)
at ChildProcess._handle.onexit (node:internal/child_process:304:5)
Error: Command failed: /home/runner/golangci-lint-1.64.8-linux-amd64/golangci-lint config verify
jsonschema: "run" does not validate with "/properties/run/type": got null, want object
jsonschema: "output.formats" does not validate with "/properties/output/properties/formats/type": got string, want array
Failed executing command with error: the configuration contains invalid elements
There was a problem hiding this comment.
opi-spdk-bridge pinned the used version of the linter here to v1.55.2
But in the mangoboost bridge, it is not, and v1.64.8 was used.
Thus, pls pin the version in go.mod
There was a problem hiding this comment.
Oh that was the reason. Thanks!
There was a problem hiding this comment.
@artek-koltun I realized that validation methods like validateCreateNvmeSubsystemRequest in the opi-spdk-bridge are not accessible from opi-mangoboost-bridge. Maybe that was the reason they were also included in another vendor's bridge?
Would there be any other way than just copying those methods?
2 participants
This patch implements MangoBoost NTT support for OPI.