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

Refurbish the grpc and http plugins for eos #4185

Merged
merged 26 commits into from
Oct 12, 2023
Merged

Conversation

ffurano
Copy link
Contributor

@ffurano ffurano commented Sep 12, 2023

Refurbish the grpc and http plugins for eos

@ffurano ffurano requested a review from a team as a code owner September 18, 2023 10:02
cmd/reva/test.go Show resolved Hide resolved
cmd/reva/test.go Show resolved Hide resolved
pkg/eosclient/eosgrpc/eosgrpc.go Show resolved Hide resolved
pkg/eosclient/eosgrpc/eosgrpc.go Outdated Show resolved Hide resolved
@cs3org cs3org deleted a comment from update-docs bot Sep 21, 2023
@labkode
Copy link
Member

labkode commented Oct 6, 2023

What is the issue of re-using existing context?

@glpatcern
Copy link
Member

For the test command, true that we want to expose the CLI at some point but at that point we can comment this test out as it's useful only for some QA tests now. If we really think it can be useful in the future, then we move it somewhere so that it is not accessible to users.

@gmgigi96 can you approve?

@glpatcern
Copy link
Member

What is the issue of re-using existing context?

I'm with @ffurano to understand this - apparently the GRPC client for eos needs an empty context and breaks if we reuse the incoming reva-created ctx. We suspect this is due to the eos protobuf and the reva protobuf being different (in version?), as when we instantiate a GRPC client in reva to talk to another microservice we never have this issue.

glpatcern
glpatcern previously approved these changes Oct 9, 2023
Makefile Outdated Show resolved Hide resolved
@gmgigi96 gmgigi96 merged commit 5065e78 into cs3org:master Oct 12, 2023
gmgigi96 added a commit to cernbox/reva that referenced this pull request Oct 24, 2023
* For GRPC to EOS use the app context, not the passed one

* Refresh compiling the eosgrpc proto

* eos_grpc: small fixes

* Add teststorageperf reva shell command

* Refurbish the grpc and http plugins for eos

* Add README for how to compile the grpc intf to eos

* Add test self-referential release note

* Modify test self-referential release note

* Modify test self-referential release note

* Cosmetics

* NSRequests: use the background context to avoid grpc ugly bug

* Fix lint warning

* Close the body of an http get when not needed anymore

* Fix lint warning

* Set cmd timeout to 15s (allows uploading larger files to loaded servers)

* eosgrpc: use a clean context derived from the original one

* Makefile: use gaia to build revad

* Fix lint warning

* Add new target cernbox-revad, using gaia to build revad

* go lint-fix

* Update Makefile

Co-authored-by: Gianmaria Del Monte <[email protected]>

---------

Co-authored-by: Fabrizio Furano <[email protected]>
Co-authored-by: Gianmaria Del Monte <[email protected]>
gmgigi96 added a commit to cernbox/reva that referenced this pull request Oct 24, 2023
* For GRPC to EOS use the app context, not the passed one

* Refresh compiling the eosgrpc proto

* eos_grpc: small fixes

* Add teststorageperf reva shell command

* Refurbish the grpc and http plugins for eos

* Add README for how to compile the grpc intf to eos

* Add test self-referential release note

* Modify test self-referential release note

* Modify test self-referential release note

* Cosmetics

* NSRequests: use the background context to avoid grpc ugly bug

* Fix lint warning

* Close the body of an http get when not needed anymore

* Fix lint warning

* Set cmd timeout to 15s (allows uploading larger files to loaded servers)

* eosgrpc: use a clean context derived from the original one

* Makefile: use gaia to build revad

* Fix lint warning

* Add new target cernbox-revad, using gaia to build revad

* go lint-fix

* Update Makefile

Co-authored-by: Gianmaria Del Monte <[email protected]>

---------

Co-authored-by: Fabrizio Furano <[email protected]>
Co-authored-by: Gianmaria Del Monte <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants