Added KVIKIO_REMOTE_VERBOSE option

[TomAugspurger]

Our HTTP library, libcurl, includes a CURLOPT_VERBOSE setting that can be useful for debugging. To help our users debug things, I've added a new KVIKIO_REMOTE_VERBOSE option that configures this. By default, it's off (no change). If the user sets KVIKIO_REMOTE_VERBOSE=1 then information from the HTTP requests and responses will be printed to stderr.

[copy-pr-bot]

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[TomAugspurger]

/ok to test 15d8939

[TomAugspurger]

/ok to test 6c1d338

[TomAugspurger]

Ok, this should be ready for review.

Maybe the biggest thing to determine: what name should we use? I've gone with KVIKIO_HTTP_VERBOSE, to align with our other curl-related options (KVIKIO_HTTP_MAX_ATTEMPTS, KVIKIO_HTTP_STATUS_CODES, and KVIKIO_HTTP_TIMEOUT).

Alternatively, we could call this something like KVIKIO_REMOTE_VERBOSE, since libcurl does support non-HTTP-based protocols (even if we don't use them yet). I don't have a preference.

[kingcrimsontianyu]

So the purpose of having the http_verbose parameter in the defaults class would be to allow runtime parameter change. Because of how CURL handles are managed in KvikIO, the change will only be applied to newly generated handles, unless we also explicitly reconfigure existing handles. I'm wondering if we really need that. Perhaps we could simply use:

CurlHandle::CurlHandle(...) {
...
    auto const verbose = getenv_or("KVIKIO_HTTP_VERBOSE", false);
    if(verbose) {
        setopt(CURLOPT_VERBOSE, 1L);
    }
...
}

PS: The CURL handles in KvikIO are reused to minimize handle creation overhead. When an object of CurlHandle goes out of scope, it is not cleaned up, but instead handed back to a global singleton LibCurl which tracks a vector of free CURL handles eligible for reuse. So if we want to apply any runtime parameter change to CURL handles, we need to iterate all the free handles in LibCurl.

[kingcrimsontianyu]

Suggested change

	**Warning** this may show sensitive contents from headers and data.
	.. warning::
	This may show sensitive contents from headers and data.

[kingcrimsontianyu]

Sample output:

[TomAugspurger]

Good to know that the handles are reused. I've updated it with the simpler implementation.

You can observe the output with

KVIKIO_REMOTE_VERBOSE=1 pytest -vs python/kvikio/tests/test_http_io.py::test_file_size

[kingcrimsontianyu]

Thanks for the PR!
Only the doc needs a minor update.

[kingcrimsontianyu]

Update suggested: KVIKIO_REMOTE_VERBOSE

[TomAugspurger]

/merge

[bdice]

How about 1? That is my favorite way to enable things.

[TomAugspurger]

Yep, based on

kvikio/cpp/src/defaults.cpp

Lines 41 to 42 in f4e022e

	// Try parsing `env_var_name` as a integer
	return static_cast<bool>(std::stoi(env_val));

.

Updated the docs to note that.