From issues-return-98364-archive-asf-public=cust-asf.ponee.io@nifi.apache.org Fri Jun 5 06:41:54 2020 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id BB0F318066C for ; Fri, 5 Jun 2020 08:41:53 +0200 (CEST) Received: (qmail 35961 invoked by uid 500); 5 Jun 2020 06:41:53 -0000 Mailing-List: contact issues-help@nifi.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@nifi.apache.org Delivered-To: mailing list issues@nifi.apache.org Received: (qmail 35900 invoked by uid 99); 5 Jun 2020 06:41:53 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 05 Jun 2020 06:41:53 +0000 From: =?utf-8?q?GitBox?= To: issues@nifi.apache.org Subject: =?utf-8?q?=5BGitHub=5D_=5Bnifi-minifi-cpp=5D_hunyadi-dev_commented_on_a_chan?= =?utf-8?q?ge_in_pull_request_=23784=3A_MINIFICPP-1206_-_Rework_and_test_Exe?= =?utf-8?q?cutePythonProcessor=2C_add_in-place_script_support?= Message-ID: <159133931290.17834.12999674421576712844.asfpy@gitbox.apache.org> Date: Fri, 05 Jun 2020 06:41:52 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit References: In-Reply-To: hunyadi-dev commented on a change in pull request #784: URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r435717734 ########## File path: extensions/script/python/ExecutePythonProcessor.cpp ########## @@ -35,155 +35,188 @@ namespace python { namespace processors { core::Property ExecutePythonProcessor::ScriptFile("Script File", // NOLINT - R"(Path to script file to execute)", ""); + R"(Path to script file to execute. Only one of Script File or Script Body may be used)", ""); +core::Property ExecutePythonProcessor::ScriptBody("Script Body", // NOLINT + R"(Script to execute. Only one of Script File or Script Body may be used)", ""); core::Property ExecutePythonProcessor::ModuleDirectory("Module Directory", // NOLINT - R"(Comma-separated list of paths to files and/or directories which - contain modules required by the script)", ""); + R"(Comma-separated list of paths to files and/or directories which contain modules required by the script)", ""); core::Relationship ExecutePythonProcessor::Success("success", "Script successes"); // NOLINT core::Relationship ExecutePythonProcessor::Failure("failure", "Script failures"); // NOLINT void ExecutePythonProcessor::initialize() { // initialization requires that we do a little leg work prior to onSchedule // so that we can provide manifest our processor identity - std::set properties; - - std::string prop; - getProperty(ScriptFile.getName(), prop); - - properties.insert(ScriptFile); - properties.insert(ModuleDirectory); - setSupportedProperties(properties); - - std::set relationships; - relationships.insert(Success); - relationships.insert(Failure); - setSupportedRelationships(std::move(relationships)); - setAcceptAllProperties(); - if (!prop.empty()) { - setProperty(ScriptFile, prop); - std::shared_ptr engine; - python_logger_ = logging::LoggerFactory::getAliasedLogger(getName()); + if (getProperties().empty()) { + setSupportedProperties({ + ScriptFile, + ScriptBody, + ModuleDirectory + }); + setAcceptAllProperties(); + setSupportedRelationships({ + Success, + Failure + }); + valid_init_ = false; + return; + } - engine = createEngine(); + python_logger_ = logging::LoggerFactory::getAliasedLogger(getName()); - if (engine == nullptr) { - throw std::runtime_error("No script engine available"); - } + getProperty(ModuleDirectory.getName(), module_directory_); - try { - engine->evalFile(prop); - auto me = shared_from_this(); - triggerDescribe(engine, me); - triggerInitialize(engine, me); + valid_init_ = false; + appendPathForImportModules(); + loadScript(); + try { + if (script_to_exec_.size()) { + std::shared_ptr engine = getScriptEngine(); + engine->eval(script_to_exec_); + auto shared_this = shared_from_this(); + engine->describe(shared_this); + engine->onInitialize(shared_this); + handleEngineNoLongerInUse(std::move(engine)); valid_init_ = true; - } catch (std::exception &exception) { - logger_->log_error("Caught Exception %s", exception.what()); - engine = nullptr; - std::rethrow_exception(std::current_exception()); - valid_init_ = false; - } catch (...) { - logger_->log_error("Caught Exception"); - engine = nullptr; - std::rethrow_exception(std::current_exception()); - valid_init_ = false; } - + } + catch (const std::exception& exception) { + logger_->log_error("Caught Exception: %s", exception.what()); + std::rethrow_exception(std::current_exception()); + } + catch (...) { + logger_->log_error("Caught Exception"); + std::rethrow_exception(std::current_exception()); } } void ExecutePythonProcessor::onSchedule(const std::shared_ptr &context, const std::shared_ptr &sessionFactory) { if (!valid_init_) { - throw std::runtime_error("Could not correctly in initialize " + getName()); - } - context->getProperty(ScriptFile.getName(), script_file_); - context->getProperty(ModuleDirectory.getName(), module_directory_); - if (script_file_.empty() && script_engine_.empty()) { - logger_->log_error("Script File must be defined"); - return; + throw std::runtime_error("Could not correctly initialize " + getName()); } - try { - std::shared_ptr engine; - - // Use an existing engine, if one is available - if (script_engine_q_.try_dequeue(engine)) { - logger_->log_debug("Using available %s script engine instance", script_engine_); - } else { - logger_->log_info("Creating new %s script instance", script_engine_); - logger_->log_info("Approximately %d %s script instances created for this processor", script_engine_q_.size_approx(), script_engine_); - - engine = createEngine(); - - if (engine == nullptr) { - throw std::runtime_error("No script engine available"); - } - - if (!script_file_.empty()) { - engine->evalFile(script_file_); - } else { - throw std::runtime_error("No Script File is available to execute"); - } + // TODO(hunyadi): When using "Script File" property, we currently re-read the script file content every time the processor is on schedule. This should change to single-read when we release 1.0.0 + // https://issues.apache.org/jira/browse/MINIFICPP-1223 + reloadScriptIfUsingScriptFileProperty(); + if (script_to_exec_.empty()) { + throw std::runtime_error("Neither Script Body nor Script File is available to execute"); } + std::shared_ptr engine = getScriptEngine(); - triggerSchedule(engine, context); + engine->eval(script_to_exec_); + engine->onSchedule(context); - // Make engine available for use again - if (script_engine_q_.size_approx() < getMaxConcurrentTasks()) { - logger_->log_debug("Releasing %s script engine", script_engine_); - script_engine_q_.enqueue(engine); - } else { - logger_->log_info("Destroying script engine because it is no longer needed"); - } - } catch (std::exception &exception) { - logger_->log_error("Caught Exception %s", exception.what()); - } catch (...) { + handleEngineNoLongerInUse(std::move(engine)); + } + catch (const std::exception& exception) { + logger_->log_error("Caught Exception: %s", exception.what()); + } + catch (...) { logger_->log_error("Caught Exception"); } } void ExecutePythonProcessor::onTrigger(const std::shared_ptr &context, const std::shared_ptr &session) { + if (!valid_init_) { + throw std::runtime_error("Could not correctly initialize " + getName()); + } try { - std::shared_ptr engine; - - // Use an existing engine, if one is available - if (script_engine_q_.try_dequeue(engine)) { - logger_->log_debug("Using available %s script engine instance", script_engine_); - } else { - logger_->log_info("Creating new %s script instance", script_engine_); - logger_->log_info("Approximately %d %s script instances created for this processor", script_engine_q_.size_approx(), script_engine_); - - engine = createEngine(); - - if (engine == nullptr) { - throw std::runtime_error("No script engine available"); - } - - if (!script_file_.empty()) { - engine->evalFile(script_file_); - } else { - throw std::runtime_error("No Script File is available to execute"); - } + // TODO(hunyadi): When using "Script File" property, we currently re-read the script file content every time the processor is on schedule. This should change to single-read when we release 1.0.0 Review comment: Comment corrected. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org