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

Upgrade fails if a file in usageEventLogs is empty #10245

Open
SydneyUni-Jim opened this issue Jul 23, 2024 · 3 comments
Open

Upgrade fails if a file in usageEventLogs is empty #10245

SydneyUni-Jim opened this issue Jul 23, 2024 · 3 comments

Comments

@SydneyUni-Jim
Copy link

Describe the bug
Migration task I8508_ConvertCurrentLogFile fails.

2024-07-24 09:39:51 [migration: PKP\migration\upgrade\v3_4_0\I8508_ConvertCurrentLogFile]
Error: File /var/www/files/usageStats/usageEventLogs/usage_events_20240724.log could not be successfully converted.

To Reproduce
Steps to reproduce the behavior:

  1. Start with an OJS 3.3 instance.
  2. Stop the instance.
  3. Process all the logs.
  4. touch files/usageStats/usageEventLogs/usage_events_YYYYMMDD.log replacing YYYYMMDD with today's date
  5. Start an upgrade to OJS 3.4.

What application are you using?
OJS 3.4.0.6

@SydneyUni-Jim
Copy link
Author

convert in PKP\cliTool\traits\ConvertLogFile initially set $isSuccessful to false and only sets it to true once it has successfully converted an entry.

$isSuccessful = false;
while (!feof($fhandle)) {

$isSuccessful = true;
}

This logic fails if:

  • the file's size is zero and feof(…) immediately returns false; or
  • the while loop never gets to its end because all of the lines trigger one of the continue statements

@SydneyUni-Jim
Copy link
Author

Relate to handling of $isSuccessful, after the first successful entry sets $isSuccessful to true it is never set back to false. This means the I8508_ConvertCurrentLogFile migration passes if and whenever at least one log entry is successfully converted.

The logics like this don't set $isSuccessful back to false.

if (!$this->isLogEntryValid($entryData)) {
fwrite(STDERR, "Invalid log entry at line {$lineNumber}." . PHP_EOL);
continue;
}

@bozana
Copy link
Collaborator

bozana commented Jul 29, 2024

Hi @SydneyUni-Jim,
Yes, I think we need to somehow consider -- at least report it correctly -- if the file is empty (also in case only continue statements are called).
But, else, if any other problem occurs (like invalid line) I think it is OK: we convert the lines that can be converted and report the existing issues, so that user can act as needed.

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

No branches or pull requests

2 participants