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(syft_exc): change client and server side response handling #9012

Conversation

tcp
Copy link
Collaborator

@tcp tcp commented Jul 3, 2024

Description

  • Adds unwrap_on_success attribute to the service method decorator
  • Wraps server side service results in SyftSuccess
  • Unwraps service results on the client side based on unwrap_on_success attribute of the service method called
  • Adds a temporary list of services where the unboxing should be evaluated

Part of #8883

tcp added 2 commits July 3, 2024 20:01
This PR introduces a significant change in how service responses are
handled on both the client and server sides.

**Server-side Enhancements (node/node.py)**
- Responses are now wrapped before being sent. If no exceptions are
  raised, responses are always wrapped in `SyftSuccess`.

**Client-side Enhancements (client/api.py)**
- Responses to service methods are unwrapped if the `unwrap_on_success`
  decorator is applied (default is True).
- Unwrapping involves retrieving the object returned by
  `SyftSuccess.unwrap_value()`, which corresponds to the original return
  value from the service method.

**Temporary**
- Introduced `UNWRAPPABLE_SERVICES_LIST`, a temporary list of services
  for which response unwrapping is applied. This list will be removed
  once all services (stashes) are migrated.

By not unwrapping on success, messages are returned to the user as
`SyftSuccess` (see notebooks).
@tcp tcp self-assigned this Jul 3, 2024
@tcp tcp merged commit 073ee56 into refactor/syft-error-handling Jul 4, 2024
4 checks passed
@tcp tcp deleted the refactor/syft-error-handling-integrate-response branch July 4, 2024 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant