Skip to content

Commit

Permalink
General improvement to Environment Variables access
Browse files Browse the repository at this point in the history
- Concentrate the Environment Variables under a single API
  specified at ecflow/core/Environment.hpp
- Replace all raw calls to ::getenv()

Re ECFLOW-1957
  • Loading branch information
marcosbento committed Sep 20, 2024
1 parent b7e9139 commit fca83b6
Show file tree
Hide file tree
Showing 64 changed files with 677 additions and 581 deletions.
2 changes: 2 additions & 0 deletions libs/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,8 @@ set(srcs
core/src/ecflow/core/DurationTimer.hpp
core/src/ecflow/core/Ecf.hpp
core/src/ecflow/core/EcfPortLock.hpp
core/src/ecflow/core/Enumerate.hpp
core/src/ecflow/core/Environment.hpp
core/src/ecflow/core/Extract.hpp
core/src/ecflow/core/File.hpp
core/src/ecflow/core/File_r.hpp
Expand Down
11 changes: 4 additions & 7 deletions libs/base/src/ecflow/base/Openssl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
#include "ecflow/base/Openssl.hpp"

#include <cassert>
#include <cstdlib> // getenv
#include <stdexcept>

#include "ecflow/core/Environment.hpp"
#include "ecflow/core/File.hpp"
#include "ecflow/core/Host.hpp"
#include "ecflow/core/Str.hpp"
Expand Down Expand Up @@ -80,9 +80,8 @@ void Openssl::enable(std::string host, const std::string& port) {
}

void Openssl::enable_if_defined(std::string host, const std::string& port) {
char* ecf_ssl = getenv("ECF_SSL");
if (ecf_ssl) {
std::string ecf_ssl_env = ecf_ssl;
if (auto ecf_ssl = ecf::environment::fetch<std::string>(ecf::environment::ECF_SSL); ecf_ssl) {
std::string ecf_ssl_env = ecf_ssl.value();

if (host == Str::LOCALHOST())
host = Host().name();
Expand Down Expand Up @@ -161,9 +160,7 @@ std::string Openssl::get_password() const {
}

std::string Openssl::certificates_dir() const {
std::string home_path = getenv("HOME");
home_path += "/.ecflowrc/ssl/";
return home_path;
return ecf::environment::get<std::string>("HOME") + "/.ecflowrc/ssl/";
}

std::string Openssl::pem() const {
Expand Down
3 changes: 2 additions & 1 deletion libs/base/src/ecflow/base/cts/task/CtsWaitCmd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "ecflow/base/AbstractServer.hpp"
#include "ecflow/base/cts/task/TaskApi.hpp"
#include "ecflow/base/stc/PreAllocatedReply.hpp"
#include "ecflow/core/Environment.hpp"
#include "ecflow/core/Log.hpp"
#include "ecflow/core/Str.hpp"
#include "ecflow/node/Defs.hpp"
Expand Down Expand Up @@ -128,7 +129,7 @@ bool TaskCmd::authenticate(AbstractServer* as, STC_Cmd_ptr& theReply) const {
/// This can be done via AlterCmd by adding a variable on the task, ECF_PASS with value
/// Submittable::FREE_JOBS_PASSWORD Note: this *does not* look for the variable up the node tree, only on the task.
std::string ecf_pass_value;
if (submittable_->findVariableValue(Str::ECF_PASS(), ecf_pass_value)) {
if (submittable_->findVariableValue(ecf::environment::ECF_PASS, ecf_pass_value)) {

if (ecf_pass_value == Submittable::FREE_JOBS_PASSWORD()) {
submittable_->flag().clear(ecf::Flag::ZOMBIE);
Expand Down
3 changes: 2 additions & 1 deletion libs/base/src/ecflow/base/cts/task/TaskCmd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

#include "ecflow/base/AbstractServer.hpp"
#include "ecflow/base/stc/PreAllocatedReply.hpp"
#include "ecflow/core/Environment.hpp"
#include "ecflow/core/Log.hpp"
#include "ecflow/node/Defs.hpp"
#include "ecflow/node/Submittable.hpp"
Expand Down Expand Up @@ -123,7 +124,7 @@ bool TaskCmd::authenticate(AbstractServer* as, STC_Cmd_ptr& theReply) const {
/// This can be done via AlterCmd by adding a variable on the task, ECF_PASS with value
/// Submittable::FREE_JOBS_PASSWORD Note: this *does not* look for the variable up the node tree, only on the task.
std::string ecf_pass_value;
if (submittable_->findVariableValue(Str::ECF_PASS(), ecf_pass_value)) {
if (submittable_->findVariableValue(ecf::environment::ECF_PASS, ecf_pass_value)) {

if (ecf_pass_value == Submittable::FREE_JOBS_PASSWORD()) {
submittable_->flag().clear(ecf::Flag::ZOMBIE);
Expand Down
5 changes: 3 additions & 2 deletions libs/base/src/ecflow/base/cts/user/AlterCmd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "ecflow/base/cts/user/CtsApi.hpp"
#include "ecflow/core/Converter.hpp"
#include "ecflow/core/Enumerate.hpp"
#include "ecflow/core/Environment.hpp"
#include "ecflow/core/Extract.hpp"
#include "ecflow/core/Log.hpp"
#include "ecflow/core/Message.hpp"
Expand Down Expand Up @@ -296,8 +297,8 @@ STC_Cmd_ptr AlterCmd::alter_server_state(AbstractServer* as) const {
else if (change_attr_type_ == AlterCmd::VARIABLE || add_attr_type_ == AlterCmd::ADD_VARIABLE) {

// ECFLOW-380: Some variable should be read only
if (name_ == Str::ECF_HOST() || name_ == Str::ECF_PORT() || name_ == "ECF_PID" || name_ == "ECF_VERSION" ||
name_ == "ECF_LISTS") {
if (name_ == ecf::environment::ECF_HOST || name_ == ecf::environment::ECF_PORT || name_ == "ECF_PID" ||
name_ == "ECF_VERSION" || name_ == "ECF_LISTS") {
std::stringstream ss;
ss << "AlterCmd:: Cannot add or change read only server variable " << name_;
throw std::runtime_error(ss.str());
Expand Down
17 changes: 9 additions & 8 deletions libs/base/src/ecflow/base/cts/user/CFileCmd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "ecflow/base/cts/user/CtsApi.hpp"
#include "ecflow/base/stc/PreAllocatedReply.hpp"
#include "ecflow/core/Converter.hpp"
#include "ecflow/core/Environment.hpp"
#include "ecflow/core/File.hpp"
#include "ecflow/node/EcfFile.hpp"
#include "ecflow/node/Submittable.hpp"
Expand Down Expand Up @@ -174,7 +175,7 @@ STC_Cmd_ptr CFileCmd::doHandleRequest(AbstractServer* as) const {

case CFileCmd::JOB: {
std::string ecf_job_file;
submittable->findParentVariableValue(Str::ECF_JOB(), ecf_job_file);
submittable->findParentVariableValue(ecf::environment::ECF_JOB, ecf_job_file);
if (!File::open(ecf_job_file, fileContents)) {
std::stringstream ss;
ss << "CFileCmd::doHandleRequest: Failed to open the job file('" << ecf_job_file << "') for task "
Expand All @@ -198,19 +199,19 @@ STC_Cmd_ptr CFileCmd::doHandleRequest(AbstractServer* as) const {
// First try user variable, if defined this has priority ECFLOW-999
std::stringstream ss;
std::string user_jobout;
if (submittable->findParentUserVariableValue(Str::ECF_JOBOUT(), user_jobout)) {
if (submittable->findParentUserVariableValue(ecf::environment::ECF_JOBOUT, user_jobout)) {
if (File::open(user_jobout, fileContents))
break;
ss << "Failed to open user specified job-out(ECF_JOBOUT='" << user_jobout << "') ";
}

const Variable& ecf_jobout_gen_var = submittable->findGenVariable(Str::ECF_JOBOUT());
const Variable& ecf_jobout_gen_var = submittable->findGenVariable(ecf::environment::ECF_JOBOUT);
if (!File::open(ecf_jobout_gen_var.theValue(), fileContents)) {

// If that fails as a backup, look under ECF_HOME/ECF_NAME.ECF_TRYNO, ECFLOW-177 preserve old SMS
// behaviour
std::string ecfhome_jobout;
submittable->findParentUserVariableValue(Str::ECF_HOME(), ecfhome_jobout);
submittable->findParentUserVariableValue(ecf::environment::ECF_HOME, ecfhome_jobout);
ecfhome_jobout += submittable->absNodePath();
ecfhome_jobout += ".";
ecfhome_jobout += submittable->tryNo();
Expand Down Expand Up @@ -239,7 +240,7 @@ STC_Cmd_ptr CFileCmd::doHandleRequest(AbstractServer* as) const {

case CFileCmd::KILL: {
std::string ecf_job_file;
submittable->findParentVariableValue(Str::ECF_JOB(), ecf_job_file);
submittable->findParentVariableValue(ecf::environment::ECF_JOB, ecf_job_file);
std::string file = ecf_job_file + ".kill";
if (!File::open(file, fileContents)) {
std::stringstream ss;
Expand All @@ -252,7 +253,7 @@ STC_Cmd_ptr CFileCmd::doHandleRequest(AbstractServer* as) const {

case CFileCmd::STAT: {
std::string ecf_job_file;
submittable->findParentVariableValue(Str::ECF_JOB(), ecf_job_file);
submittable->findParentVariableValue(ecf::environment::ECF_JOB, ecf_job_file);
std::string file = ecf_job_file + ".stat";
if (!File::open(file, fileContents)) {
std::stringstream ss;
Expand All @@ -275,7 +276,7 @@ STC_Cmd_ptr CFileCmd::doHandleRequest(AbstractServer* as) const {

// First look for .man files in ECF_FILES and then ECF_HOME
std::string ecf_files;
node->findParentUserVariableValue(Str::ECF_FILES(), ecf_files);
node->findParentUserVariableValue(ecf::environment::ECF_FILES, ecf_files);
if (!ecf_files.empty() && fs::is_directory(ecf_files)) {

std::string manFile = File::backwardSearch(ecf_files, node->absNodePath(), File::MAN_EXTN());
Expand All @@ -289,7 +290,7 @@ STC_Cmd_ptr CFileCmd::doHandleRequest(AbstractServer* as) const {
if (fileContents.empty()) {
// Try under ECF_HOME
std::string ecf_home;
node->findParentUserVariableValue(Str::ECF_HOME(), ecf_home);
node->findParentUserVariableValue(ecf::environment::ECF_HOME, ecf_home);
if (!ecf_home.empty() && fs::is_directory(ecf_home)) {

std::string manFile = File::backwardSearch(ecf_home, node->absNodePath(), File::MAN_EXTN());
Expand Down
6 changes: 4 additions & 2 deletions libs/base/src/ecflow/base/cts/user/LogCmd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "ecflow/base/cts/user/CtsApi.hpp"
#include "ecflow/base/stc/PreAllocatedReply.hpp"
#include "ecflow/core/Converter.hpp"
#include "ecflow/core/Environment.hpp"
#include "ecflow/core/Log.hpp"
#include "ecflow/core/Str.hpp"
#include "ecflow/node/Defs.hpp"
Expand Down Expand Up @@ -145,12 +146,13 @@ STC_Cmd_ptr LogCmd::doHandleRequest(AbstractServer* as) const {
// This is done adding it as a *USER* variable. This overloads the server variables
// It also allows us to see the change in GUI. Note: Defs/server_variables are not synced
// ECFLOW-376
as->defs()->set_server().add_or_update_user_variables(Str::ECF_LOG(), Log::instance()->path());
as->defs()->set_server().add_or_update_user_variables(ecf::environment::ECF_LOG,
Log::instance()->path());
}
else {
// User could have overridden ECF_LOG variable
// *FIRST* look at user variables, then look at *server* variables.
std::string log_file_name = as->defs()->server().find_variable(Str::ECF_LOG());
std::string log_file_name = as->defs()->server().find_variable(ecf::environment::ECF_LOG);

// ECFLOW-377 should remove leading/trailing spaces from path
ecf::algorithm::trim(log_file_name);
Expand Down
14 changes: 8 additions & 6 deletions libs/base/test/TestAlterCmd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "ecflow/base/cts/user/PathsCmd.hpp"
#include "ecflow/base/cts/user/RequeueNodeCmd.hpp"
#include "ecflow/core/Ecf.hpp"
#include "ecflow/core/Environment.hpp"
#include "ecflow/core/Str.hpp"
#include "ecflow/node/Defs.hpp"
#include "ecflow/node/Family.hpp"
Expand Down Expand Up @@ -959,15 +960,16 @@ BOOST_AUTO_TEST_CASE(test_alter_cmd) {
{ // free password
TestStateChanged changed(s);
std::string returnedValue;
BOOST_CHECK_MESSAGE(!task->findVariableValue(Str::ECF_PASS(), returnedValue),
BOOST_CHECK_MESSAGE(!task->findVariableValue(ecf::environment::ECF_PASS, returnedValue),
"Expected no variable of name ECF_PASS");

TestHelper::invokeRequest(
&defs,
Cmd_ptr(new AlterCmd(
task->absNodePath(), AlterCmd::ADD_VARIABLE, Str::ECF_PASS(), Submittable::FREE_JOBS_PASSWORD())));
TestHelper::invokeRequest(&defs,
Cmd_ptr(new AlterCmd(task->absNodePath(),
AlterCmd::ADD_VARIABLE,
ecf::environment::ECF_PASS,
Submittable::FREE_JOBS_PASSWORD())));

BOOST_CHECK_MESSAGE(task->findVariableValue(Str::ECF_PASS(), returnedValue),
BOOST_CHECK_MESSAGE(task->findVariableValue(ecf::environment::ECF_PASS, returnedValue),
"Expected to find variable ECF_PASS on the task");
BOOST_CHECK_MESSAGE(returnedValue == Submittable::FREE_JOBS_PASSWORD(),
"Expected variable value of name " << Submittable::FREE_JOBS_PASSWORD() << " but found "
Expand Down
13 changes: 7 additions & 6 deletions libs/base/test/TestArchiveAndRestoreCmd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "ecflow/base/cts/user/DeleteCmd.hpp"
#include "ecflow/base/cts/user/PathsCmd.hpp"
#include "ecflow/base/cts/user/RequeueNodeCmd.hpp"
#include "ecflow/core/Environment.hpp"
#include "ecflow/core/File.hpp"
#include "ecflow/core/Pid.hpp"
#include "ecflow/core/Str.hpp"
Expand Down Expand Up @@ -51,7 +52,7 @@ BOOST_AUTO_TEST_CASE(test_archive_and_restore_suite) {
// We use Pid::unique_name, to allow multiple invocation of this test
Defs theDefs;
std::string ecf_home = File::test_data("libs/base/test", "libs/base");
theDefs.set_server().add_or_update_user_variables(Str::ECF_HOME(), ecf_home);
theDefs.set_server().add_or_update_user_variables(ecf::environment::ECF_HOME, ecf_home);
suite_ptr suite = theDefs.add_suite(Pid::unique_name("test_archive_and_restore_suite"));
suite->add_family("f1")->add_task("t1");
// cout << theDefs << "\n";
Expand Down Expand Up @@ -97,7 +98,7 @@ BOOST_AUTO_TEST_CASE(test_archive_and_restore_family) {

Defs theDefs;
std::string ecf_home = File::test_data("libs/base/test", "libs/base");
theDefs.set_server().add_or_update_user_variables(Str::ECF_HOME(), ecf_home);
theDefs.set_server().add_or_update_user_variables(ecf::environment::ECF_HOME, ecf_home);
suite_ptr suite = theDefs.add_suite(Pid::unique_name("test_archive_and_restore_family"));
family_ptr f3 = suite->add_family("f1")->add_family("f2")->add_family("f3");
f3->add_task("t1");
Expand Down Expand Up @@ -148,7 +149,7 @@ BOOST_AUTO_TEST_CASE(test_archive_and_restore_all) {
Defs theDefs;
{
std::string ecf_home = File::test_data("libs/base/test", "libs/base");
theDefs.set_server().add_or_update_user_variables(Str::ECF_HOME(), ecf_home);
theDefs.set_server().add_or_update_user_variables(ecf::environment::ECF_HOME, ecf_home);
suite_ptr suite = theDefs.add_suite(Pid::unique_name("test_archive_and_restore_all"));
family_ptr f1 = suite->add_family("f1");
f1->add_task("t1");
Expand Down Expand Up @@ -244,7 +245,7 @@ BOOST_AUTO_TEST_CASE(test_archive_and_restore_overlap) {

Defs theDefs;
std::string ecf_home = File::test_data("libs/base/test", "libs/base");
theDefs.set_server().add_or_update_user_variables(Str::ECF_HOME(), ecf_home);
theDefs.set_server().add_or_update_user_variables(ecf::environment::ECF_HOME, ecf_home);
suite_ptr suite = theDefs.add_suite(Pid::unique_name("test_archive_and_restore_overlap"));
std::string f1_abs_node_path;
{
Expand Down Expand Up @@ -290,7 +291,7 @@ BOOST_AUTO_TEST_CASE(test_archive_and_delete_suite) {
// We use Pid::unique_name, to allow multiple invocation of this test
Defs theDefs;
std::string ecf_home = File::test_data("libs/base/test", "libs/base");
theDefs.set_server().add_or_update_user_variables(Str::ECF_HOME(), ecf_home);
theDefs.set_server().add_or_update_user_variables(ecf::environment::ECF_HOME, ecf_home);
suite_ptr suite = theDefs.add_suite(Pid::unique_name("test_archive_and_delete_suite"));
family_ptr family = suite->add_family("f1");
family->add_task("t1");
Expand Down Expand Up @@ -327,7 +328,7 @@ BOOST_AUTO_TEST_CASE(test_archive_and_restore_errors) {
// We use Pid::unique_name, to allow multiple invocation of this test
Defs theDefs;
std::string ecf_home = File::test_data("libs/base/test", "libs/base");
theDefs.set_server().add_or_update_user_variables(Str::ECF_HOME(), ecf_home);
theDefs.set_server().add_or_update_user_variables(ecf::environment::ECF_HOME, ecf_home);
suite_ptr suite = theDefs.add_suite(Pid::unique_name("test_archive_and_restore_errors"));
family_ptr f1 = suite->add_family("f1");
f1->add_task("t1");
Expand Down
3 changes: 2 additions & 1 deletion libs/base/test/TestCmd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "ecflow/base/cts/user/BeginCmd.hpp"
#include "ecflow/base/cts/user/CtsCmd.hpp"
#include "ecflow/core/Converter.hpp"
#include "ecflow/core/Environment.hpp"
#include "ecflow/core/Str.hpp"
#include "ecflow/node/Defs.hpp"
#include "ecflow/node/Family.hpp"
Expand Down Expand Up @@ -82,7 +83,7 @@ BOOST_AUTO_TEST_CASE(test_simple_cmd) {
// should be re-submitted, until the task try number > ECF_TRIES
{
std::string varValue;
if (t1->findParentUserVariableValue(Str::ECF_TRIES(), varValue)) {
if (t1->findParentUserVariableValue(ecf::environment::ECF_TRIES, varValue)) {
auto ecf_tries = ecf::convert_to<int>(varValue);
while (true) {
TestHelper::invokeRequest(
Expand Down
3 changes: 2 additions & 1 deletion libs/base/test/TestLogCmd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "TestHelper.hpp"
#include "TestNaming.hpp"
#include "ecflow/base/cts/user/LogCmd.hpp"
#include "ecflow/core/Environment.hpp"
#include "ecflow/core/File.hpp"
#include "ecflow/core/Log.hpp"
#include "ecflow/core/Str.hpp"
Expand Down Expand Up @@ -83,7 +84,7 @@ BOOST_AUTO_TEST_CASE(test_log_cmd) {
<< defs.server().find_variable("ECF_LOG") << "'");

// Update ECF_LOG to have a *SPACE* at the end. ECFLOW-377
defs.set_server().add_or_update_user_variables(Str::ECF_LOG(), new_log_file);
defs.set_server().add_or_update_user_variables(ecf::environment::ECF_LOG, new_log_file);
BOOST_CHECK_MESSAGE(defs.server().find_variable("ECF_LOG") == new_log_file,
"expected to find ECF_LOG with value '" << new_log_file << "' but found '"
<< defs.server().find_variable("ECF_LOG") << "'");
Expand Down
Loading

0 comments on commit fca83b6

Please sign in to comment.