Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[storage] [lint] Re-enable several rules #218

Merged
merged 6 commits into from
Dec 8, 2022
Merged

[storage] [lint] Re-enable several rules #218

merged 6 commits into from
Dec 8, 2022

Conversation

glimchb
Copy link
Member

@glimchb glimchb commented Dec 8, 2022 •
edited
Loading

  • [storage] [lint] Re-enable rule=core::0135
  • [storage] [lint] Re-enable rule=core::0131
  • [storage] [lint] Re-enable rule=core::0132
  • [storage] [lint] Re-enable rule=core::0134
  • [storage] [lint] Re-enable rule=core::0133

after this PR:

+--------------------------------------+------------------+----------------+
|                 RULE                 | TOTAL VIOLATIONS | VIOLATED FILES |
+--------------------------------------+------------------+----------------+
| core::0156::forbidden-methods        |               11 |              7 |
| core::0127::http-annotation          |               12 |              7 |
| core::0127::resource-name-extraction |               22 |              7 |
| core::0192::has-comments             |              380 |              8 |
+--------------------------------------+------------------+----------------+

@glimchb
[storage] [lint] Re-enable rule=core::0135
ccbc2e6 
Signed-off-by: Boris Glimcher <Boris.Glimcher@emc.com>
@glimchb
[storage] [lint] Re-enable rule=core::0131
43188e3 
Signed-off-by: Boris Glimcher <Boris.Glimcher@emc.com>
@glimchb
[storage] [lint] Re-enable rule=core::0132
77d372e 
Signed-off-by: Boris Glimcher <Boris.Glimcher@emc.com>
@glimchb
[storage] [lint] Re-enable rule=core::0134
b14b925 
Signed-off-by: Boris Glimcher <Boris.Glimcher@emc.com>
@glimchb
[storage] [lint] Re-enable rule=core::0133
8942105 
Signed-off-by: Boris Glimcher <Boris.Glimcher@emc.com>
@glimchb
[storage] auto-generate protos
ba185e7 
Signed-off-by: Boris Glimcher <Boris.Glimcher@emc.com>
@glimchb glimchb marked this pull request as ready for review December 8, 2022 16:20
@glimchb glimchb requested a review from a team as a code owner December 8, 2022 16:20
@glimchb glimchb mentioned this pull request Dec 8, 2022
Closed
@glimchb glimchb added Storage APIs or code related to storage area Merge Candidate in the open merge window, next candidate for merge labels Dec 8, 2022
@glimchb glimchb self-assigned this Dec 8, 2022
glimchb
glimchb commented Dec 8, 2022
View reviewed changes
storage/v1alpha1/backend_aio.proto
import "object_key.proto";
import "uuid.proto";

// Back End (network-facing) APIs. This service is for AIO generic kernel block device.
service AioControllerService {
rpc CreateAioController (CreateAioControllerRequest) returns (AioController) {
option (google.api.http) = {
post: "/v1/subsystems"
body: "subsystem"
post: "/v1/{parent=subsystems}"
Copy link
Member Author

Choose a reason for hiding this comment

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

don't review URLs, they will be fixed here #188

storage/v1alpha1/backend_null.proto
import "object_key.proto";
import "uuid.proto";

// Back End (network-facing) APIs. This is debug interface for null block devices.
service NullDebugService {
rpc CreateNullDebug (CreateNullDebugRequest) returns (NullDebug) {
option (google.api.http) = {
post: "/v1/subsystems"
body: "subsystem"
post: "/v1/{parent=subsystems}"
Copy link
Member Author

Choose a reason for hiding this comment

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

don't review URLs, they will be fixed here #188

storage/v1alpha1/backend_nvme_tcp.proto
import "object_key.proto";

// Back End (network-facing) APIs. NVMe/TCP and NVMe/RoCEv2 protocols are covered by this service.
service NVMfRemoteControllerService {
rpc CreateNVMfRemoteController (CreateNVMfRemoteControllerRequest) returns (NVMfRemoteController) {
option (google.api.http) = {
post: "/v1/subsystems"
Copy link
Member Author

Choose a reason for hiding this comment

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

don't review URLs, they will be fixed here #188

storage/v1alpha1/frontend_virtio_scsi.proto

// Front End (host-facing) APIs. Mostly used for Virtio-scsi emulation and host presentation as alternative to Virtio-blk.
service FrontendVirtioScsiService {
rpc CreateVirtioScsiTarget (CreateVirtioScsiTargetRequest) returns (VirtioScsiTarget) {
option (google.api.http) = {
post: "/v1/virtioscsitargets"
body: "virtioscsitarget"
post: "/v1/{parent=subsystems}"
Copy link
Member Author

Choose a reason for hiding this comment

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

don't review URLs, they will be fixed here #188

storage/v1alpha1/frontend_virtio_blk.proto

// Front End (host-facing) APIs. Mostly used for Virtio-blk emulation emulation and host presentation as alternative to Nvme.
service FrontendVirtioBlkService {
rpc CreateVirtioBlk (CreateVirtioBlkRequest) returns (VirtioBlk) {
option (google.api.http) = {
post: "/v1/virtioblks"
body: "virtioblk"
post: "/v1/{parent=subsystems}"
Copy link
Member Author

Choose a reason for hiding this comment

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

don't review URLs, they will be fixed here #188

storage/v1alpha1/frontend_nvme_pcie.proto

// Front End (host-facing) APIs. Mostly used for NVMe/PCIe emulation and host presentation.
service FrontendNvmeService {
rpc CreateNVMeSubsystem (CreateNVMeSubsystemRequest) returns (NVMeSubsystem) {
option (google.api.http) = {
post: "/v1/subsystems"
body: "subsystem"
post: "/v1/{parent=subsystems}"
Copy link
Member Author

Choose a reason for hiding this comment

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

don't review URLs, they will be fixed here #188

@glimchb glimchb merged commit b089acb into opiproject:main Dec 8, 2022
@glimchb glimchb deleted the crypto branch December 8, 2022 20:57
@llabordehpe
Copy link

What are these new "parent" fields in the CreateXxx messages ? What is the API client supposed to put in this field ?
And what are these new _id fields in the CreateXxx messages ? Is it a new caller's handle ? How does it relates to the key in the Spec message?
And why this "nv_me" ?

@glimchb
Copy link
Member Author

glimchb commented Dec 8, 2022 •
edited
Loading

good questions @llabordehpe
the answers are

I just enabled the linters and fixed all errors reported there
because we have refence code that passes CI you can see how those used here opiproject/opi-spdk-bridge#68

@glimchb glimchb mentioned this pull request Dec 12, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mestery mestery mestery approved these changes

@seroyer seroyer seroyer approved these changes

@benlwalker benlwalker Awaiting requested review from benlwalker

@jainvipin jainvipin Awaiting requested review from jainvipin

@GottliebNoam GottliebNoam Awaiting requested review from GottliebNoam

@llabordehpe llabordehpe Awaiting requested review from llabordehpe

@mkalderon mkalderon Awaiting requested review from mkalderon

@orendu orendu Awaiting requested review from orendu

@sandersms sandersms Awaiting requested review from sandersms

Assignees

@glimchb glimchb

Labels
Merge Candidate in the open merge window, next candidate for merge Storage APIs or code related to storage area
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@glimchb @llabordehpe @mestery @seroyer