[CELEBORN-1978][CIP-14] Add code style checking for cppClient

### What changes were proposed in this pull request?
This PR adds code style checking in github action for cppClient.

### Why are the changes needed?
To keep the cpp code style consistent.

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
By github action procedure.

Closes #3252 from HolyLow/issue/celeborn-1978-add-codestyle-check-to-cppclient.

Authored-by: HolyLow <jiaming.xie7@gmail.com>
Signed-off-by: mingji <fengmingxiao.fmx@alibaba-inc.com>
This commit is contained in:
HolyLow 2025-05-12 11:16:36 +08:00 committed by mingji
parent 6ceadd3b2a
commit 3896249b92
12 changed files with 57 additions and 39 deletions

View File

@ -28,9 +28,22 @@ on:
- branch-*
jobs:
celeborn_cpp_check_lint:
runs-on: ubuntu-22.04
container: holylow/celeborn-cpp-dev:0.3
steps:
- uses: actions/checkout@v4
with:
persist-credentials: 'false'
fetch-depth: 0
- name: Check Cpp Code Lint
working-directory: ./cpp
run: |
find ./ -iname '*.h' -o -iname '*.cpp' | \
xargs clang-format-15 -style=file:./.clang-format -n --Werror
celeborn_cpp_unit_test:
runs-on: ubuntu-22.04
container: holylow/celeborn-cpp-dev:0.2
container: holylow/celeborn-cpp-dev:0.3
steps:
- uses: actions/checkout@v4
with:
@ -46,7 +59,7 @@ jobs:
run: ctest
celeborn_cpp_integration_test:
runs-on: ubuntu-22.04
container: holylow/celeborn-cpp-dev:0.2
container: holylow/celeborn-cpp-dev:0.3
steps:
- uses: actions/checkout@v4
with:

View File

@ -21,7 +21,7 @@ docker run \
-w /celeborn \
-it --rm \
--name celeborn-cpp-dev-container \
holylow/celeborn-cpp-dev:0.2 \
holylow/celeborn-cpp-dev:0.3 \
/bin/bash
```

View File

@ -53,9 +53,8 @@ std::unique_ptr<CelebornInputStream> ShuffleClientImpl::readPartition(
std::vector<std::shared_ptr<const protocol::PartitionLocation>> locations;
if (!reducerFileGroupInfo.fileGroups.empty() &&
reducerFileGroupInfo.fileGroups.count(partitionId)) {
locations = std::move(
utils::toVector(
reducerFileGroupInfo.fileGroups.find(partitionId)->second));
locations = std::move(utils::toVector(
reducerFileGroupInfo.fileGroups.find(partitionId)->second));
}
return std::make_unique<CelebornInputStream>(
shuffleKey,

View File

@ -52,9 +52,8 @@ class ShuffleClientImpl : public ShuffleClient {
void setupLifecycleManagerRef(std::string& host, int port) override;
void setupLifecycleManagerRef(
std::shared_ptr<network::NettyRpcEndpointRef>& lifecycleManagerRef)
override;
void setupLifecycleManagerRef(std::shared_ptr<network::NettyRpcEndpointRef>&
lifecycleManagerRef) override;
std::unique_ptr<CelebornInputStream> readPartition(
int shuffleId,

View File

@ -29,11 +29,14 @@ std::string bool2String(bool value) {
return value ? "true" : "false";
}
#define STR_PROP(_key_, _val_) {std::string(_key_), std::string(_val_)}
#define STR_PROP(_key_, _val_) \
{ std::string(_key_), std::string(_val_) }
#define NUM_PROP(_key_, _val_) \
{std::string(_key_), folly::to<std::string>(_val_)}
#define BOOL_PROP(_key_, _val_) {std::string(_key_), bool2String(_val_)}
#define NONE_PROP(_key_) {std::string(_key_), folly::none}
{ std::string(_key_), folly::to<std::string>(_val_) }
#define BOOL_PROP(_key_, _val_) \
{ std::string(_key_), bool2String(_val_) }
#define NONE_PROP(_key_) \
{ std::string(_key_), folly::none }
enum class CapacityUnit {
BYTE,
@ -169,7 +172,8 @@ void CelebornConf::registerProperty(
}
Timeout CelebornConf::rpcLookupTimeout() const {
return utils::toTimeout(toDuration(optionalProperty(kRpcLookupTimeout).value()));
return utils::toTimeout(
toDuration(optionalProperty(kRpcLookupTimeout).value()));
}
Timeout CelebornConf::clientRpcGetReducerFileGroupRpcAskTimeout() const {
@ -183,7 +187,8 @@ Timeout CelebornConf::networkConnectTimeout() const {
}
Timeout CelebornConf::clientFetchTimeout() const {
return utils::toTimeout(toDuration(optionalProperty(kClientFetchTimeout).value()));
return utils::toTimeout(
toDuration(optionalProperty(kClientFetchTimeout).value()));
}
int CelebornConf::networkIoNumConnectionsPerPeer() const {

View File

@ -155,8 +155,7 @@ TEST(BaseConfTest, registerToConfInitedWithMutableConfigFile) {
conf->initialize(filename);
// The annotated kv would not be recorded.
EXPECT_THROW(
conf->requiredProperty(annotatedKey), CelebornUserError);
EXPECT_THROW(conf->requiredProperty(annotatedKey), CelebornUserError);
// Test init.
testInitedConf(conf.get(), true, key0, val0);
@ -192,8 +191,7 @@ TEST(BaseConfTest, registerToConfInitedWithImmutableConfigFile) {
conf->initialize(filename);
// The annotated kv would not be recorded.
EXPECT_THROW(
conf->requiredProperty(annotatedKey), CelebornUserError);
EXPECT_THROW(conf->requiredProperty(annotatedKey), CelebornUserError);
// Test init.
testInitedConf(conf.get(), false, key0, val0);

View File

@ -18,7 +18,7 @@
#include "celeborn/memory/ByteBuffer.h"
namespace celeborn {
namespace memory{
namespace memory {
std::unique_ptr<WriteOnlyByteBuffer> ByteBuffer::createWriteOnly(
size_t initialCapacity,
bool isBigEndian) {

View File

@ -182,14 +182,16 @@ TEST(TransportClientTest, fetchChunkAsyncSuccess) {
protocol::StreamChunkSlice onSuccessStreamChunkSlice;
std::unique_ptr<memory::ReadOnlyByteBuffer> onSuccessBuffer;
FetchChunkSuccessCallback onSuccess =
[&](protocol::StreamChunkSlice slice, std::unique_ptr<memory::ReadOnlyByteBuffer> buffer) {
[&](protocol::StreamChunkSlice slice,
std::unique_ptr<memory::ReadOnlyByteBuffer> buffer) {
onSuccessStreamChunkSlice = slice;
onSuccessBuffer = std::move(buffer);
};
protocol::StreamChunkSlice onFailureStreamChunkSlice;
std::unique_ptr<std::exception> onFailureException;
FetchChunkFailureCallback onFailure =
[&](protocol::StreamChunkSlice slice, std::unique_ptr<std::exception> exception) {
[&](protocol::StreamChunkSlice slice,
std::unique_ptr<std::exception> exception) {
onFailureStreamChunkSlice = slice;
onFailureException = std::move(exception);
};
@ -229,14 +231,16 @@ TEST(TransportClientTest, fetchChunkAsyncFailure) {
protocol::StreamChunkSlice onSuccessStreamChunkSlice;
std::unique_ptr<memory::ReadOnlyByteBuffer> onSuccessBuffer;
FetchChunkSuccessCallback onSuccess =
[&](protocol::StreamChunkSlice slice, std::unique_ptr<memory::ReadOnlyByteBuffer> buffer) {
[&](protocol::StreamChunkSlice slice,
std::unique_ptr<memory::ReadOnlyByteBuffer> buffer) {
onSuccessStreamChunkSlice = slice;
onSuccessBuffer = std::move(buffer);
};
protocol::StreamChunkSlice onFailureStreamChunkSlice;
std::unique_ptr<std::exception> onFailureException;
FetchChunkFailureCallback onFailure =
[&](protocol::StreamChunkSlice slice, std::unique_ptr<std::exception> exception) {
[&](protocol::StreamChunkSlice slice,
std::unique_ptr<std::exception> exception) {
onFailureStreamChunkSlice = slice;
onFailureException = std::move(exception);
};

View File

@ -53,7 +53,6 @@ std::unique_ptr<const PartitionLocation> PartitionLocation::fromPb(
return std::move(result);
}
std::unique_ptr<PartitionLocation> PartitionLocation::fromPackedPb(
const PbPackedPartitionLocations& pb,
int idx) {

View File

@ -62,7 +62,7 @@ inline Timeout toTimeout(Duration duration) {
/// parse string like "Any-Host-Str:Port#1:Port#2:...:Port#num", split into
/// {"Any-Host-Str", "Port#1", "Port#2", ..., "Port#num"}. Note that the
/// "Any-Host_Str" might contain ':' in IPV6 address.
/// "Any-Host-Str" might contain ':' in IPV6 address.
std::vector<std::string_view> parseColonSeparatedHostPorts(
const std::string_view& s,
int num);

View File

@ -109,12 +109,11 @@ void testExceptionTraceCollectionControl(bool userException, bool enabled) {
false);
}
} catch (CelebornException& e) {
SCOPED_TRACE(
fmt::format(
"enabled: {}, user flag: {}, sys flag: {}",
enabled,
FLAGS_celeborn_exception_user_stacktrace_enabled,
FLAGS_celeborn_exception_system_stacktrace_enabled));
SCOPED_TRACE(fmt::format(
"enabled: {}, user flag: {}, sys flag: {}",
enabled,
FLAGS_celeborn_exception_user_stacktrace_enabled,
FLAGS_celeborn_exception_system_stacktrace_enabled));
ASSERT_EQ(
userException, e.exceptionType() == CelebornException::Type::kUser);
ASSERT_EQ(enabled, e.stackTrace() != nullptr);
@ -175,13 +174,12 @@ void testExceptionTraceCollectionRateControl(
false);
}
} catch (CelebornException& e) {
SCOPED_TRACE(
fmt::format(
"userException: {}, hasRateLimit: {}, user limit: {}ms, sys limit: {}ms",
userException,
hasRateLimit,
FLAGS_celeborn_exception_user_stacktrace_rate_limit_ms,
FLAGS_celeborn_exception_system_stacktrace_rate_limit_ms));
SCOPED_TRACE(fmt::format(
"userException: {}, hasRateLimit: {}, user limit: {}ms, sys limit: {}ms",
userException,
hasRateLimit,
FLAGS_celeborn_exception_user_stacktrace_rate_limit_ms,
FLAGS_celeborn_exception_system_stacktrace_rate_limit_ms));
ASSERT_EQ(
userException, e.exceptionType() == CelebornException::Type::kUser);
ASSERT_EQ(!hasRateLimit || ((iter % 2) == 0), e.stackTrace() != nullptr);

View File

@ -106,6 +106,9 @@ function install_build_prerequisites {
if [[ ${USE_CLANG} != "false" ]]; then
install_clang15
fi
# Install clang-format-15 for lint checking.
${SUDO} apt install -y clang-format-15
}
# Install packages required for build.