object: Limit header length #2749
Conversation
cthulhu-rider
commented
Feb 22, 2024
cthulhu-rider
commented
- based on and blocked by Object header size limitation neofs-api#262
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2749 +/- ##
==========================================
- Coverage 28.73% 28.71% -0.03%
==========================================
Files 427 427
Lines 33184 33188 +4
==========================================
- Hits 9537 9530 -7
- Misses 22795 22804 +9
- Partials 852 854 +2 ☔ View full report in Codecov by Sentry. |
cthulhu-rider
force-pushed
the
object-limit-header
branch
2 times, most recently
from
f7841fd to
a4c5f88
Compare
| @@ -140,7 +140,6 @@ func eaclFiltersToString(fs []eacl.Filter) string { | |||
| _, _ = tw.Write([]byte("\t==\t")) | |||
| case eacl.MatchStringNotEqual: | |||
| _, _ = tw.Write([]byte("\t!=\t")) | |||
| case eacl.MatchUnknown: | |||
There was a problem hiding this comment.
Can be left as is not requiring subsequent linter changes.
There was a problem hiding this comment.
i bet it was previously added to satisfy the linter. Dont see any reason to have empty case
There was a problem hiding this comment.
Well, you either have this line to satisfy the linter, or another line to do the same thing. If there is no real difference, whatever we already have wins to me. We just don't gain anything with the change.
There was a problem hiding this comment.
have this line to satisfy the linter
having it dont fix the linter, otherwise i wouldnt touch this code place at all. It's just an empty case, which is useless
There was a problem hiding this comment.
i strongly disagree with dropping cases and using nolint for them. in general, it shows that you skip smth intentionally and does not require to think about it if you are looking for a bug. not critical though
There was a problem hiding this comment.
nspcc-dev/go-ordered-json@c406b37
The other option of course is to not use exhaustive like in https://github.com/nspcc-dev/neo-go/blob/24041451335d2f6ff64b8e334401d2dff7c3b211/.golangci.yml#L44
There was a problem hiding this comment.
i don't realize the benefit of exhaustive. It helped me 0 times, but swears all the time
There was a problem hiding this comment.
it helped me with updating for sure. if you add more enums, the linter does not allow you to skip places where enums are used, you have to look at them and be sure they are ok or extend switches
There was a problem hiding this comment.
But if you have default or //nolint it's useless. So it's only when you really list every possible case.
cthulhu-rider
force-pushed
the
object-limit-header
branch
from
3e588a9 to
832d719
Compare
|
But conflicts. |
cthulhu-rider
force-pushed
the
object-limit-header
branch
from
832d719 to
e44a1e4
Compare
Previously, NeoFS used 4MB as object header's length limit. The value originally resulted from the default max gRPC message length. Now header length can be up to 16KB only. To ensure the safety of data uploaded before the restriction was introduced, this limit does not apply to intra-container replication. Refs nspcc-dev/neofs-api#262. Signed-off-by: Leonard Lyubich <[email protected]>
Signed-off-by: Leonard Lyubich <[email protected]>
cthulhu-rider
force-pushed
the
object-limit-header
branch
from
e44a1e4 to
a6343cf
Compare
