Also, In addition to a new API TSFetchFlagSet to set the fetch flags,
also need to change the return type for TSFetchUrl() to return the FetchSM
context.
TSFetchSM
TSFetchUrl(const char *headers, int request_len, sockaddr const *ip, TSCont
contp, TSFetchWakeUpOptions callback_options,
TSFetchEvent events)
> +void
> +TSFetchFlagSet(TSFetchSM fetch_sm, int flags)
> On Mar 6, 2020, at 8:40 AM, Sudheer Vinukonda <[email protected]>
> wrote:
>
>
> Sorry, I just realized that TSFetchUrl() can be called directly without
> calling TSFetchCreate(). So, to preserve backward compatibility and to be
> able to set the fetch flags, I'm going to add a new `TSFetchFlagSet` API as
> below.
>
> The API accepts a previously created FetchSM context and the flags to be set
> on it.
>
>
> +void
> +TSFetchFlagSet(TSFetchSM fetch_sm, int flags)
> +{
> + sdk_assert(sdk_sanity_check_fetch_sm(fetch_sm) == TS_SUCCESS);
> + (reinterpret_cast<FetchSM *>(fetch_sm))->set_fetch_flags(flags);
> +}
> +
>
>
> Since this is a new API, I'll wait for a few more days for the review.
>
>
>
>
>
> On Friday, March 6, 2020, 08:03:03 AM PST, Sudheer Vinukonda
> <[email protected]> wrote:
>
>
> I'm going to call this in and make the change to the TSFetchUrl() API to
> remove setting `TS_FETCH_FLAGS_DECHUNK` flag by default.
>
> After discussing with Leif over slack, it's worth explicitly clarifying that
> plugins that do want to still have `TS_FETCH_FLAGS_DECHUNK` set can do so by
> simply passing the flag during `TSFetchCreate()` explicitly (that is exactly
> what we are going to do in our plugins).
>
>
> -- Sudheer
>
>
>
>
>
> On Wednesday, March 4, 2020, 10:06:00 AM PST, Sudheer Vinukonda
> <[email protected]> wrote:
>
>
> We use TSFetchUrl() API in a bunch of our use cases to trigger sideways
> calls. The API by default sets `TS_FETCH_FLAGS_DECHUNK`,which implies the
> response returned is dechunked. There's another flag called
> 'TS_FETCH_FLAGS_STREAM' in FechSM, which is NOT set by default.
> This makes TSFetchUrl API a bit conflating and contradictory since dechunking
> is only done when 'TS_FETCH_FLAGS_STREAM' is set ignoring the
> `TS_FETCH_FLAGS_DECHUNK`.
> Our plugins can not handle chunked responses on their own and we don't want
> TS_FETCH_FLAGS_STREAM. We just want the entire response (headers + body) to
> be returned together set by the callback_options AFTER_BODY (similar to how
> esi plugin works).
> The side-effect of all of this is that we can not use HTTP/1.1 to the Origins
> return chunked responses and the contradictory flags above mean that our
> plugins end up getting chunked body (with chunk sizes, trailers etc).
> I'd like to change the default TSFetchUrl() API mode to not set
> `TS_FETCH_FLAGS_DECHUNK` anymore and let callers ask explicitly if they want
> dechunked body similar to how 'TS_FETCH_FLAGS_STREAM' flag works.
> The resultant behavior would be that a plugin would get chunked body only
> when it does not set ` 'TS_FETCH_FLAGS_STREAM' ` and `TS_FETCH_FLAGS_DECHUNK`
> explicitly. If it does either, it'd get dechunked body (streamed in the first
> case and non-streamed in the latter).
> This will make the flags consistent and preserves the backward compatibility
> that esi plugin needs as it does not set `TS_FETCH_FLAGS_STREAM` (and
> `TS_FETCH_FLAGS_DECHUNK`) explicitly. We will update our plugins to
> explicitly ask for `TS_FETCH_FLAGS_DECHUNK`.
> Let me know if there are any concerns/suggestions.
