Migrate some logic from generated code into the runtime library #475
Conversation
This approach tends to be easier to support, and safer to fix with library upgrades. Code reuse should be preferred over excessive generation because it produces more readable code which the JVM is able to optimize more efficiently.
Generate changelog in
|
| @@ -52,6 +53,11 @@ public PlainSerDe plainSerDe() { | |||
| return ConjurePlainSerDe.INSTANCE; | |||
| } | |||
|
|
|||
| @Override | |||
| public Clients clients() { | |||
There was a problem hiding this comment.
Thoughts on naming?
| @Override | ||
| public <T> T blocking(Channel channel, Endpoint endpoint, Request request, Deserializer<T> deserializer) { | ||
| ListenableFuture<T> call = call(channel, endpoint, request, deserializer); | ||
| return RemoteExceptions.getUnchecked(call); |
There was a problem hiding this comment.
The RemoteExceptions utility can be moved out of the compilation target (break for older generated code).
There was a problem hiding this comment.
I'll plan to move this after we've cut a release where generated code uses the new methods, that way we have a window for libraries to safely upgrade.
|
I'll follow up and update the codegen accordingly |
|
Great, I'll update this with docs + revapi :-) |
| * limitations under the License. | ||
| */ | ||
|
|
||
| package com.palantir.conjure.java.dialogue.serde; |
There was a problem hiding this comment.
the serde package+module don't make a great deal of sense for the entire runtime object, but I'm not going to couple changing that with this PR.
| import org.mockito.Mock; | ||
| import org.mockito.junit.MockitoJUnitRunner; | ||
|
|
||
| @RunWith(MockitoJUnitRunner.class) |
There was a problem hiding this comment.
does this project not use junit-jupiter?
| * This approach is used instead of using a separate method to block on a single future to simplify | ||
| * otherwise redundant generated code. | ||
| */ | ||
| <T> T blocking(Channel channel, Endpoint endpoint, Request request, Deserializer<T> deserializer); |
There was a problem hiding this comment.
this interface doesn't quite work with the existing codegen. It would force us to duplicate the code for building the request in the blocking/async impls
There was a problem hiding this comment.
Ah doh, our sample services are no longer representative
There was a problem hiding this comment.
I could go either way naming <T> T blocking(ListenableFuture<T>). Preference on return clients().block(future) vs return clients().blocking(future);
There was a problem hiding this comment.
I think i prefer block so that both methods are verbs
|
I need to step away for a half hour, will update this when I get back |
|
Released 0.12.2 |
This approach tends to be easier to support, and safer to fix with
library upgrades.
Code reuse should be preferred over excessive generation because
it produces more readable code which the JVM is able to optimize
more efficiently.
After this PR
==COMMIT_MSG==
Migrate some logic from generated code into the runtime library
==COMMIT_MSG==