Skip to content
This repository has been archived by the owner on Mar 13, 2021. It is now read-only.

Prototype grpc server #17

Closed
wants to merge 7 commits into from
Closed

Conversation

dsyer
Copy link
Contributor

@dsyer dsyer commented Jan 11, 2018

The app now listens on port 10382 for a stream of riff
Messages, processing them using a Function from
the URI passed in on startup.

@markfisher
Copy link
Contributor

getting build failures with this in the log:

java.lang.ClassNotFoundException: io.netty.buffer.PoolArena$1

these are the errors:

Tests in error:
  FunctionConfigurationTests$ConsumerCompositionTests.testConsumer » IllegalState
  FunctionConfigurationTests$FunctionCompositionTests.testFunction » IllegalState
  FunctionConfigurationTests$FunctionCompositionTests.testThen » IllegalState Fa...
  FunctionConfigurationTests$SingleFunctionTests.testDouble » IllegalState Faile...
  FunctionConfigurationTests$SupplierCompositionTests.testFunction » IllegalState
  FunctionConfigurationTests$SupplierCompositionTests.testSupplier » IllegalState

Tests run: 18, Failures: 0, Errors: 6, Skipped: 0

@dsyer
Copy link
Contributor Author

dsyer commented Jan 11, 2018

Bizarre error (cause still unknown - something to do with an abstract test class). I think it's fixed now. Not sure what the abstract class does.

@markfisher
Copy link
Contributor

yes, the build passes now

thanks!


}

private Message<byte[]> payloadToBytes(Message<?> message) {

Choose a reason for hiding this comment

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

Maybe extract from here down, along with the mapper, to a separate class

HeaderValue header = entry.getValue();
if (header.getValuesCount() > 0) {
Object value;
ProtocolStringList list = header.getValuesList();

Choose a reason for hiding this comment

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

ProtocolStringList can be a List here

private final ManagedChannel channel;
private final MessageFunctionStub asyncStub;

/** Construct client for accessing RouteGuide server at {@code host:port}. */
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment leftover from gRPC getting started guide I believe. will remove

@ericbottard
Copy link
Contributor

Can't make this to work with the Greeter sample from riif, modified to return headers like this (needed to that correlationId is propagated):

public class Greeter implements Function<Message<String>, Message<String>> {

	public Message<String> apply(Message<String> in) {
		return MessageBuilder.withPayload("Hello " + in.getPayload())
				.setHeaders(new MessageHeaderAccessor(in)).build();
	}
}

The Message class is not found.

Tried with --function.runner.isolated=false, or with a boot packaged uberjar. No luck.

@ericbottard
Copy link
Contributor

I have a rebased branch with this, with some additional polish that you may want to grab at https://github.com/ericbottard/java-function-invoker/tree/grpc

@dsyer
Copy link
Contributor Author

dsyer commented Feb 14, 2018

@ericbottard I don't think that message function would work on master either. Did you say you had it working there (confused)?

@ericbottard
Copy link
Contributor

No, didn't say (or looked) that it worked on master either.

This is prompted by the fact that, switching to this gRPC style, we now need to take care of propagating the correlationId at the function level (at least for now), so this PR would be useless if we can't use spring-messaging in the function classloader

@dsyer
Copy link
Contributor Author

dsyer commented Feb 14, 2018

It doesn't work on master then, as I suspected. We can discuss OOB.

@jldec
Copy link
Contributor

jldec commented Feb 15, 2018

from OOB discussion today

i don't have enough context to offer guidance on the <Message> idea
i do think it would be ok to release 0.0.4 with a java invoker that supports req/resp on http and flux streams (without correlation) on grpc if that is easier

@dsyer
Copy link
Contributor Author

dsyer commented Feb 16, 2018

Superseded by #26.

@dsyer dsyer closed this Feb 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants