ijokarumawak commented on a change in pull request #3647: NIFI-6530 - HTTP
SiteToSite server returns 201 in case no data is ava…
URL: https://github.com/apache/nifi/pull/3647#discussion_r314555681
##########
File path:
nifi-commons/nifi-site-to-site-client/src/main/java/org/apache/nifi/remote/client/http/HttpClient.java
##########
@@ -168,6 +169,9 @@ public Transaction createTransaction(final
TransferDirection direction) throws H
try {
transactionUrl = apiClient.initiateTransaction(direction,
portId);
commSession.setUserDn(apiClient.getTrustedPeerDn());
+ } catch (final NoContentException e) {
+ logger.debug("Peer {} has no flowfiles to provide", peer);
+ throw e;
Review comment:
I totally agree with you on the necessity of separating the two cases. How
about changing SiteToSiteClient.createTransaction semantics as follows?
- AS IS
- HandshakeException, PortNotRunningException, UnknownPortException in
irregular case
- Returns a Transaction to use for sending or receiving data, or `null`
if all nodes are penalized.
- TO BE (by this PR)
- HandshakeException, PortNotRunningException, UnknownPortException in
irregular case
- Or (new) NoPeerAvailableException when all peers are penalized.
Because this is an error case. We need to change
[SocketClient](https://github.com/apache/nifi/blob/master/nifi-commons/nifi-site-to-site-client/src/main/java/org/apache/nifi/remote/client/socket/SocketClient.java#L129),
too.
- Returns a Transaction to use for sending or receiving data, or `null`
if communication was successful but no transaction is needed, ex. no data to
transfer.
This allows callers to handle each cases correctly, and the API looks more
intuitive. How do you think?
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
