Giter VIP home page Giter VIP logo

Comments (9)

marcellanz avatar marcellanz commented on July 26, 2024 2

We can change that.

from massa.

sleipnir avatar sleipnir commented on July 26, 2024 1

My question is that first if the user sends ten messages the user function should probably respond with ten messages, this is a point where the Cloudstate description is confusing by not making it clear that this is its intention, that is, the protocol should say that an empty message body containing only the metadata should be added to the beginning of the stream, being the first message that the user function would have to deal with. Second information like service_name and command_name for me are crucial in all sends and not just in the first message, making this a must makes the design of messages and sdks considerably simpler (at least in the sdks I had to implement).
How it ever made sense I don't know, I've always gone against the grain but James seems to have convinced everyone at the time that this was good design.

It seems to me that certain things in Cloudstate and Akkaserveless are the way they are because they better match the Akka/Scala way of doing things and not because they make general architectural sense.

from massa.

ralphlaude avatar ralphlaude commented on July 26, 2024 1

I fully agree with the point made by @sleipnir. Cloudstate and Akkaserveless are making thing that fits theirs Akka/Scala and not because they make general architectural sense. We should be able to remove things if they do not make sense. But also we should strive for a general architectural which makes sense

from massa.

marcellanz avatar marcellanz commented on July 26, 2024

@sleipnir this might be the reason for what is observed here in this issue https://github.com/eigr-labs/akkaserverless-protocol/blob/main/cloudstateio-protocol/cloudstate-protocols-0.6.0/protocol/cloudstate/action.proto

// For streamed in and duplex streamed calls, the first command sent will just contain the service
// name and command name, but no payload. This will indicate that the action has been invoked.
// Subsequent commands on the stream will only have the payload set, the service name and command
// name will not be set.

from massa.

marcellanz avatar marcellanz commented on July 26, 2024

@sleipnir I think I misunderstood this issue, while the proxy does not sent the first message, I found that from the user functions perspective, in detail, in perspective of its runtime support, the proxy has to send first a command without any payload at all. I though you describe this issue as an issue of the SDK implementation, which seems not to be.

from massa.

sleipnir avatar sleipnir commented on July 26, 2024

@sleipnir this might be the reason for what is observed here in this issue https://github.com/eigr-labs/akkaserverless-protocol/blob/main/cloudstateio-protocol/cloudstate-protocols-0.6.0/protocol/cloudstate/action.proto

// For streamed in and duplex streamed calls, the first command sent will just contain the service
// name and command name, but no payload. This will indicate that the action has been invoked.
// Subsequent commands on the stream will only have the payload set, the service name and command
// name will not be set.

Hi @marcellanz, I think we have two things here.

Firstly, an implementation misconception that causes a message sent by the user to not have a complete response and second, and what led to the first mistake, a serious protocol design flaw (in my opinion), I don't know why this separation of first and other messages is necessary, especially regarding stateless functions such as Actions.
The fact that you don't send valuable information in every payload generates all sorts of work arounds in SDK implementations just to maintain this kind of state that should be co-located with the payload. I find this literally bizarre and completely unnecessary, I even tested it by making the proxy send the service_name, and name in all requests and the user role implemented with the Go SDK kept returning me without problems, that is, there is no validation on this on the user side, and neither should there be.
I think here we can clearly diverge from the original protocol.

from massa.

marcellanz avatar marcellanz commented on July 26, 2024

there is more

// HandleStreamedIn handles a streamed in command. The first message in will
// contain the request metadata, including the service name and command name.
// It will not have an associated payload set. This will be followed by zero to
// many messages in with a payload, but no service name or command name set.
//
// If the underlying transport supports per stream metadata, rather than per
// message metadata, then that metadata will only be included in the metadata
// of the first message. In contrast, if the underlying transport supports per
// message metadata, there will be no metadata on the first message, the
// metadata will instead be found on each subsequent message.
//
// The semantics of stream closure in this protocol map 1:1 with the semantics
// of gRPC stream closure, that is, when the client closes the stream, the
// stream is considered half closed, and the server should eventually, but not
// necessarily immediately, send a response message with a status code and
// trailers.
// If however the server sends a response message before the client closes the
// stream, the stream is completely closed, and the client should handle this
// and stop sending more messages.
//
// Either the client or the server may cancel the stream at any time,
// cancellation is indicated through an HTTP2 stream RST message.

https://github.com/eigr/functions-go-sdk/blob/main/cloudstate/action/server.go#L110-L131

from massa.

sleipnir avatar sleipnir commented on July 26, 2024

Yes, I had read that. Overall, it is unnecessary complexity and so far only justified by sending Metadata in the first message.

from massa.

marcellanz avatar marcellanz commented on July 26, 2024

@sleipnir why does the current protocol and its description not make sense? Or why did it even made sense in the beginning?

Current AS still follows this protocol:
https://github.com/eigr-labs/akkaserverless-protocol/blob/main/akkaserverless-proxy-protocol/akkaserverless-proxy-protocol-0.8.6/akkaserverless/component/action/action.proto#L47

from massa.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.