Skip to content
This repository was archived by the owner on Jul 4, 2025. It is now read-only.

Conversation

@nguyenhoangthuan99
Copy link
Contributor

@nguyenhoangthuan99 nguyenhoangthuan99 commented Sep 12, 2024

Fix #1198
Summary changes:

  • Add end to end test for start server
  • Add end to end test for create log folder after server started

@nguyenhoangthuan99 nguyenhoangthuan99 changed the title Fix create log folder before logging Fix: create log folder before logging Sep 12, 2024
@nguyenhoangthuan99 nguyenhoangthuan99 marked this pull request as ready for review September 12, 2024 09:06
engine/main.cc Outdated
<< " Port: " << config.apiServerPort << "\n";

// Create logs/ folder and setup log to file
std::filesystem::create_directory(config.logFolderPath);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using std::filesystem::create_directories should be better. Seems like it handle create directory recursively.
https://en.cppreference.com/w/cpp/filesystem/create_directory

Also, I'm worry about concatenate path using / might not work on windows. Should using std::filesystem::path

Copy link
Contributor

@dan-menlo dan-menlo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nguyenhoangthuan99 Approved to unblock. However:

  • Please add tests!
  • This is the sort of thing that an intern will not realize, remove folder creation, and end up bricking the entire product

@nguyenhoangthuan99 nguyenhoangthuan99 merged commit 82f7823 into dev Sep 13, 2024
@nguyenhoangthuan99 nguyenhoangthuan99 deleted the fix/create-log-folder-if-didn't-existed branch September 13, 2024 01:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: cortex-nightly unable to run on first install

5 participants