pivotal-jbarrett commented on a change in pull request #706:
URL: https://github.com/apache/geode-native/pull/706#discussion_r538883695
##########
File path: cppcache/integration/framework/Cluster.cpp
##########
@@ -110,6 +109,12 @@ void Locator::start() {
.withPreferIPv6(cluster_.getUseIPv6())
.withJmxManagerStart(true);
+ if(cluster_.getLogLevel().empty()) {
Review comment:
Why pull this out of the `.withLogLevel` model? Why not have cluster
default non-empty value and then always pass cluster value through to
`gfsh.withLogLevel()`?
##########
File path: cppcache/integration/test/TransactionsTest.cpp
##########
@@ -36,6 +40,30 @@ using apache::geode::client::Pool;
using apache::geode::client::Region;
using apache::geode::client::RegionShortcut;
+std::string getServerLogName() {
Review comment:
Using logs to validate tests is really dangerous. Log messages change
and aren't part of API. Is there a more concrete way to test this? I would
think a unit tests that asserts these two methods set the correct 'no tx' value
of `-1` should be sufficient.
##########
File path: cppcache/src/TcrMessage.cpp
##########
@@ -2096,14 +2096,9 @@ TcrMessagePing::TcrMessagePing(DataOutput* dataOutput,
bool decodeAll) {
m_request->writeInt(static_cast<int32_t>(
0)); // 17 is fixed message len ... PING only has a header.
m_request->writeInt(static_cast<int32_t>(0)); // Number of parts.
- // int32_t txId = TcrMessage::m_transactionId++;
- // Setting the txId to 0 for all ping message as it is not being used on the
- // SERVER side or the
- // client side.
- m_request->writeInt(static_cast<int32_t>(0));
+ m_request->writeInt(m_txId);
Review comment:
A ping is not a transactional item. It should send `-1`.
##########
File path: cppcache/src/TcrMessage.cpp
##########
@@ -2114,8 +2109,7 @@
TcrMessageCloseConnection::TcrMessageCloseConnection(DataOutput* dataOutput,
m_request->writeInt(m_msgType);
m_request->writeInt(static_cast<int32_t>(6));
m_request->writeInt(static_cast<int32_t>(1)); // Number of parts.
- // int32_t txId = TcrMessage::m_transactionId++;
- m_request->writeInt(static_cast<int32_t>(0));
+ m_request->writeInt(m_txId);
Review comment:
Not transactional, should send `-1`.
----------------------------------------------------------------
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]
