phrocker commented on a change in pull request #558: MINIFICPP-542 - Add PutSFTP processor URL: https://github.com/apache/nifi-minifi-cpp/pull/558#discussion_r285620210
########## File path: extensions/sftp/client/SFTPClient.h ########## @@ -0,0 +1,183 @@ +/** + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#ifndef __SFTP_CLIENT_H__ +#define __SFTP_CLIENT_H__ + +#include <curl/curl.h> +#include <libssh2.h> +#include <libssh2_sftp.h> +#include <vector> +#include <iostream> +#include <string> +#include <uuid/uuid.h> +#include <vector> + +#include "utils/HTTPClient.h" +#include "core/logging/Logger.h" +#include "core/logging/LoggerConfiguration.h" +#include "properties/Configure.h" +#include "io/validation.h" +#include "io/BaseStream.h" + +namespace org { +namespace apache { +namespace nifi { +namespace minifi { +namespace utils { + +/** + * Initializes and cleans up curl and libssh2 once. + * Cleanup will only occur at the end of our execution since we are relying on a static variable. + */ +class SFTPClientInitializer { + public: + static SFTPClientInitializer *getInstance() { + static SFTPClientInitializer initializer; + return &initializer; + } + void initialize() { + + } + private: + SFTPClientInitializer() { + libssh2_init(0); + curl_global_init(CURL_GLOBAL_DEFAULT); Review comment: @bakaid Thanks for pointing that out. One option is create a varying dependency tree of lazy initializers -- but I think as you point out that will be error prone. Extensions were intended to be groups of functionality and one other issue is that the bootstrap script will eventually run out of characters. I'd like to get to a point where more of these extensions that rely on the same dependencies are combined. Another option here could be combining sftp and http-curl into a curl extension, and using the HTTPCurlLoader functionality to eagerly initialize these components. We can still have SFTP be an included component within that extension, but then all curl based functionality will be included within a single extension with the ability to pick and choose sub-features as needed. Does that make sense? There will be more features that use curl, so having http-curl and other curl reliant extensions doesn't make a ton of sense to me. We can still have the same ability to limit features included and also have the byproduct that initialization can be done via the ObjectFactory ( a component of the class loader that is loaded before all processors start ). While we certainly could have the case where something "like this" happens again, I'm not sure we can avoid that as that's a consistent issue amongst Java Classloaders, too. What are your thoughts on this? It can be something we do after this is merged, labeling it a blocker to get it done, but I'll leave that up to you since there is a ton of good work here and I don't want to slow this up for such a change unless you are open to do that. I'll give it more thought, but those are two options I can think of that would solve this and other issues that will arise by having the split we have. ---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services
