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 <sudheervinuko...@yahoo.com> 
> 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 
> <sudheervinuko...@yahoo.com> 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 
> <sudheervinuko...@yahoo.com.invalid> 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.

Reply via email to