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

api: add HTTP-like object getter #114

Merged
merged 1 commit into from
Dec 14, 2023
Merged

api: add HTTP-like object getter #114

merged 1 commit into from
Dec 14, 2023

Conversation

tatiana-nspcc
Copy link
Contributor

Add GET /get/{cid}/{oid} and HEAD /get/{cid}/{oid} APIs.

Close #109.

Copy link

codecov bot commented Nov 10, 2023

Codecov Report

Attention: 242 lines in your changes are missing coverage. Please review.

Comparison is base (d0c6513) 15.50% compared to head (14b20e6) 13.70%.
Report is 3 commits behind head on master.

Files Patch % Lines
handlers/objects.go 0.00% 231 Missing ⚠️
handlers/api.go 0.00% 11 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #114      +/-   ##
==========================================
- Coverage   15.50%   13.70%   -1.81%     
==========================================
  Files          10       10              
  Lines        1838     2080     +242     
==========================================
  Hits          285      285              
- Misses       1523     1765     +242     
  Partials       30       30              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tatiana-nspcc tatiana-nspcc force-pushed the 109-getter branch 2 times, most recently from 22efb6e to 8e2b1b1 Compare November 21, 2023 09:00
Copy link
Member

@roman-khimov roman-khimov left a comment

Choose a reason for hiding this comment

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

What are the problems we currently have? Bearer token?

Have you tried neofs-testcases with it?

Looks rather legit otherwise.

spec/rest.yaml Outdated Show resolved Hide resolved
handlers/objects.go Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
handlers/objects.go Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
spec/rest.yaml Outdated Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
handlers/objects.go Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
handlers/objects.go Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
handlers/objects.go Outdated Show resolved Hide resolved
@tatiana-nspcc
Copy link
Contributor Author

tatiana-nspcc commented Dec 13, 2023

@cthulhu-rider, I cannot comment directly here: #114 (comment). But contentType is determined for GET and HEAD in http-gw in different ways, so I've kept it.

@cthulhu-rider
Copy link
Contributor

cthulhu-rider commented Dec 13, 2023

@tatiana-nspcc

I cannot comment directly here

but why? try again to follow ur #114 (comment), it must lead to the active thread where u should be able to comment. Afaik GH duplicates review comments left in threads in form of inline uncommentable blocks, im tryin to avoid them but sometimes they intrude ;( think u meant them

But contentType is determined for GET and HEAD in rest-gw in different ways

yeah that's for sure, but regardless of this we need to check if payloadSize > 0 in HEAD handler cuz ObjectRangeInit requires length > 0. For empty payload we can fallback to http.DetectContentType(nil) as in GET to keep them in sync

Add `GET /get/{cid}/{oid}` and `HEAD /get/{cid}/{oid}` APIs.

Signed-off-by: Tatiana Nesterenko <[email protected]>
@roman-khimov roman-khimov merged commit f5cf63e into master Dec 14, 2023
11 of 13 checks passed
@roman-khimov roman-khimov deleted the 109-getter branch December 14, 2023 12:07
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.

Add HTTP-like object getter
4 participants