search

openfaas / connector-sdk

SDK for connecting events to functions
MIT License
54 stars 25 forks source link

[Feature Request] Include contextual information in responses #26

Closed bmcustodio closed 3 years ago

bmcustodio commented 5 years ago

Currently, a ResponseSubscriber gets no contextual information whatsoever about a given request in its Response method. This means that if, for example, I am writing a connector for a given message broker, I cannot mark a given message as having been processed / requiring a retry based on the response(s) because I have no way of correlating responses with whatever message/... originated them.

As a (hopefully) more concrete example, we are building a connector for AWS SQS queue that receives messages from a queue and invokes a function with each message's payload, and we need to...

  1. Delete the message if the function could process it successfully.
  2. Change its visibility timeout if the function could not process it successfully.

I thought about two ways of working around this, both of which require code changes.

  1. Support specifying a string-valued correlationID parameter which would be passed to Invoke and be echoed back in each response.
  2. Support specifying a context which would work the same way, with the added benefit of being able to carry multiple values (and not just a single correlation ID) whenever that's required, as well as to be used as the actual HTTP requests' context.

I chose to go with solution (2) as #25 because it seems to be the most flexible one to me.

alexellis commented 5 years ago

It sounds like you want a way to "ack" that a message has been received or processed?

Could you do that in the code that dequeues a message and invokes the function?

For the Kafka Connector, we invoke, followed by marking the offset:

https://github.com/openfaas-incubator/kafka-connector/blob/master/main.go#L99

https://github.com/openfaas-incubator/kafka-connector/blob/master/main.go#L101

What if you did the same in your SQS connector?

Is the shortfall that you only want to ack messages conditionally depending on whether the function returned a 2xx status code?

alexellis commented 5 years ago

Something which may make this harder to model is that each "Invoke()" call, may lead to an invocation of 0..* functions:

https://github.com/openfaas-incubator/connector-sdk/blob/master/types/invoker.go#L40

What happens if a message is executed by 1 function successfully and a second unsuccessfully? What if no functions exist and there are no status codes?

Alex

bmcustodio commented 5 years ago

What if you did the same in your SQS connector?

I looked into that but that doesn't cut it for us.

Is the shortfall that you only want to ack messages conditionally depending on whether the function returned a 2xx status code?

Kind of, for now. In the future we may need to extend this behaviour, so we need a way of associating responses with the originating entity (message/request/...).

What happens if a message is executed by 1 function successfully and a second unsuccessfully? What if no functions exist and there are no status codes?

That would be up to whoever implements the connector to decide, provided we give them a way to understand if the invocations were successful (which we currently don't, and which my PR tries to introduce).

alexellis commented 5 years ago

@bmcstdio do we have this now? Can I close?