smithy icon indicating copy to clipboard operation
smithy copied to clipboard

Confusion with the streaming example in section 13.1 of IDL v2

Open david-perez opened this issue 2 years ago • 2 comments

Link. Example as of writing:

operation StreamingOperation {
    input: StreamingOperationInput
    output: StreamingOperationOutput
}

@input
structure StreamingOperationInput {}

@output
structure StreamingOperationOutput {
    @required
    streamId: String
    output: StreamingBlob
}

@streaming
blob StreamingBlob

I'm confused because:

  1. Smithy IDL v2 introduces a change where members targeting streaming blobs now MUST have @default or @required. However, output in the example above does not have any of the two traits attached.
  2. In IDL v1 there is a restriction on the streaming trait that says:

If a service supports a protocol that supports the httpPayload trait, any member that targets a streaming blob must also be marked as @httpPayload.

However, this restriction does not appear in IDL v2. Is that intentional? The Smithy library is still enforcing it for IDL v2 models:

= Member com.amazonaws.simple#AnOperationInput$streamingBlob referencing @streaming shape com.amazonaws.simple#StreamingBlob must have the @httpPayload trait, as service com.amazonaws.simple#SimpleService has a protocol that supports @httpPayload.

  1. The example is confusing because you can't send both a string (streamId) and a streaming blob (output) in the same HTTP message body (?). I acknowledge that in another application-level protocol sending both pieces of data might be possible (which one?), but if you want to keep the example as-is, I think it would be better to call out immediately below it that it's not realizable in HTTP.

david-perez avatar Sep 02 '22 12:09 david-perez

However, output in the example above does not have any of the two traits attached.

Looks like an accidental omission.

[...] However, this restriction does not appear in IDL v2. Is that intentional?

The same limitation applies, and is documented here: https://awslabs.github.io/smithy/2.0/spec/http-bindings.html#event-streams. We should make it more clear this applies to event streams and string/blob streams too.

The example is confusing because you can't send both a string (streamId) and a streaming blob (output) in the same HTTP message body (?). I acknowledge that in another application-level protocol sending both pieces of data might be possible (which one?), but if you want to keep the example as-is, I think it would be better to call out immediately below it that it's not realizable in HTTP.

RPC protocols like AWS JSON can work with this example.

mtdowling avatar Sep 05 '22 00:09 mtdowling

RPC protocols like AWS JSON can work with this example.

How? What does an HTTP body message look like in AWS JSON 1.x for that example?

david-perez avatar Sep 05 '22 11:09 david-perez

I'm updating the docs to address 1 and 2.

How? What does an HTTP body message look like in AWS JSON 1.x for that example?

RPC protocols use an initial event that's sent as an actual event. We don't have docs for this yet. We can track this in https://github.com/awslabs/smithy/issues/1287

mtdowling avatar Oct 21 '22 16:10 mtdowling

But isn't #1287 for event streams? This is for a @streaming blob shape.

david-perez avatar Oct 21 '22 18:10 david-perez

Oh! You're right. I completely glossed over that this targeted a blob. The example is theoretically fine as is, but doesn't work with any existing protocols, so I'll change it.

mtdowling avatar Oct 21 '22 18:10 mtdowling

The example was changed in https://github.com/awslabs/smithy/pull/1458 to:

@http(method: "GET", uri: "/streaming-operation")
operation StreamingOperation {
    input := {}
    output := {
        @required
        streamId: String

        @httpPayload
        output: StreamingBlob = ""
    }
}

@streaming
blob StreamingBlob

This is still invalid because the member targeting the streaming blob is not marked with @required or @default.

Moreover, the example is still confusing because it's irrealizable in any of the supported protocols: we can't send both a streaming blob and a string in the output payload.

david-perez avatar Oct 24 '22 10:10 david-perez

The streaming blob has a default value assigned using = "" which is exactly the same as @default(""). I'll add an HTTP binding for the streamId member. (Edit) Actually, we can just remove the streamId member. I don't think it's needed for the example.

mtdowling avatar Oct 24 '22 16:10 mtdowling