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.

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

feat(storage): modify storage frontend_nvme_pcie proto file to support http api #333

Merged
merged 3 commits into from
Sep 20, 2023

Conversation

devkevinyy
Copy link
Contributor

Modify the google.api.http definition in the frontend_nvme_pcie proto file in the storage module to better support converting proto to grpc gateway http service

@devkevinyy devkevinyy requested a review from a team as a code owner August 24, 2023 07:14
storage/Makefile Outdated Show resolved Hide resolved
storage/Makefile Show resolved Hide resolved
storage/v1alpha1/frontend_nvme_pcie.proto Show resolved Hide resolved
storage/v1alpha1/frontend_nvme_pcie.proto Outdated Show resolved Hide resolved
storage/v1alpha1/frontend_nvme_pcie.proto Outdated Show resolved Hide resolved
storage/v1alpha1/frontend_nvme_pcie.proto Outdated Show resolved Hide resolved
storage/v1alpha1/frontend_nvme_pcie.proto Outdated Show resolved Hide resolved
storage/v1alpha1/frontend_nvme_pcie.proto Outdated Show resolved Hide resolved
storage/v1alpha1/frontend_nvme_pcie.proto Outdated Show resolved Hide resolved
@artek-koltun artek-koltun requested a review from a team August 24, 2023 12:50
@artek-koltun artek-koltun added the Storage APIs or code related to storage area label Aug 24, 2023
@glimchb
Copy link
Member

glimchb commented Aug 31, 2023

@chujieyang please rebase so we can review it again and run through ci
I like this work and want to merge this

@devkevinyy devkevinyy force-pushed the storage-proto-http branch 2 times, most recently from 5411636 to a82e696 Compare September 1, 2023 01:43
@devkevinyy
Copy link
Contributor Author

@chujieyang please rebase so we can review it again and run through ci I like this work and want to merge this

done

Copy link
Member

@glimchb glimchb left a comment

Choose a reason for hiding this comment

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

not sure why nvme_namespace_id is removed

Copy link
Member

@glimchb glimchb left a comment

Choose a reason for hiding this comment

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

@artek-koltun I'm ok with this change, please review as well before we merging it

@glimchb glimchb self-requested a review September 6, 2023 20:05
@glimchb glimchb added the Merge Candidate in the open merge window, next candidate for merge label Sep 6, 2023
Copy link
Contributor

@artek-koltun artek-koltun left a comment

Choose a reason for hiding this comment

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

Sorry for the delay. LGTM!

@glimchb
Copy link
Member

glimchb commented Sep 19, 2023

@chujieyang please rebase

@devkevinyy
Copy link
Contributor Author

@chujieyang please rebase

Done.

@glimchb glimchb merged commit 916ef93 into opiproject:main Sep 20, 2023
7 checks passed
@glimchb
Copy link
Member

glimchb commented Sep 20, 2023

@chujieyang please check the opiproject/godpu#306 that fixes this change API in our client code

you already have draft opened for opiproject/opi-spdk-bridge#610

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merge Candidate in the open merge window, next candidate for merge Storage APIs or code related to storage area
Projects
None yet
Development

Successfully merging this pull request may close these issues.

proto file define the same http annotation with different rpc method
3 participants