[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1599: MINIFICPP-2152 Remove own server and client socket implementations
szaszm commented on code in PR #1599:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1599#discussion_r1301181584
##
libminifi/include/utils/net/AsioSocketUtils.h:
##
@@ -63,6 +64,64 @@ asio::awaitable>
handshake(SslSocket& socket, asio::
asio::ssl::context getSslContext(const controllers::SSLContextService&
ssl_context_service, asio::ssl::context::method ssl_context_method =
asio::ssl::context::tlsv12_client);
+
+struct SocketData {
+ std::string host = "localhost";
+ int port = -1;
+ std::shared_ptr ssl_context_service;
+};
+
+class AsioSocketConnection : public io::BaseStream {
+ public:
+ explicit AsioSocketConnection(SocketData socket_data);
+ int initialize() override;
+ size_t read(std::span out_buffer) override {
+gsl_Expects(stream_);
+return stream_->read(out_buffer);
+ }
+ size_t write(const uint8_t *in_buffer, size_t len) override {
+gsl_Expects(stream_);
+return stream_->write(in_buffer, len);
+ }
+
+ void setInterface(const std::string& local_network_interface) {
+local_network_interface_ = local_network_interface;
+ }
+
+ private:
+ template
+ bool bindToLocalInterface(SocketType& socket) {
+if (local_network_interface_.empty()) {
+ return true;
+}
+
+asio::ip::tcp::endpoint
local_endpoint(asio::ip::address::from_string(local_network_interface_), 0);
+asio::error_code err;
+socket.open(local_endpoint.protocol(), err);
+if (err) {
+ logger_->log_error("Failed to open socket on network interface '%s' with
the following message: '%s'", local_network_interface_, err.message());
+ return false;
+}
+socket.set_option(asio::ip::tcp::socket::reuse_address(true));
Review Comment:
What's the reason for this `reuse_address(true)` option? I think normally
it's used to be able to quickly restart a server program that binds to the same
address and port as the old process (of the same software) did. We should avoid
`bind` and `reuse_address(true)` for client connections: let closed connections
linger in the TIME_WAIT state, and let the dynamic binding of ports assign
another random port to the client process.
##
libminifi/src/utils/net/AsioSocketUtils.cpp:
##
@@ -44,4 +47,66 @@ asio::ssl::context getSslContext(const
controllers::SSLContextService& ssl_conte
return ssl_context;
}
+AsioSocketConnection::AsioSocketConnection(SocketData socket_data) :
socket_data_(std::move(socket_data)) {
+}
+
+int AsioSocketConnection::initialize() {
+ bool result = false;
+ if (socket_data_.ssl_context_service) {
+result = connectTcpSocketOverSsl();
+ } else {
+result = connectTcpSocket();
+ }
+ return result ? 0 : -1;
+}
+
+bool AsioSocketConnection::connectTcpSocketOverSsl() {
+ auto ssl_context =
utils::net::getSslContext(*socket_data_.ssl_context_service);
+ asio::ssl::stream socket(io_context_, ssl_context);
+
+ bindToLocalInterface(socket.lowest_layer());
Review Comment:
As mentioned in the previous comment: client sockets (that initiate a
connection) should probably not bind to anything.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[jira] [Assigned] (MINIFICPP-1774) Set properties from command line arguments
[ https://issues.apache.org/jira/browse/MINIFICPP-1774?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Gábor Gyimesi reassigned MINIFICPP-1774: Assignee: Gábor Gyimesi > Set properties from command line arguments > -- > > Key: MINIFICPP-1774 > URL: https://issues.apache.org/jira/browse/MINIFICPP-1774 > Project: Apache NiFi MiNiFi C++ > Issue Type: Improvement >Reporter: Marton Szasz >Assignee: Gábor Gyimesi >Priority: Major > > Add a command line parser that can override properties from the > minifi.properties config file. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[GitHub] [nifi] turcsanyip commented on a diff in pull request #7588: NIFI-11924 Closing FileSystem after using in HDFSExternalResourceProvider
turcsanyip commented on code in PR #7588:
URL: https://github.com/apache/nifi/pull/7588#discussion_r1301140166
##
nifi-nar-bundles/nifi-hadoop-bundle/nifi-hdfs-processors/src/main/java/org/apache/nifi/flow/resource/hadoop/HDFSExternalResourceProvider.java:
##
@@ -140,13 +146,16 @@ public InputStream fetchExternalResource(final
ExternalResourceDescriptor descri
final HdfsResources hdfsResources = getHdfsResources();
try {
-return
hdfsResources.getUserGroupInformation().doAs((PrivilegedExceptionAction)
() -> {
-if (!hdfsResources.getFileSystem().exists(path)) {
-throw new IOException("Cannot find file in HDFS at
location " + location);
-}
-
-return hdfsResources.getFileSystem().open(path,
BUFFER_SIZE_DEFAULT);
-});
+final FSDataInputStream fsDataInputStream =
+
hdfsResources.getUserGroupInformation().doAs((PrivilegedExceptionAction)
() -> {
+if (!hdfsResources.getFileSystem().exists(path)) {
+throw new IOException("Cannot find file in HDFS at
location " + location);
+}
+
+return hdfsResources.getFileSystem().open(path,
BUFFER_SIZE_DEFAULT);
+});
+// The acquired InputStream is used by the client and cannot be
closed here. The decorator is responsible for that.
Review Comment:
I also think some additional documentation would not be useless.
Regarding the "why": the point is that the `FileSystem` cannot be closed
here so I would modify the comment:
```suggestion
// The acquired InputStream is used by the client and for this
reason the FileSystem cannot be closed here. The decorator is responsible for
that.
```
Regarding the "how": I would highlight which method behaves differently and
does not simply delegates the call to the decorated object. It could be added
in the javadoc of `HDFSResourceInputStream` or simply here.
I agree that the benefits of patterns is sharing the intention, however it
is useful to highlight the additional behaviour in case of a decorator.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[jira] [Updated] (MINIFICPP-1403) Create TagS3Object processor
[ https://issues.apache.org/jira/browse/MINIFICPP-1403?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Gábor Gyimesi updated MINIFICPP-1403: - Priority: Trivial (was: Minor) > Create TagS3Object processor > > > Key: MINIFICPP-1403 > URL: https://issues.apache.org/jira/browse/MINIFICPP-1403 > Project: Apache NiFi MiNiFi C++ > Issue Type: New Feature >Reporter: Gábor Gyimesi >Priority: Trivial > > Create new processor to tag S3 objects of a bucket with similar functionality > defined in Nifi: > [https://nifi.apache.org/docs/nifi-docs/components/org.apache.nifi/nifi-aws-nar/1.13.2/org.apache.nifi.processors.aws.s3.TagS3Object/index.htmlhttps://nifi.apache.org/docs/nifi-docs/components/nifi-docs/components/org.apache.nifi/nifi-aws-nar/1.9.0/org.apache.nifi.processors.aws.s3.TagS3Object/index.htmlhttps://nifi.apache.org/docs/nifi-docs/components/org.apache.nifi/nifi-aws-nar/1.12.1/org.apache.nifi.processors.aws.s3.ListS3/index.html|https://nifi.apache.org/docs/nifi-docs/components/org.apache.nifi/nifi-aws-nar/1.12.1/org.apache.nifi.processors.aws.s3.ListS3/index.html] -- This message was sent by Atlassian Jira (v8.20.10#820010)
[GitHub] [nifi-minifi-cpp] martinzink opened a new pull request, #1637: MINIFICPP-2189 Fixing gcc-13 compatibility CI
martinzink opened a new pull request, #1637: URL: https://github.com/apache/nifi-minifi-cpp/pull/1637 Thank you for submitting a contribution to Apache NiFi - MiNiFi C++. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Does your PR title start with MINIFICPP- where is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character. - [ ] Has your PR been rebased against the latest commit within the target branch (typically main)? - [ ] Is your initial contribution a single, squashed commit? ### For code changes: - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? - [ ] If applicable, have you updated the LICENSE file? - [ ] If applicable, have you updated the NOTICE file? ### For documentation related changes: - [ ] Have you ensured that format looks appropriate for the output in which it is rendered? ### Note: Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible. -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
