Add armeria decorators #25
Conversation
src/main/java/com/fluxninja/aperture/armeria/DecoratingHTTPClient.java
Outdated
Show resolved
Hide resolved
| try { | ||
| res = unwrap().execute(ctx, req); | ||
| flow.end(FlowStatus.OK); | ||
| } catch (InterruptedException | ApertureSDKException e) { |
There was a problem hiding this comment.
What is InterruptedException?
When does ApertureSDKException happen?
There was a problem hiding this comment.
Not sure what was the reasoning behind InterruptedException, removed it for now;
ApertureSDKException in this case can happen when trying to end a flow that was already ended.
src/main/java/com/fluxninja/aperture/armeria/DecoratingHTTPClient.java
Outdated
Show resolved
Hide resolved
| return res; | ||
| } else { | ||
| try { | ||
| flow.end(FlowStatus.Error); |
There was a problem hiding this comment.
The argument to flow.end is supposed to represent errors in feature execution. Can leave it unset in the scenario where flow is not accepted by Aperture I.E. Feature/Server handler did not run
There was a problem hiding this comment.
the FlowStatus accepted as argument is an enum, so one of the possible values has to be passed; Shall we add a third value, other than "OK" and "Error"?
| } catch (ApertureSDKException e) { | ||
| e.printStackTrace(); | ||
| } | ||
| return HttpResponse.of(HttpStatus.BAD_REQUEST); |
There was a problem hiding this comment.
Need to set response code based on CheckResponse.RejectReason
429 Too many requests in case of REJECT_REASON_RATE_LIMITED
503 Service unavailable in case of REJECT_REASON_CONCURRENCY_LIMITED
There was a problem hiding this comment.
Should we also use http codes for RPC responses?
There was a problem hiding this comment.
Return an execptionally completed response? https://javadoc.io/doc/com.linecorp.armeria/armeria-javadoc/latest/com/linecorp/armeria/common/RpcResponse.html
src/main/java/com/fluxninja/aperture/armeria/DecoratingHTTPService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/fluxninja/aperture/armeria/DecoratingRPCClient.java
Outdated
Show resolved
Hide resolved
IridiumOxide
force-pushed
the
armeria-support
branch
2 times, most recently
from
f41b4fd to
36980e6
Compare
IridiumOxide
force-pushed
the
armeria-support
branch
from
890610c to
bbba279
Compare
kwapik
left a comment
kwapik
left a comment
There was a problem hiding this comment.
Reviewed 14 of 14 files at r2, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @IridiumOxide and @tanveergill)
This change is