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

first implementation of sql_auditing #1367

Closed
wants to merge 4 commits into from

Conversation

juandent
Copy link
Contributor

No description provided.

@juandent
Copy link
Contributor Author

I don't know what went wrong with formatting

Copy link
Owner

@fnc12 fnc12 left a comment

Choose a reason for hiding this comment

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

  1. the feature is not production ready
  2. formatting is broken, it has to be fixed

}
}
inline static sql_auditor& auditor() {
static sql_auditor auditor{"sql_auditor.txt"};
Copy link
Owner

Choose a reason for hiding this comment

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

what if somebody wants to store logs not in sql_auditor.txt but somewhere else?

std::tm local_time = *std::localtime(&now_time);

// Print the local time in a human-readable format
auditor().log_file << "@: " << std::put_time(&local_time, "%Y-%m-%d %H:%M:%S") // Custom format
Copy link
Owner

Choose a reason for hiding this comment

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

what if somebody wants to use different date format? how devs who already has working log system would integrate this logic into their systems?

}
}
inline static sql_auditor& auditor() {
static sql_auditor auditor{"sql_auditor.txt"};
Copy link
Owner

Choose a reason for hiding this comment

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

what if the process which runs this code doesn't have file permissions to sql_auditor.txt especially or to write in the folder where it is located at all? it will start throwingstd::system_error{sqlite_orm::orm_error_code::failure_to_init_logfile} even though it worked well before this PR

@@ -61,6 +62,7 @@ namespace sqlite_orm {
if(sqlite3_prepare_v2(db, query.c_str(), -1, &stmt, nullptr) != SQLITE_OK) {
throw_translated_sqlite_error(db);
}
sql_auditor::log(query);
Copy link
Owner

Choose a reason for hiding this comment

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

logs are enabled always even though devs don't need them?

@juandent
Copy link
Contributor Author

juandent commented Dec 17, 2024 via email

@fnc12
Copy link
Owner

fnc12 commented Dec 17, 2024

@juandent probably clang-format version is incorrect (use 17) but maybe something else. I can't say cause it happened on your machine and you have to fix it no matter the reason

@fnc12
Copy link
Owner

fnc12 commented Dec 19, 2024

@juandent you need to fix formatting

@juandent
Copy link
Contributor Author

@juandent you need to fix formatting

the error seems to be in .clang-format file
can you send me this file?

@fnc12
Copy link
Owner

fnc12 commented Dec 20, 2024

@juandent the error is not in clang-format file cause all other PR creator have it working. The file is located in repo root

@juandent juandent closed this by deleting the head repository Dec 20, 2024
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.

2 participants