Skip to content

Commit

Permalink
tensorstore/python: Improve initialization of flags and logging
Browse files Browse the repository at this point in the history
Upon module loading the stderr logging threshold is set to INFO.

Also exposes ts.parse_tensorstore_flags which allows callers to
invoke ts.parse_tensorstore_flags(sys.argv) to correctly propagate
---tensorstore... command line flags to tensorstore.

When running as python module these settings are otherwise unavailable.
NOTE: C++ users will want to call the following in their code:
  absl::ParseCommandLine(argc, argv);
  absl::InitializeLog();
  absl::SetStderrThreshold(absl::LogSeverityAtLeast::kInfo);
PiperOrigin-RevId: 675771349
Change-Id: Iacacc7e1a920b1e5b319b6a24a13302a2094bf50
  • Loading branch information
laramiel authored and copybara-github committed Sep 18, 2024
1 parent 99573a2 commit 041a999
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 9 deletions.
6 changes: 5 additions & 1 deletion python/tensorstore/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ pybind11_py_extension(
"//tensorstore:all_drivers",
"//tensorstore/kvstore/ocdbt/python",
"@com_github_pybind_pybind11//:pybind11",
"@com_google_absl//absl/base:log_severity",
"@com_google_absl//absl/log:globals",
"@com_google_absl//absl/log:initialize",
],
)
Expand Down Expand Up @@ -266,8 +268,9 @@ pybind11_cc_library(
":sequence_parameter",
"//tensorstore:index",
"//tensorstore/index_space:dim_expression",
"//tensorstore/index_space:index_vector_or_scalar",
"//tensorstore/index_space:numpy_indexing_spec",
"//tensorstore/util:str_cat",
"//tensorstore/util:span",
"@com_github_pybind_pybind11//:pybind11",
],
)
Expand Down Expand Up @@ -1167,6 +1170,7 @@ pybind11_cc_library(
"//tensorstore/util:status",
"@com_github_nlohmann_json//:json",
"@com_github_pybind_pybind11//:pybind11",
"@com_google_absl//absl/flags:parse",
"@com_google_absl//absl/strings:cord",
],
alwayslink = True,
Expand Down
27 changes: 27 additions & 0 deletions python/tensorstore/experimental.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <utility>
#include <vector>

#include "absl/flags/parse.h"
#include "absl/strings/cord.h"
#include <nlohmann/json.hpp>
#include "python/tensorstore/future.h"
Expand Down Expand Up @@ -102,6 +103,21 @@ Future<uint32_t> PushMetricsToPrometheus(std::string pushgateway,
request, internal_http::IssueRequestOptions(std::move(payload))));
}

void ParseTensorstoreAbslFlags(std::vector<std::string> python_argv) {
// Convert vector of strings to c-style command line arguments.
std::vector<char*> char_args;
char_args.reserve(python_argv.size());
for (std::string& s : python_argv) {
char_args.push_back(s.data());
}
char** argv = char_args.data();
int argc = static_cast<int>(char_args.size());

std::vector<char*> positional_args;
std::vector<absl::UnrecognizedFlag> unrecognized_flags;
absl::ParseAbseilFlagsOnly(argc, argv, positional_args, unrecognized_flags);
}

} // namespace

void RegisterMetricBindings(pybind11::module_ m, Executor defer) {
Expand Down Expand Up @@ -170,6 +186,17 @@ TENSORSTORE_VERBOSE_LOGGING flags.
flags: :py:obj:`str` comma separated list of flags with optional values.
overwrite: When true overwrites existing flags, otherwise updates.
Group:
Experimental
)");

m.def("parse_tensorstore_flags", &ParseTensorstoreAbslFlags,
pybind11::arg("argv"), R"(
Parses and initializes internal tensorstore flags from argv.
Args:
argv: list of command line argument strings, such as sys.argv.
Group:
Experimental
)");
Expand Down
5 changes: 4 additions & 1 deletion python/tensorstore/index.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,16 @@
#include "python/tensorstore/index.h"

// Other headers
#include <cstddef>
#include <string>
#include <variant>
#include <vector>

#include "python/tensorstore/sequence_parameter.h"
#include "tensorstore/index.h"
#include "tensorstore/util/str_cat.h"
#include "tensorstore/index_space/index_vector_or_scalar.h"
#include "tensorstore/index_space/internal/numpy_indexing_spec.h"
#include "tensorstore/util/span.h"

namespace tensorstore {
namespace internal_python {
Expand Down
4 changes: 2 additions & 2 deletions python/tensorstore/tensorstore_class.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ struct SetCanReferenceSourceDataUntilCommit {
template <typename Self>
static absl::Status Apply(Self& self, type value) {
if (value) {
self.Set(can_reference_source_data_until_commit);
return self.Set(can_reference_source_data_until_commit);
}
return absl::OkStatus();
}
Expand All @@ -166,7 +166,7 @@ struct SetCanReferenceSourceDataIndefinitely {
template <typename Self>
static absl::Status Apply(Self& self, type value) {
if (value) {
self.Set(can_reference_source_data_indefinitely);
return self.Set(can_reference_source_data_indefinitely);
}
return absl::OkStatus();
}
Expand Down
13 changes: 11 additions & 2 deletions python/tensorstore/tensorstore_module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
#include <string>
#include <utility>

#include "absl/log/initialize.h" // IWYU pragma: keep
#include "absl/base/log_severity.h" // IWYU pragma: keep
#include "absl/log/globals.h" // IWYU pragma: keep
#include "absl/log/initialize.h" // IWYU pragma: keep
#include "python/tensorstore/gil_safe.h"
#include "python/tensorstore/python_imports.h"
#include "python/tensorstore/tensorstore_module_components.h"
Expand Down Expand Up @@ -55,9 +57,16 @@ class ScopedModuleNameOverride {
py::object original_name_;
};

PYBIND11_MODULE(_tensorstore, m) {
void InitializeAbslLogging() {
// NOTE: Consider calling absl::ParseAbseilFlagsOnly with sys.argv
// directly here to initialize the internal flags. In some cases there may
// be interactions between absl::Flags and absl::Logging.
absl::InitializeLog();
absl::SetStderrThreshold(absl::LogSeverityAtLeast::kInfo);
}

PYBIND11_MODULE(_tensorstore, m) {
InitializeAbslLogging();
internal_python::InitializeNumpy();

// Ensure that members of this module display as `tensorstore.X` rather than
Expand Down
7 changes: 4 additions & 3 deletions python/tensorstore/tests/experimental_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@
import pytest
import tensorstore as ts

pytestmark = pytest.mark.asyncio


@pytest.mark.asyncio
async def test_collect_matching_metrics():
# Open a tensorstore and read to ensure that some metric is populated.
t = await ts.open({
Expand All @@ -34,6 +33,7 @@ async def test_collect_matching_metrics():
assert m['name'].startswith('/tensorstore/')


@pytest.mark.asyncio
async def test_collect_prometheus_format_metrics():
# Open a tensorstore and read to ensure that some metric is populated.
t = await ts.open({
Expand All @@ -46,7 +46,7 @@ async def test_collect_prometheus_format_metrics():
metric_list = ts.experimental_collect_prometheus_format_metrics(
'/tensorstore/'
)
assert len(metric_list) > 0
assert metric_list

for m in metric_list:
if m.startswith('#'): # Skip comments.
Expand All @@ -58,3 +58,4 @@ def test_experimental_update_verbose_logging():
# There's no way to query whether the verbose logging flags are set, so just
# check that the function exists.
assert hasattr(ts, 'experimental_update_verbose_logging')
ts.experimental_update_verbose_logging('')

0 comments on commit 041a999

Please sign in to comment.