[GitHub] arpadboda commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
arpadboda commented on a change in pull request #470: MINIFICPP-706 -
RawSiteToSite: remove code duplication
URL: https://github.com/apache/nifi-minifi-cpp/pull/470#discussion_r252667506
##
File path: libminifi/src/sitetosite/RawSocketProtocol.cpp
##
@@ -395,97 +395,37 @@ bool
RawSiteToSiteClient::getPeerList(std::vector ) {
}
}
-int RawSiteToSiteClient::writeRequestType(RequestType type) {
- if (type >= MAX_REQUEST_TYPE)
-return -1;
-
- return peer_->writeUTF(SiteToSiteRequest::RequestTypeStr[type]);
-}
-
-int RawSiteToSiteClient::readRequestType(RequestType ) {
- std::string requestTypeStr;
-
- int ret = peer_->readUTF(requestTypeStr);
-
- if (ret <= 0)
-return ret;
+ int RawSiteToSiteClient::writeRequestType(RequestType type) {
+if (type >= MAX_REQUEST_TYPE)
+ return -1;
- for (int i = NEGOTIATE_FLOWFILE_CODEC; i <= SHUTDOWN; i++) {
-if (SiteToSiteRequest::RequestTypeStr[i] == requestTypeStr) {
- type = (RequestType) i;
- return ret;
-}
+return peer_->writeUTF(SiteToSiteRequest::RequestTypeStr[type]);
}
- return -1;
-}
-
-int RawSiteToSiteClient::readRespond(const std::shared_ptr
, RespondCode , std::string ) {
- uint8_t firstByte;
Review comment:
Two things I did:
-Executed site2site related unit tests
-Verified transfers from MiNiFi to NiFi using the C examples (which still
rely on C++ S2S implementation atm)
Any further verification is welcome and thanks in advance for that!
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] arpadboda commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
arpadboda commented on a change in pull request #470: MINIFICPP-706 -
RawSiteToSite: remove code duplication
URL: https://github.com/apache/nifi-minifi-cpp/pull/470#discussion_r252667506
##
File path: libminifi/src/sitetosite/RawSocketProtocol.cpp
##
@@ -395,97 +395,37 @@ bool
RawSiteToSiteClient::getPeerList(std::vector ) {
}
}
-int RawSiteToSiteClient::writeRequestType(RequestType type) {
- if (type >= MAX_REQUEST_TYPE)
-return -1;
-
- return peer_->writeUTF(SiteToSiteRequest::RequestTypeStr[type]);
-}
-
-int RawSiteToSiteClient::readRequestType(RequestType ) {
- std::string requestTypeStr;
-
- int ret = peer_->readUTF(requestTypeStr);
-
- if (ret <= 0)
-return ret;
+ int RawSiteToSiteClient::writeRequestType(RequestType type) {
+if (type >= MAX_REQUEST_TYPE)
+ return -1;
- for (int i = NEGOTIATE_FLOWFILE_CODEC; i <= SHUTDOWN; i++) {
-if (SiteToSiteRequest::RequestTypeStr[i] == requestTypeStr) {
- type = (RequestType) i;
- return ret;
-}
+return peer_->writeUTF(SiteToSiteRequest::RequestTypeStr[type]);
}
- return -1;
-}
-
-int RawSiteToSiteClient::readRespond(const std::shared_ptr
, RespondCode , std::string ) {
- uint8_t firstByte;
Review comment:
Two things I did:
-Execute site2site related unit tests
-Verified transfers from MiNiFi to NiFi using the C examples (which still
rely on C++ S2S implementation atm)
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] arpadboda commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
arpadboda commented on a change in pull request #470: MINIFICPP-706 -
RawSiteToSite: remove code duplication
URL: https://github.com/apache/nifi-minifi-cpp/pull/470#discussion_r249948870
##
File path: libminifi/src/sitetosite/RawSocketProtocol.cpp
##
@@ -395,97 +395,37 @@ bool
RawSiteToSiteClient::getPeerList(std::vector ) {
}
}
-int RawSiteToSiteClient::writeRequestType(RequestType type) {
- if (type >= MAX_REQUEST_TYPE)
-return -1;
-
- return peer_->writeUTF(SiteToSiteRequest::RequestTypeStr[type]);
-}
-
-int RawSiteToSiteClient::readRequestType(RequestType ) {
- std::string requestTypeStr;
-
- int ret = peer_->readUTF(requestTypeStr);
-
- if (ret <= 0)
-return ret;
+ int RawSiteToSiteClient::writeRequestType(RequestType type) {
+if (type >= MAX_REQUEST_TYPE)
+ return -1;
- for (int i = NEGOTIATE_FLOWFILE_CODEC; i <= SHUTDOWN; i++) {
-if (SiteToSiteRequest::RequestTypeStr[i] == requestTypeStr) {
- type = (RequestType) i;
- return ret;
-}
+return peer_->writeUTF(SiteToSiteRequest::RequestTypeStr[type]);
}
- return -1;
-}
-
-int RawSiteToSiteClient::readRespond(const std::shared_ptr
, RespondCode , std::string ) {
- uint8_t firstByte;
Review comment:
That's the case.
The implementation here was similar to "ReadResponse", which exists in base
class, where it's required.
The change I made was to call that instead of copy-pasting.
"ReadResponse" implementation however cannot be removed from base as http
client relies on that: it overrides, but calls base implementation in a case.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] arpadboda commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
arpadboda commented on a change in pull request #470: MINIFICPP-706 -
RawSiteToSite: remove code duplication
URL: https://github.com/apache/nifi-minifi-cpp/pull/470#discussion_r249233347
##
File path: libminifi/include/sitetosite/SiteToSiteClient.h
##
@@ -221,12 +221,8 @@ class SiteToSiteClient : public core::Connectable {
// deleteTransaction
virtual void deleteTransaction(std::string transactionID);
- virtual void tearDown();
-
- // write Request Type
- virtual int writeRequestType(RequestType type);
- // read Request Type
- virtual int readRequestType(RequestType );
+ virtual void tearDown() = 0;
Review comment:
This exists in both http and raw, with different implementations, so I guess
it's fine here as pure virtual.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] arpadboda commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
arpadboda commented on a change in pull request #470: MINIFICPP-706 -
RawSiteToSite: remove code duplication
URL: https://github.com/apache/nifi-minifi-cpp/pull/470#discussion_r249233347
##
File path: libminifi/include/sitetosite/SiteToSiteClient.h
##
@@ -221,12 +221,8 @@ class SiteToSiteClient : public core::Connectable {
// deleteTransaction
virtual void deleteTransaction(std::string transactionID);
- virtual void tearDown();
-
- // write Request Type
- virtual int writeRequestType(RequestType type);
- // read Request Type
- virtual int readRequestType(RequestType );
+ virtual void tearDown() = 0;
Review comment:
It exist in both http and raw, with different implementation, so I guess
it's fine here as pure virtual.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] arpadboda commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
arpadboda commented on a change in pull request #470: MINIFICPP-706 -
RawSiteToSite: remove code duplication
URL: https://github.com/apache/nifi-minifi-cpp/pull/470#discussion_r249233347
##
File path: libminifi/include/sitetosite/SiteToSiteClient.h
##
@@ -221,12 +221,8 @@ class SiteToSiteClient : public core::Connectable {
// deleteTransaction
virtual void deleteTransaction(std::string transactionID);
- virtual void tearDown();
-
- // write Request Type
- virtual int writeRequestType(RequestType type);
- // read Request Type
- virtual int readRequestType(RequestType );
+ virtual void tearDown() = 0;
Review comment:
This exists in both http and raw, with different implementation, so I guess
it's fine here as pure virtual.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] arpadboda commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
arpadboda commented on a change in pull request #470: MINIFICPP-706 -
RawSiteToSite: remove code duplication
URL: https://github.com/apache/nifi-minifi-cpp/pull/470#discussion_r247095016
##
File path: libminifi/src/sitetosite/RawSocketProtocol.cpp
##
@@ -394,98 +394,12 @@ bool
RawSiteToSiteClient::getPeerList(std::vector ) {
return false;
}
}
-
-int RawSiteToSiteClient::writeRequestType(RequestType type) {
Review comment:
Done, it seems github failed to handle the diff visualisaiton properly.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] arpadboda commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
arpadboda commented on a change in pull request #470: MINIFICPP-706 -
RawSiteToSite: remove code duplication
URL: https://github.com/apache/nifi-minifi-cpp/pull/470#discussion_r247094612
##
File path: libminifi/include/sitetosite/RawSocketProtocol.h
##
@@ -143,21 +140,21 @@ class RawSiteToSiteClient : public
sitetosite::SiteToSiteClient {
// tearDown
virtual void tearDown();
// write Request Type
- virtual int writeRequestType(RequestType type);
+ virtual int writeRequestType(RequestType type) {
+return SiteToSiteClient::writeRequestType(type);
Review comment:
Okay, good shout, this helped to remove even more duplications. :]
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] arpadboda commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
arpadboda commented on a change in pull request #470: MINIFICPP-706 -
RawSiteToSite: remove code duplication
URL: https://github.com/apache/nifi-minifi-cpp/pull/470#discussion_r247094612
##
File path: libminifi/include/sitetosite/RawSocketProtocol.h
##
@@ -143,21 +140,21 @@ class RawSiteToSiteClient : public
sitetosite::SiteToSiteClient {
// tearDown
virtual void tearDown();
// write Request Type
- virtual int writeRequestType(RequestType type);
+ virtual int writeRequestType(RequestType type) {
+return SiteToSiteClient::writeRequestType(type);
Review comment:
Thanks, good shout, this helped to remove even more duplications. :]
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
