Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CVS-154703, Implement createTempPath(std::string* local_path) for windows #2879

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

18582088138
Copy link
Collaborator

🛠 Summary

Implement createTempPath(std::string* local_path) for windows
JIRA/Issue: CVS-154703

🧪 Checklist

  • Unit tests added.
  • The documentation updated.
  • Change follows security best practices.
    ``

@rasapala rasapala changed the title CVS-154703, Implement createTempPath(std::string* local_path) for windows [WIP] CVS-154703, Implement createTempPath(std::string* local_path) for windows Dec 10, 2024
@rasapala
Copy link
Collaborator

rasapala commented Jan 2, 2025

Enable those unit tests for the change: "test/localfilesystem_test.cpp",

@18582088138 18582088138 changed the title [WIP] CVS-154703, Implement createTempPath(std::string* local_path) for windows CVS-154703, Implement createTempPath(std::string* local_path) for windows Jan 9, 2025

DWORD path_len = GetTempPathW(MAX_PATH, temp_path);
if (path_len == 0 || path_len > MAX_PATH) {
SPDLOG_LOGGER_ERROR(modelmanager_logger, "Failed to get temp path: {}", GetLastError());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add logging error message:
DWORD error = GetLastError();
std::string message = std::system_category().message(error);

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add message to log in all LastError cases.

#elif _WIN32
static StatusCode createTempPath(std::string* local_path) {
if (!local_path)
return StatusCode::FILESYSTEM_ERROR;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets add log here that target temp path is not set: SPDLOG_LOGGER_ERROR(modelmanager_logger, "Target path variable for createTempPAth not set. {}"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

following the commit and the modify the code

@rasapala rasapala requested a review from dkalinowski January 9, 2025 09:35
}
#elif _WIN32
static StatusCode createTempPath(std::string* local_path) {
if (!local_path)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The error path should be in { } for the local_path check

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants