From 0480c8391b6eba9efa6edf734642e7c7e39f3edf Mon Sep 17 00:00:00 2001 From: Alberto Miranda Date: Wed, 19 Oct 2022 22:58:06 +0200 Subject: [PATCH] C++ API: amire::error_code is now a class --- src/common/api/CMakeLists.txt | 2 +- src/common/api/admire_types.hpp | 102 +++++++++++++++++++--------- src/{lib => common/api}/errors.c | 2 +- src/common/api/internal_types.hpp | 4 +- src/common/api/types.cpp | 10 +++ src/lib/CMakeLists.txt | 2 +- src/lib/admire.cpp | 13 ++-- src/lib/admire.hpp | 4 +- src/lib/detail/impl.cpp | 59 ++++++++-------- src/scord/adhoc_storage_manager.hpp | 20 +++--- src/scord/job_manager.hpp | 12 ++-- src/scord/rpc_handlers.cpp | 14 ++-- 12 files changed, 145 insertions(+), 99 deletions(-) rename src/{lib => common/api}/errors.c (98%) diff --git a/src/common/api/CMakeLists.txt b/src/common/api/CMakeLists.txt index b4c84785..e2eb4e98 100644 --- a/src/common/api/CMakeLists.txt +++ b/src/common/api/CMakeLists.txt @@ -25,7 +25,7 @@ add_library(_api_types STATIC) target_sources(_api_types PUBLIC admire_types.h admire_types.hpp PRIVATE - types.cpp convert.hpp convert.cpp internal_types.hpp) + types.cpp convert.hpp convert.cpp internal_types.hpp errors.c) target_include_directories(_api_types PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}) diff --git a/src/common/api/admire_types.hpp b/src/common/api/admire_types.hpp index 2f9b3455..8d13791b 100644 --- a/src/common/api/admire_types.hpp +++ b/src/common/api/admire_types.hpp @@ -36,7 +36,74 @@ namespace admire { -using error_code = ADM_return_t; +struct error_code { + + static const error_code success; + static const error_code snafu; + static const error_code bad_args; + static const error_code out_of_memory; + static const error_code entity_exists; + static const error_code no_such_entity; + static const error_code adhoc_in_use; + static const error_code other; + + constexpr error_code() : m_value(ADM_SUCCESS) {} + constexpr explicit error_code(ADM_return_t ec) : m_value(ec) {} + constexpr explicit error_code(int32_t ec) + : m_value(static_cast(ec)) {} + + constexpr operator ADM_return_t() const { // NOLINT + return m_value; + } + + constexpr explicit operator bool() const { + return m_value == ADM_SUCCESS; + } + + ADM_return_t + value() const { + return m_value; + } + + constexpr std::string_view + name() const { + switch(m_value) { + case ADM_SUCCESS: + return "ADM_SUCCESS"; + case ADM_ESNAFU: + return "ADM_ESNAFU"; + case ADM_EBADARGS: + return "ADM_EBADARGS"; + case ADM_ENOMEM: + return "ADM_ENOMEM"; + case ADM_EEXISTS: + return "ADM_EEXISTS"; + case ADM_ENOENT: + return "ADM_ENOENT"; + case ADM_EADHOC_BUSY: + return "ADM_EADHOC_BUSY"; + case ADM_EOTHER: + return "ADM_EOTHER"; + default: + return "INVALID_ERROR_VALUE"; + } + } + + std::string_view + message() const; + +private: + ADM_return_t m_value; +}; + +constexpr error_code error_code::success = error_code{ADM_SUCCESS}; +constexpr error_code error_code::snafu = error_code{ADM_ESNAFU}; +constexpr error_code error_code::bad_args = error_code{ADM_EBADARGS}; +constexpr error_code error_code::out_of_memory = error_code{ADM_ENOMEM}; +constexpr error_code error_code::entity_exists = error_code{ADM_EEXISTS}; +constexpr error_code error_code::no_such_entity = error_code{ADM_ENOENT}; +constexpr error_code error_code::adhoc_in_use = error_code{ADM_EADHOC_BUSY}; +constexpr error_code error_code::other = error_code{ADM_EOTHER}; using job_id = std::uint64_t; using slurm_job_id = std::uint64_t; @@ -439,38 +506,7 @@ struct fmt::formatter : formatter { template auto format(const admire::error_code& ec, FormatContext& ctx) const { - std::string_view name = "unknown"; - - switch(ec) { - case ADM_SUCCESS: - name = "ADM_SUCCESS"; - break; - case ADM_ESNAFU: - name = "ADM_ESNAFU"; - break; - case ADM_EBADARGS: - name = "ADM_EBADARGS"; - break; - case ADM_ENOMEM: - name = "ADM_ENOMEM"; - break; - case ADM_EEXISTS: - name = "ADM_EEXISTS"; - break; - case ADM_ENOENT: - name = "ADM_ENOENT"; - break; - case ADM_EADHOC_BUSY: - name = "ADM_EADHOC_BUSY"; - break; - case ADM_EOTHER: - name = "ADM_EOTHER"; - break; - default: - break; - } - - return formatter::format(name, ctx); + return formatter::format(ec.name(), ctx); } }; diff --git a/src/lib/errors.c b/src/common/api/errors.c similarity index 98% rename from src/lib/errors.c rename to src/common/api/errors.c index 3a57c926..19cc00ea 100644 --- a/src/lib/errors.c +++ b/src/common/api/errors.c @@ -22,7 +22,7 @@ * SPDX-License-Identifier: GPL-3.0-or-later *****************************************************************************/ -#include +#include const char* const adm_errlist[ADM_ERR_MAX + 1] = { [ADM_SUCCESS] = "Success", diff --git a/src/common/api/internal_types.hpp b/src/common/api/internal_types.hpp index 7afede19..5aa9e412 100644 --- a/src/common/api/internal_types.hpp +++ b/src/common/api/internal_types.hpp @@ -86,12 +86,12 @@ struct adhoc_storage_info { if(m_client_info) { LOGGER_ERROR("adhoc storage {} already has a client", m_adhoc_storage.id()); - return ADM_EADHOC_BUSY; + return error_code::adhoc_in_use; } m_client_info = std::move(job_info); - return ADM_SUCCESS; + return error_code::success; } void diff --git a/src/common/api/types.cpp b/src/common/api/types.cpp index 1eee0ec0..0f1a05d7 100644 --- a/src/common/api/types.cpp +++ b/src/common/api/types.cpp @@ -1013,8 +1013,18 @@ ADM_qos_limit_list_destroy(ADM_qos_limit_list_t list) { /* C++ Type definitions and related functions */ /******************************************************************************/ +extern "C" { +const char* +ADM_strerror(ADM_return_t errnum); +}; + namespace admire { +std::string_view +error_code::message() const { + return ::ADM_strerror(m_value); +} + class server::impl { public: diff --git a/src/lib/CMakeLists.txt b/src/lib/CMakeLists.txt index 68351cbf..69263c2a 100644 --- a/src/lib/CMakeLists.txt +++ b/src/lib/CMakeLists.txt @@ -26,7 +26,7 @@ add_library(adm_iosched SHARED) target_sources(adm_iosched PUBLIC admire.h admire.hpp - PRIVATE admire.cpp c_wrapper.cpp detail/impl.hpp detail/impl.cpp errors.c env.hpp) + PRIVATE admire.cpp c_wrapper.cpp detail/impl.hpp detail/impl.cpp env.hpp) set_target_properties(adm_iosched PROPERTIES PUBLIC_HEADER "admire.h;admire.hpp") diff --git a/src/lib/admire.cpp b/src/lib/admire.cpp index 482421f5..54a51124 100644 --- a/src/lib/admire.cpp +++ b/src/lib/admire.cpp @@ -200,10 +200,9 @@ namespace admire { void ping(const server& srv) { - - if(const auto ec = detail::ping(srv)) { + if(const auto rv = detail::ping(srv); !rv) { throw std::runtime_error( - fmt::format("ADM_register_job() error: {}", ADM_strerror(ec))); + fmt::format("ADM_ping() error: {}", rv.message())); } } @@ -216,19 +215,19 @@ register_job(const server& srv, const job::resources& resources, if(!rv) { throw std::runtime_error(fmt::format("ADM_register_job() error: {}", - ADM_strerror(rv.error()))); + rv.error().message())); } return rv.value(); } -ADM_return_t +admire::error_code update_job(const server& srv, const job& job, const job::resources& job_resources) { return detail::update_job(srv, job, job_resources); } -ADM_return_t +admire::error_code remove_job(const server& srv, const job& job) { return detail::remove_job(srv, job); } @@ -243,7 +242,7 @@ register_adhoc_storage(const server& srv, const std::string& name, if(!rv) { throw std::runtime_error( fmt::format("ADM_register_adhoc_storage() error: {}", - ADM_strerror(rv.error()))); + rv.error().message())); } return rv.value(); diff --git a/src/lib/admire.hpp b/src/lib/admire.hpp index 68315fc0..00e44943 100644 --- a/src/lib/admire.hpp +++ b/src/lib/admire.hpp @@ -51,10 +51,10 @@ admire::job register_job(const server& srv, const job::resources& job_resources, const job_requirements& reqs, admire::slurm_job_id slurm_id); -ADM_return_t +admire::error_code update_job(const server& srv, const job&, const job::resources& job_resources); -ADM_return_t +admire::error_code remove_job(const server& srv, const job& job); admire::adhoc_storage diff --git a/src/lib/detail/impl.cpp b/src/lib/detail/impl.cpp index 73556145..91451d9e 100644 --- a/src/lib/detail/impl.cpp +++ b/src/lib/detail/impl.cpp @@ -191,9 +191,9 @@ ping(const server& srv) { LOGGER_INFO("rpc id: {} name: {} from: {} <= " "body: {{retval: {}}} [op_id: {}]", rpc_id, std::quoted("ADM_"s + __FUNCTION__), - std::quoted(rpc.origin()), - static_cast(out.retval), out.op_id); - return ADM_SUCCESS; + std::quoted(rpc.origin()), admire::error_code{out.retval}, + out.op_id); + return admire::error_code::success; } tl::expected @@ -221,12 +221,12 @@ register_job(const server& srv, const job::resources& job_resources, const auto rpc = endp.call("ADM_register_job", &in, &out); - if(out.retval != ADM_SUCCESS) { + if(const auto rv = admire::error_code{out.retval}; !rv) { LOGGER_ERROR("rpc id: {} name: {} from: {} <= " "body: {} [op_id: {}]", rpc_id, std::quoted("ADM_"s + __FUNCTION__), - std::quoted(rpc.origin()), out.retval, out.op_id); - return tl::make_unexpected(static_cast(out.retval)); + std::quoted(rpc.origin()), rv, out.op_id); + return tl::make_unexpected(rv); } const admire::job job = api::convert(out.job); @@ -234,7 +234,8 @@ register_job(const server& srv, const job::resources& job_resources, LOGGER_INFO("rpc id: {} name: {} from: {} <= " "body: {{retval: {}, job: {}}} [op_id: {}]", rpc_id, std::quoted("ADM_"s + __FUNCTION__), - std::quoted(rpc.origin()), ADM_SUCCESS, job, out.op_id); + std::quoted(rpc.origin()), admire::error_code::success, job, + out.op_id); return job; } @@ -261,20 +262,20 @@ update_job(const server& srv, const job& job, const auto rpc = endp.call("ADM_update_job", &in, &out); - if(out.retval != ADM_SUCCESS) { - const auto retval = static_cast(out.retval); + if(const auto rv = admire::error_code{out.retval}; !rv) { LOGGER_ERROR("rpc id: {} name: {} from: {} <= " "body: {{retval: {}}} [op_id: {}]", rpc_id, std::quoted("ADM_"s + __FUNCTION__), - std::quoted(rpc.origin()), retval, out.op_id); - return retval; + std::quoted(rpc.origin()), rv, out.op_id); + return rv; } LOGGER_INFO("rpc id: {} name: {} from: {} <= " "body: {{retval: {}}} [op_id: {}]", rpc_id, std::quoted("ADM_"s + __FUNCTION__), - std::quoted(rpc.origin()), ADM_SUCCESS, out.op_id); - return ADM_SUCCESS; + std::quoted(rpc.origin()), admire::error_code::success, + out.op_id); + return admire::error_code::success; } admire::error_code @@ -297,20 +298,20 @@ remove_job(const server& srv, const job& job) { const auto rpc = endp.call("ADM_remove_job", &in, &out); - if(out.retval != ADM_SUCCESS) { - const auto retval = static_cast(out.retval); + if(const auto rv = admire::error_code{out.retval}; !rv) { LOGGER_ERROR("rpc id: {} name: {} from: {} <= " "body: {{retval: {}}} [op_id: {}]", rpc_id, std::quoted("ADM_"s + __FUNCTION__), - std::quoted(rpc.origin()), retval, out.op_id); - return retval; + std::quoted(rpc.origin()), rv, out.op_id); + return rv; } LOGGER_INFO("rpc id: {} name: {} from: {} <= " "body: {{retval: {}}} [op_id: {}]", rpc_id, std::quoted("ADM_"s + __FUNCTION__), - std::quoted(rpc.origin()), ADM_SUCCESS, out.op_id); - return ADM_SUCCESS; + std::quoted(rpc.origin()), admire::error_code::success, + out.op_id); + return admire::error_code::success; } tl::expected @@ -337,13 +338,12 @@ register_adhoc_storage(const server& srv, const std::string& name, const auto rpc = endp.call("ADM_register_adhoc_storage", &in, &out); - if(out.retval != ADM_SUCCESS) { - const auto retval = static_cast(out.retval); + if(const auto rv = admire::error_code{out.retval}; !rv) { LOGGER_ERROR("rpc id: {} name: {} from: {} <= " "body: {{retval: {}}} [op_id: {}]", rpc_id, std::quoted("ADM_"s + __FUNCTION__), - std::quoted(rpc_client.self_address()), retval, out.op_id); - return tl::make_unexpected(retval); + std::quoted(rpc_client.self_address()), rv, out.op_id); + return tl::make_unexpected(rv); } auto rpc_adhoc_storage = admire::adhoc_storage{type, name, out.id, ctx}; @@ -351,8 +351,8 @@ register_adhoc_storage(const server& srv, const std::string& name, LOGGER_INFO("rpc id: {} name: {} from: {} <= " "body: {{retval: {}, id: {}}} [op_id: {}]", rpc_id, std::quoted("ADM_"s + __FUNCTION__), - std::quoted(rpc_client.self_address()), ADM_SUCCESS, out.id, - out.op_id); + std::quoted(rpc_client.self_address()), + admire::error_code::success, out.id, out.op_id); return rpc_adhoc_storage; } @@ -389,12 +389,12 @@ transfer_datasets(const server& srv, const job& job, [[maybe_unused]] const auto rpc = endp.call("ADM_transfer_datasets", &in, &out); - if(out.retval < 0) { + if(const auto rv = admire::error_code{out.retval}; !rv) { LOGGER_ERROR("rpc id: {} name: {} from: {} <= " "body: {{retval: {}}} [op_id: {}]", rpc_id, std::quoted("ADM_"s + __FUNCTION__), - std::quoted(rpc.origin()), out.retval, out.op_id); - return tl::make_unexpected(static_cast(out.retval)); + std::quoted(rpc.origin()), rv, out.op_id); + return tl::make_unexpected(rv); } const admire::transfer tx = api::convert(out.tx); @@ -402,7 +402,8 @@ transfer_datasets(const server& srv, const job& job, LOGGER_INFO("rpc id: {} name: {} from: {} <= " "body: {{retval: {}, transfer: {}}} [op_id: {}]", rpc_id, std::quoted("ADM_"s + __FUNCTION__), - std::quoted(rpc.origin()), ADM_SUCCESS, tx, out.op_id); + std::quoted(rpc.origin()), admire::error_code::success, tx, + out.op_id); return tx; } diff --git a/src/scord/adhoc_storage_manager.hpp b/src/scord/adhoc_storage_manager.hpp index 160692ff..65d0a1f7 100644 --- a/src/scord/adhoc_storage_manager.hpp +++ b/src/scord/adhoc_storage_manager.hpp @@ -58,14 +58,14 @@ struct adhoc_storage_manager : scord::utils::singleton { if(!inserted) { LOGGER_ERROR("{}: Emplace failed", __FUNCTION__); - return tl::make_unexpected(ADM_ESNAFU); + return tl::make_unexpected(admire::error_code::snafu); } return it_adhoc->second; } LOGGER_ERROR("{}: Adhoc storage '{}' already exists", __FUNCTION__, id); - return tl::make_unexpected(ADM_EEXISTS); + return tl::make_unexpected(admire::error_code::entity_exists); } admire::error_code @@ -77,11 +77,11 @@ struct adhoc_storage_manager : scord::utils::singleton { it != m_adhoc_storages.end()) { const auto current_adhoc_info = it->second; current_adhoc_info->update(std::move(new_ctx)); - return ADM_SUCCESS; + return admire::error_code::success; } LOGGER_ERROR("{}: Adhoc storage '{}' does not exist", __FUNCTION__, id); - return ADM_ENOENT; + return admire::error_code::no_such_entity; } tl::expected, @@ -97,7 +97,7 @@ struct adhoc_storage_manager : scord::utils::singleton { LOGGER_ERROR("Adhoc storage '{}' was not registered or was already " "deleted", id); - return tl::make_unexpected(ADM_ENOENT); + return tl::make_unexpected(admire::error_code::no_such_entity); } admire::error_code @@ -107,14 +107,14 @@ struct adhoc_storage_manager : scord::utils::singleton { if(m_adhoc_storages.count(id) != 0) { m_adhoc_storages.erase(id); - return ADM_SUCCESS; + return admire::error_code::success; } LOGGER_ERROR("Adhoc storage '{}' was not registered or was already " "deleted", id); - return ADM_ENOENT; + return admire::error_code::no_such_entity; } admire::error_code @@ -126,7 +126,7 @@ struct adhoc_storage_manager : scord::utils::singleton { return adhoc_storage_info->add_client_info(std::move(job_info)); } - return ADM_ENOENT; + return admire::error_code::no_such_entity; } admire::error_code @@ -134,10 +134,10 @@ struct adhoc_storage_manager : scord::utils::singleton { if(auto am_result = find(adhoc_id); am_result.has_value()) { const auto adhoc_storage_info = *am_result; adhoc_storage_info->remove_client_info(); - return ADM_SUCCESS; + return admire::error_code::success; } - return ADM_ENOENT; + return admire::error_code::no_such_entity; } diff --git a/src/scord/job_manager.hpp b/src/scord/job_manager.hpp index 97fa25a5..2c8ead05 100644 --- a/src/scord/job_manager.hpp +++ b/src/scord/job_manager.hpp @@ -59,14 +59,14 @@ struct job_manager : scord::utils::singleton { if(!inserted) { LOGGER_ERROR("{}: Emplace failed", __FUNCTION__); - return tl::make_unexpected(ADM_ESNAFU); + return tl::make_unexpected(admire::error_code::snafu); } return it_job->second; } LOGGER_ERROR("{}: Job '{}' already exists", __FUNCTION__, id); - return tl::make_unexpected(ADM_EEXISTS); + return tl::make_unexpected(admire::error_code::entity_exists); } admire::error_code @@ -77,11 +77,11 @@ struct job_manager : scord::utils::singleton { if(const auto it = m_jobs.find(id); it != m_jobs.end()) { const auto& current_job_info = it->second; current_job_info->update(std::move(job_resources)); - return ADM_SUCCESS; + return admire::error_code::success; } LOGGER_ERROR("{}: Job '{}' does not exist", __FUNCTION__, id); - return ADM_ENOENT; + return admire::error_code::no_such_entity; } tl::expected, @@ -95,7 +95,7 @@ struct job_manager : scord::utils::singleton { } LOGGER_ERROR("Job '{}' was not registered or was already deleted", id); - return tl::make_unexpected(ADM_ENOENT); + return tl::make_unexpected(admire::error_code::no_such_entity); } tl::expected, @@ -111,7 +111,7 @@ struct job_manager : scord::utils::singleton { LOGGER_ERROR("Job '{}' was not registered or was already deleted", id); - return tl::make_unexpected(ADM_ENOENT); + return tl::make_unexpected(admire::error_code::no_such_entity); } private: diff --git a/src/scord/rpc_handlers.cpp b/src/scord/rpc_handlers.cpp index 5f1d713c..b172e67e 100644 --- a/src/scord/rpc_handlers.cpp +++ b/src/scord/rpc_handlers.cpp @@ -61,7 +61,7 @@ ADM_ping(hg_handle_t h) { LOGGER_INFO("rpc id: {} name: {} to: {} <= " "body: {{retval: {}}}", id, std::quoted(__FUNCTION__), std::quoted(get_address(h)), - ADM_SUCCESS); + admire::error_code::success); ret = margo_respond(h, &out); assert(ret == HG_SUCCESS); @@ -98,7 +98,7 @@ ADM_register_job(hg_handle_t h) { rpc_id, std::quoted(__FUNCTION__), std::quoted(get_address(h)), job_resources, reqs, slurm_id); - admire::error_code ec = ADM_SUCCESS; + admire::error_code ec = admire::error_code::success; std::optional out_job; auto& jm = scord::job_manager::instance(); @@ -220,7 +220,7 @@ ADM_remove_job(hg_handle_t h) { rpc_id, std::quoted(__FUNCTION__), std::quoted(get_address(h)), job); - admire::error_code ec = ADM_SUCCESS; + admire::error_code ec; auto& jm = scord::job_manager::instance(); const auto jm_result = jm.remove(job.id()); @@ -285,7 +285,7 @@ ADM_register_adhoc_storage(hg_handle_t h) { rpc_id, std::quoted(__FUNCTION__), std::quoted(get_address(h)), name, type, ctx); - admire::error_code ec = ADM_SUCCESS; + admire::error_code ec; std::uint64_t out_adhoc_id = 0; auto& adhoc_manager = scord::adhoc_storage_manager::instance(); @@ -1134,17 +1134,17 @@ ADM_transfer_datasets(hg_handle_t h) { id, std::quoted(__FUNCTION__), std::quoted(get_address(h)), job, sources, targets, limits, mapping); - admire::error_code rv = ADM_SUCCESS; + admire::error_code ec; const auto transfer = admire::transfer{42}; out.op_id = id; - out.retval = rv; + out.retval = ec; out.tx = admire::api::convert(transfer).release(); LOGGER_INFO("rpc id: {} name: {} to: {} <= " "body: {{retval: {}, transfer: {}}}", - id, std::quoted(__FUNCTION__), std::quoted(get_address(h)), rv, + id, std::quoted(__FUNCTION__), std::quoted(get_address(h)), ec, transfer); ret = margo_respond(h, &out); -- GitLab