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

Improve logging of archive errors due to possible permission problems #908

Merged
merged 3 commits into from
Mar 17, 2020

Conversation

racke
Copy link
Contributor

@racke racke commented Mar 7, 2020

No description provided.

@racke racke requested a review from ikedas March 7, 2020 09:52
$ERRNO;
$log->syslog('err', 'Cannot open dir %s: %s', $self->{arc_directory},
$ERRNO);
return undef;
Copy link
Member

Choose a reason for hiding this comment

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

I think this is the case when "Sympa has no choice but to die" (cf. #90 ). It's fatal that $self->{arc_directory} is set, i.e. select_archive() has succeeded, while it cannot be opened.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can see your point ... but it leaves the web interface user with the die output which should not happen other than real corner cases. What about checking whether the directory can be read in the beginning of do_arc?

Copy link
Member

Choose a reason for hiding this comment

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

Checking readability may not be the solution, I think. But this topic seems beyond the scope of this pr, and I’ll stop commenting for the present.

@ikedas ikedas added this to the 6.2.56 milestone Mar 17, 2020
@ikedas ikedas merged commit 18e0355 into sympa-community:sympa-6.2 Mar 17, 2020
@racke racke deleted the pr/arc-improve-logging branch March 17, 2020 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants