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

Strip useless res structs #426

Merged
merged 9 commits into from
May 29, 2023
Merged

Strip useless res structs #426

merged 9 commits into from
May 29, 2023

Conversation

roman-khimov
Copy link
Member

We've got a number of Res* things with a single member only. They were somewhat useful before #413 (to get request status), but now they provide zero value and just complicate the code using these APIs. Notice also that this makes client interface a little close to pool interface which is nice wrt. #380.

Copy link
Contributor

@smallhive smallhive left a comment

Choose a reason for hiding this comment

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

Looks easier, perfect

Copy link
Contributor

@notimetoname notimetoname left a comment

Choose a reason for hiding this comment

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

How will we handle statuses with this scheme? I mean details and non-error responses.

There is nothing more there to return.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
It's the only thing in the ResContainerEACL.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
There is nothing else it can return.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
It's just a list, it doesn't need a wrapper.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
The only thing it can return.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
It returns a network map, that's it.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
Do we have anything else it can return? No.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
Not very likely to change with the current NeoFS scheme.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
Not the most convenient API type-wise, but that's the way it is.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
@roman-khimov
Copy link
Member Author

How will we handle statuses with this scheme? I mean details and non-error responses.

If it's OK, it's just OK. If it's an error, you get some apistatus.ErrXXX that has all of the details and can be checked with apistatus.ErrorIs.

Copy link
Contributor

@notimetoname notimetoname left a comment

Choose a reason for hiding this comment

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

If it's OK, it's just OK.

Agree with you for now but i think it is possible to have some OK results that may have some details (and -- with that PR -- may not be added to any Res* structs anymore) but do not have real examples now.
Just do not see any problems outside of the SDK with getters (they do not add any code lines or make any code much more complex) so i would not spend time on it. But since the work is done, i have no strong cons.

@roman-khimov roman-khimov merged commit e3fe2f3 into master May 29, 2023
@roman-khimov roman-khimov deleted the strip-useless-res-structs branch May 29, 2023 14:13
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