[GitHub] phrocker commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
phrocker 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_r252668613
##
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:
Yep, I usually do full scope when anything touches core code, no matter how
trivial the changes appear: docker tests, site to site secure, unsecure,
transfers -- in this case I'll do raw and http just to be safe ( in minifi note
C agent )...maybe more if I think it's prudent -- once all that is done I'll
take another look at the PR and approve for merge in 0.7.0. Thanks.
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] phrocker commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
phrocker 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_r252668613
##
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:
Yep, I usually do full scope: docker tests, site to site secure, unsecure,
transfers -- in this case I'll do raw and http just to be safe ( in minifi note
C agent ). -- once all that is done I'll take another look at the PR and
approve for merge in 0.7.0. Thanks.
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] phrocker commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
phrocker 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_r252643034
##
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:
I didn't see a response for this. I have some time in the next few days to
run tests. What have you been able to run? Thanks!
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] phrocker commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
phrocker 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_r250646367
##
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:
Then that reliance may not be valid any longer. I think some parent class
details changed, but I will have to further investigate the HTTP portion. I
want to fully understand this, instead of going off of memory. If it is a bug
that can be a follow on, but once I take a closer look at HTTP it may become
obvious. What the parent currently does, is an invalid reliance from the end of
HTTP, but since things changed in some of the accessory code it may never get
there, meaning we can make it pure virtual; however, I'll take a closer look at
it while running to validate that it's not a bug before going that route.
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] phrocker commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
phrocker 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_r250647228
##
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:
I'll also take another look at this PR closely and approve for merge in
0.7.0 if it doesn't change status quo. Thanks. By the way, what scope of
testing have you performed on this code via docker or in a deployed environment?
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] phrocker commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
phrocker 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_r250647228
##
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:
I'll also take another look at this PR closely and approve for merge in
0.7.0 if it doesn't change status quo. Thanks.
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] phrocker commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
phrocker 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_r250646367
##
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:
This isn't valid any longer. I think some parent class details changed, but
I will have to further investigate the HTTP portion. I want to fully understand
this. If it is a bug that may push when this should be merged, but once I take
a closer look at HTTP it may become obvious. What the parent currently does, is
an invalid reliance from the end of HTTP, but since things changed in some of
the accessory code it may never get there, meaning we can make it pure virtual;
however, I'll take a closer look at it while running to validate that it's not
a bug before going that route.
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] phrocker commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
phrocker 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_r250647228
##
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:
I'll also take another look at this PR closely and approve for merge in
0.7.0 if it doesn't change status quo for http ( which it doesn't look like it
does at a quick glance ).
By the way, what scope of testing have you performed on this code via docker
or in a deployed environment? I understand it's "moving code," but mistakes
happen. Don't want to duplicate that effort.
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] phrocker commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
phrocker 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_r250647228
##
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:
I'll also take another look at this PR closely and approve for merge in
0.7.0 if it doesn't change status quo. Thanks. By the way, what scope of
testing have you performed on this code via docker or in a deployed
environment? I understand it's "moving code," but mistakes happen. Don't want
to duplicate that effort.
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] phrocker commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
phrocker 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_r250646367
##
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:
Then that reliance may not be valid any longer. I think some parent class
details changed, but I will have to further investigate the HTTP portion. I
want to fully understand this, instead of going off of memory. If it is a bug
that may push when this should be merged, but once I take a closer look at HTTP
it may become obvious. What the parent currently does, is an invalid reliance
from the end of HTTP, but since things changed in some of the accessory code it
may never get there, meaning we can make it pure virtual; however, I'll take a
closer look at it while running to validate that it's not a bug before going
that route.
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] phrocker commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
phrocker 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_r250646367
##
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:
Then that reliance may not be valid any longer. I think some parent class
details changed, but I will have to further investigate the HTTP portion. I
want to fully understand this. If it is a bug that may push when this should be
merged, but once I take a closer look at HTTP it may become obvious. What the
parent currently does, is an invalid reliance from the end of HTTP, but since
things changed in some of the accessory code it may never get there, meaning we
can make it pure virtual; however, I'll take a closer look at it while running
to validate that it's not a bug before going that route.
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] phrocker commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
phrocker 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_r249923759
##
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:
This is again the case where the base class doesn't need to deal with the
the innards of reading bytes. Probably means they can be removed from the base
and made pure virtual since I believe they are overridden in http site to site
if I'm not mistaken.
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] phrocker commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
phrocker 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_r249102672
##
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:
There probably don't need to be any changes in this file, right? Seems that
there have been.
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] phrocker commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
phrocker 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_r249102304
##
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:
i don't think this is needed at all. is it used elsewhere? Teardown is
protocol specific of when and how it occurs.
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] phrocker commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
phrocker 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_r246921467
##
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:
I think the code being in SiteToSiteClient is vestigial. It probably
shouldn't be there. That also would reduce the risk of any testing. Just remove
it from the base class. The only other implementation ( and others I have in
mind ) wouldn't need this.
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] phrocker commented on a change in pull request #470: MINIFICPP-706 - RawSiteToSite: remove code duplication
phrocker 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_r246921592
##
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:
^ Same as comments above, This can stay and we can remove the duplicate in
the base.
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
