Skip to content
Snippets Groups Projects

F grpc f acm

Merged Timm Fitschen requested to merge f-grpc-f-acm into dev

Summary

Two things happen here. First, the Boost log headers are being removed from our own headers. This is in itself not an enhancement, but it will help in the future making boost a pure build dependency of our library. Currently, boost is a "peer dependency" and all code which uses our headers also have to include the boost headers.

Second, there was a bug in the server (fixed in caosdb-server!53 (merged)) and during fixing it, I noticed that there is also a bug in the cpplib which is only one missing line. As I was at it, I also moved the file_descriptor struct into it's own source.

Focus

  • Look how all occurrences of "boost.log" disappear from the header files and move to the source files instead.
  • The missing line is annotated explicitly.

Test Environment

Manual testing is not necessary. See corresponding caosdb-cppinttest!19 (merged)

Check List for the Author

  • All automated tests pass
  • Reference related Issues
  • Up-to-date CHANGELOG.md
  • Annotations in code (Gitlab comments)
    • Intent of new code
    • Problems with old code
    • Why this implementation?

Check List for the Reviewer

  • I understand the intent of this MR
  • All automated tests pass
  • Up-to-date CHANGELOG.md
  • The test environment setup works and the intended behavior is reproducible in the test environment
  • In-code documentation and comments are up-to-date.
  • Check: Are there specifications? Are they satisfied?

For further good practices have a look at our review guidelines.

Edited by Timm Fitschen

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
109 103 {Role::PROPERTY, "PROPERTY"},
110 104 {Role::FILE, "FILE"}};
111 105
112 struct FileDescriptor {
  • Timm Fitschen
  • 42 43 ${CMAKE_CURRENT_SOURCE_DIR}/caosdb/utility.h
    43 44 ${CMAKE_CURRENT_SOURCE_DIR}/caosdb/value.h
    44 45 ${CMAKE_CURRENT_SOURCE_DIR}/caosdb/file_transmission/register_file_upload_handler.h
    45 # TODO this file is still missing
  • Timm Fitschen
  • 154 148 }
    155 149
    156 150 void UploadRequestHandler::handleSendingFileState() {
    157 const uint64_t DefaultChunkSize = 4 * 1024; // 4K
    151 const auto DefaultChunkSize = static_cast<uint64_t>(4 * 1024); // 4K
  • Timm Fitschen
  • 78 73 return this->certificate_provider;
    79 74 }
    80 75
    81 PemCertificateProvider::PemCertificateProvider(const std::string &certificate_provider) {
    82 this->certificate_provider = certificate_provider;
    83 }
    76 PemCertificateProvider::PemCertificateProvider(std::string certificate_provider)
    77 : certificate_provider(std::move(certificate_provider)) {}
  • Timm Fitschen
  • 81 PemCertificateProvider::PemCertificateProvider(const std::string &certificate_provider) {
    82 this->certificate_provider = certificate_provider;
    83 }
    76 PemCertificateProvider::PemCertificateProvider(std::string certificate_provider)
    77 : certificate_provider(std::move(certificate_provider)) {}
    84 78
    85 79 auto PemCertificateProvider::GetCertificatePem() const -> std::string {
    86 80 return this->certificate_provider;
    87 81 }
    88 82
    89 ConnectionConfiguration::ConnectionConfiguration(const std::string &host, int port) {
    90 this->host = host;
    91 this->port = port;
    92 }
    83 ConnectionConfiguration::ConnectionConfiguration(std::string host, int port)
    84 : host(std::move(host)), port(port) {}
  • Timm Fitschen
  • 399 391 auto default_connection = connections.at("default");
    400 392 if (default_connection.is_object()) {
    401 393 // the name is actually "default"
    402 return std::string("default");
    394 return {"default"};
    403 395 } else {
    404 396 assert(default_connection.is_string());
    405 397 auto default_connection_name = default_connection.as_string();
    406 398 // return the string value of connections.default
    407 return std::string(default_connection_name.c_str());
    399 return {default_connection_name.c_str()};
  • Timm Fitschen
  • 509 501 }
    510 502
    511 503 if (configuration_file_path != nullptr && this->json_configuration.is_object()) {
    512 CAOSDB_LOG_INFO(logger_name) << "Loaded configuration from " << *(configuration_file_path.get())
    504 CAOSDB_LOG_INFO(logger_name) << "Loaded configuration from " << *(configuration_file_path)
  • Timm Fitschen
  • 34 #include <vector> // for vector
    26 #include "caosdb/log_level.h" // for CAOSDB_LOG_...
    27 #include <cstdint> // for uint64_t
    28 #include <memory> // for shared_ptr
    29 #include <ostream> // for ostream
    30 #include <string> // for string
    31 #include <vector> // for vector
    35 32
    36 33 namespace caosdb::logging {
    37 34
    38 35 const std::string logger_name = "caosdb::logging";
    39 36
    40 typedef boost::log::sources::severity_channel_logger_mt<int, std::string> boost_logger_class;
    41
    42 class logger {
    37 class LoggerOutputStream {
  • Timm Fitschen
  • 112 114 friend auto initialize_logging(const LoggingConfiguration &logging_configuration) -> void;
    113 115
    114 116 protected:
    115 virtual auto Configure(boost::log::settings &settings) const -> void;
    117 virtual auto Configure(void *settings) const -> void;
  • Timm Fitschen
  • 61 122
    62 123 [[nodiscard]] auto SinkConfiguration::GetName() const -> const std::string & { return this->name; }
    63 124
    64 auto SinkConfiguration::Configure(boost::log::settings &settings) const -> void {
    65 CAOSDB_LOG_TRACE(logger_name) << "Enter SinkConfiguration::Configure(&settings)";
    125 auto SinkConfiguration::Configure(void *settings) const -> void {
    • hiding boost in the signature of these methods has this slight disadvantage, that the interface does not now about the class of the settings. However, this is now completely under the responsibility of the implementation in the compiled source file and thus we shouldn't be bothered.

    • Please register or sign in to reply
  • Timm Fitschen
  • 89 84
    90 85 Transaction::Transaction(std::shared_ptr<EntityTransactionService::Stub> entity_service,
    91 86 std::shared_ptr<FileTransmissionService::Stub> file_service)
    92 : request(Arena::CreateMessage<MultiTransactionRequest>(GetArena())),
    93 response(Arena::CreateMessage<MultiTransactionResponse>(GetArena())) {
    94 this->entity_service = std::move(entity_service);
    95 this->file_service = std::move(file_service);
    96 this->query_count = -1;
    97 }
    87 : entity_service(std::move(entity_service)), file_service(std::move(file_service)),
  • Timm Fitschen
  • 323 323 else()
    324 324 message(STATUS "clang-tidy: ${clang_tidy}")
    325 325 set(_CMAKE_CXX_CLANG_TIDY_CHECKS
    326 "--checks=*,-fuchsia-*,-llvmlibc-*,-readability-convert-member-functions-to-static,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-hicpp-no-array-decay,-llvm-else-after-return,-readability-else-after-return,-modernize-use-trailing-return-type,-bugprone-branch-clone")
    326 "--checks=*,-fuchsia-*,-llvmlibc-*,-readability-convert-member-functions-to-static,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-hicpp-no-array-decay,-llvm-else-after-return,-readability-else-after-return,-modernize-use-trailing-return-type,-bugprone-branch-clone,-altera-*")
  • Timm Fitschen added 1 commit

    added 1 commit

    Compare with previous version

  • Timm Fitschen added 1 commit

    added 1 commit

    Compare with previous version

  • Timm Fitschen added 1 commit

    added 1 commit

    Compare with previous version

  • 719 707 : static_cast<ProtoDataType *>(nullptr)) {
    720 708 properties.wrapped = this->wrapped->mutable_properties();
    721 709 parents.wrapped = this->wrapped->mutable_parents();
    710 file_descriptor.wrapped = this->wrapped->mutable_file_descriptor();
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading