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

Lowering the priority from error to debug #27826

Merged
merged 1 commit into from May 10, 2017

Conversation

Projects
None yet
2 participants
@sharidas
Member

sharidas commented May 8, 2017

Lowering the priority of the messages from
error to debug.

Signed-off-by: Sujith H sharidasan@owncloud.com

Description

This change reduces the priority of the messages of trashbin from error to debug.

Related Issue

The issue which this change tries to solve is owncloud/enterprise#1557

Motivation and Context

It would be annoying to see repeated messages for the admin. Hence reducing the priority.

How Has This Been Tested?

  • Tested on 8.2.11. The error log message comes in the log file without this change. After applying this change the logs disappeared because of priority lowering. When the loglevel is reduced to debugging in the config.php, "trash bin database inconsistent!" log appears again.
  • Tested on master branch.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
@@ -71,7 +71,7 @@
if ( !OCA\Files_Trashbin\Trashbin::restore($path, $filename, $timestamp) ) {
$error[] = $filename;
\OCP\Util::writeLog('trashbin', 'can\'t restore ' . $filename, \OCP\Util::ERROR);
\OCP\Util::writeLog('trashbin', 'can\'t restore ' . $filename, \OCP\Util::DEBUG);

This comment has been minimized.

@PVince81

PVince81 May 9, 2017

Member

now thinking of it, you said that the restoration does work, so let's change the message to "original location file $filename not found in database, restoring into user's root instead"

@PVince81

PVince81 May 9, 2017

Member

now thinking of it, you said that the restoration does work, so let's change the message to "original location file $filename not found in database, restoring into user's root instead"

This comment has been minimized.

@sharidas

sharidas May 9, 2017

Member

Yes the restoration work. But the message 'can't restore $filename' doesn't appear to me, because https://github.com/owncloud/core/blob/master/apps/files_trashbin/lib/Trashbin.php#L444 is true and hence returns true. Which causes if ( !OCA\Files_Trashbin\Trashbin::restore($path, $filename, $timestamp) ) fails and goes to the else part. Since restore works, do we need to change the message?

@sharidas

sharidas May 9, 2017

Member

Yes the restoration work. But the message 'can't restore $filename' doesn't appear to me, because https://github.com/owncloud/core/blob/master/apps/files_trashbin/lib/Trashbin.php#L444 is true and hence returns true. Which causes if ( !OCA\Files_Trashbin\Trashbin::restore($path, $filename, $timestamp) ) fails and goes to the else part. Since restore works, do we need to change the message?

This comment has been minimized.

@PVince81

PVince81 May 9, 2017

Member

keep it then

@PVince81

PVince81 May 9, 2017

Member

keep it then

@sharidas sharidas referenced this pull request May 9, 2017

Merged

[Stable9.1] Lower the priority from error to debug #27836

1 of 9 tasks complete
Show outdated Hide outdated apps/files_trashbin/lib/Trashbin.php
@@ -415,7 +415,7 @@ public static function restore($file, $filename, $timestamp) {
if ($timestamp) {
$location = self::getLocation($user, $filename, $timestamp);
if ($location === false) {
\OCP\Util::writeLog('files_trashbin', 'trash bin database inconsistent!', \OCP\Util::ERROR);
\OCP\Util::writeLog('files_trashbin', 'trash bin database inconsistent!', \OCP\Util::DEBUG);

This comment has been minimized.

@sharidas

sharidas May 9, 2017

Member

How about having "Original location of file $filename not found in database, hence restoring into user's root instead" here?

@sharidas

sharidas May 9, 2017

Member

How about having "Original location of file $filename not found in database, hence restoring into user's root instead" here?

This comment has been minimized.

@PVince81

PVince81 May 9, 2017

Member

sounds good

@PVince81

PVince81 May 9, 2017

Member

sounds good

@PVince81

👍

@PVince81 PVince81 added this to the 10.0.1 milestone May 9, 2017

Lowering the priority from error to debug
Lowering the priority of the messages from
error to debug. A minor modification of the
message to make it look more meaningful.

Signed-off-by: Sujith H <sharidasan@owncloud.com>

@PVince81 PVince81 merged commit 03caa51 into master May 10, 2017

4 checks passed

Scrutinizer No new issues
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@PVince81 PVince81 deleted the trashbin-task-1557 branch May 10, 2017

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 May 10, 2017

Member

stable9.1: #27836

Member

PVince81 commented May 10, 2017

stable9.1: #27836

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment