From c4a1a6c0b4a53711c825257f0aa2c645c581447d Mon Sep 17 00:00:00 2001
From: Teon Banek <teon.banek@memgraph.io>
Date: Mon, 9 Mar 2020 10:42:07 +0100
Subject: [PATCH] Extract py::AppendToSysPath function
Reviewers: mferencevic, ipaljak, llugovic
Reviewed By: mferencevic
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D2714
---
src/py/py.hpp | 22 ++++++++++++++++++++--
src/query/procedure/module.cpp | 16 +++-------------
2 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/src/py/py.hpp b/src/py/py.hpp
index 642b4d5f6..da8e91ad0 100644
--- a/src/py/py.hpp
+++ b/src/py/py.hpp
@@ -190,7 +190,7 @@ inline std::ostream &operator<<(std::ostream &os, const Object &py_object) {
/// Stores information on a raised Python exception.
/// @sa FetchError
-struct ExceptionInfo final {
+struct [[nodiscard]] ExceptionInfo final {
/// Type of the exception, if nullptr there is no exception.
Object type;
/// Optional value of the exception.
@@ -222,7 +222,7 @@ inline std::ostream &operator<<(std::ostream &os,
/// Get the current exception info and clear the current exception indicator.
///
/// This is normally used to catch and handle exceptions via C API.
-inline std::optional<ExceptionInfo> FetchError() {
+[[nodiscard]] inline std::optional<ExceptionInfo> FetchError() {
PyObject *exc_type, *exc_value, *traceback;
PyErr_Fetch(&exc_type, &exc_value, &traceback);
if (!exc_type) return std::nullopt;
@@ -230,4 +230,22 @@ inline std::optional<ExceptionInfo> FetchError() {
return ExceptionInfo{Object(exc_type), Object(exc_value), Object(traceback)};
}
+/// Append `dir` to Python's `sys.path`.
+///
+/// The function does not check whether the directory exists, or is readable.
+/// ExceptionInfo is returned if an error occurred.
+[[nodiscard]] inline std::optional<ExceptionInfo> AppendToSysPath(
+ const char *dir) {
+ CHECK(dir);
+ auto *py_path = PySys_GetObject("path");
+ CHECK(py_path);
+ py::Object import_dir(PyUnicode_FromString(dir));
+ if (!import_dir) return py::FetchError();
+ int import_dir_in_path = PySequence_Contains(py_path, import_dir);
+ if (import_dir_in_path == -1) return py::FetchError();
+ if (import_dir_in_path == 1) return std::nullopt;
+ if (PyList_Append(py_path, import_dir) == -1) return py::FetchError();
+ return std::nullopt;
+}
+
} // namespace py
diff --git a/src/query/procedure/module.cpp b/src/query/procedure/module.cpp
index 1b349e832..4d0ed6772 100644
--- a/src/query/procedure/module.cpp
+++ b/src/query/procedure/module.cpp
@@ -345,21 +345,11 @@ bool PythonModule::Load(std::filesystem::path file_path) {
CHECK(!py_module_) << "Attempting to load an already loaded module...";
LOG(INFO) << "Loading module " << file_path << " ...";
auto gil = py::EnsureGIL();
- auto *py_path = PySys_GetObject("path");
- CHECK(py_path);
- py::Object import_dir(PyUnicode_FromString(file_path.parent_path().c_str()));
- int import_dir_in_path = PySequence_Contains(py_path, import_dir);
- if (import_dir_in_path == -1) {
- LOG(ERROR) << "Unexpected error when loading module " << file_path;
+ auto maybe_exc = py::AppendToSysPath(file_path.parent_path().c_str());
+ if (maybe_exc) {
+ LOG(ERROR) << "Unable to load module " << file_path << "; " << *maybe_exc;
return false;
}
- if (import_dir_in_path == 0) {
- if (PyList_Append(py_path, import_dir) != 0) {
- auto exc_info = py::FetchError().value();
- LOG(ERROR) << "Unable to load module " << file_path << "; " << exc_info;
- return false;
- }
- }
py_module_ =
WithModuleRegistration(&procedures_, [&](auto *module_def, auto *memory) {
return ImportPyModule(file_path.stem().c_str(), module_def);