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

Option to mount a new local storage disabled #27590

Merged
merged 2 commits into from
Apr 10, 2017
Merged

Option to mount a new local storage disabled #27590

merged 2 commits into from
Apr 10, 2017

Conversation

noveens
Copy link
Contributor

@noveens noveens commented Apr 6, 2017

Fixes #26653

Description

Option to mount a new local storage removed unless files_external_allow_create_new_local variable specifically set to true in config.php

Related Issue

#26653

How Has This Been Tested?

Included the files_external_allow_create_new_local option and set to true and false, works as desired

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.

@PVince81

@noveens
Copy link
Contributor Author

noveens commented Apr 6, 2017

To discuss :

  • The colored status indicator for already existing local mounts at the http://localhost/core/index.php/settings/admin?sectionid=storage page always turns to RED even if being displayed at the home page.
  • The log fills up like hell.

@PVince81 ?

@PVince81
Copy link
Contributor

PVince81 commented Apr 7, 2017

Are these two checkbox items related to your change or do you observe these also on master ?
What log entries ?

@noveens
Copy link
Contributor Author

noveens commented Apr 7, 2017

@PVince81 ,
The two checkbox items are related to my change,
And I needed to discuss with you how to deal with these changes.

The logs are :

{"reqId":"E1qVs3bsCjnSHqDjNdaz","level":3,"time":"2017-04-06T23:16:10+00:00","remoteAddr":"127.0.0.1","user":"--","app":"PHP","method":"GET","url":"\/core\/cron.php","message":"Undefined index: size at \/var\/www\/core\/lib\/private\/Files\/Cache\/Scanner.php#419"}
{"reqId":"E1qVs3bsCjnSHqDjNdaz","level":3,"time":"2017-04-06T23:16:10+00:00","remoteAddr":"127.0.0.1","user":"--","app":"PHP","method":"GET","url":"\/core\/cron.php","message":"Undefined index: size at \/var\/www\/core\/lib\/private\/Files\/Cache\/Scanner.php#416"}
{"reqId":"E1qVs3bsCjnSHqDjNdaz","level":3,"time":"2017-04-06T23:16:10+00:00","remoteAddr":"127.0.0.1","user":"--","app":"PHP","method":"GET","url":"\/core\/cron.php","message":"Undefined index: size at \/var\/www\/core\/lib\/private\/Files\/Cache\/Scanner.php#419"}
{"reqId":"E1qVs3bsCjnSHqDjNdaz","level":3,"time":"2017-04-06T23:16:10+00:00","remoteAddr":"127.0.0.1","user":"--","app":"PHP","method":"GET","url":"\/core\/cron.php","message":"Undefined index: size at \/var\/www\/core\/lib\/private\/Files\/Cache\/Scanner.php#416"}
{"reqId":"E1qVs3bsCjnSHqDjNdaz","level":3,"time":"2017-04-06T23:16:10+00:00","remoteAddr":"127.0.0.1","user":"--","app":"PHP","method":"GET","url":"\/core\/cron.php","message":"Undefined index: size at \/var\/www\/core\/lib\/private\/Files\/Cache\/Scanner.php#419"}
{"reqId":"E1qVs3bsCjnSHqDjNdaz","level":3,"time":"2017-04-06T23:16:10+00:00","remoteAddr":"127.0.0.1","user":"--","app":"PHP","method":"GET","url":"\/core\/cron.php","message":"Undefined index: size at \/var\/www\/core\/lib\/private\/Files\/Cache\/Scanner.php#416"}
{"reqId":"E1qVs3bsCjnSHqDjNdaz","level":3,"time":"2017-04-06T23:16:10+00:00","remoteAddr":"127.0.0.1","user":"--","app":"PHP","method":"GET","url":"\/core\/cron.php","message":"Undefined index: size at \/var\/www\/core\/lib\/private\/Files\/Cache\/Scanner.php#419"}
{"reqId":"E1qVs3bsCjnSHqDjNdaz","level":3,"time":"2017-04-06T23:16:10+00:00","remoteAddr":"127.0.0.1","user":"--","app":"PHP","method":"GET","url":"\/core\/cron.php","message":"Undefined index: size at \/var\/www\/core\/lib\/private\/Files\/Cache\/Scanner.php#416"}
{"reqId":"E1qVs3bsCjnSHqDjNdaz","level":3,"time":"2017-04-06T23:16:10+00:00","remoteAddr":"127.0.0.1","user":"--","app":"PHP","method":"GET","url":"\/core\/cron.php","message":"Undefined index: size at \/var\/www\/core\/lib\/private\/Files\/Cache\/Scanner.php#419"}
{"reqId":"E1qVs3bsCjnSHqDjNdaz","level":3,"time":"2017-04-06T23:16:10+00:00","remoteAddr":"127.0.0.1","user":"--","app":"PHP","method":"GET","url":"\/core\/cron.php","message":"Undefined index: size at \/var\/www\/core\/lib\/private\/Files\/Cache\/Scanner.php#416"}
{"reqId":"E1qVs3bsCjnSHqDjNdaz","level":3,"time":"2017-04-06T23:16:10+00:00","remoteAddr":"127.0.0.1","user":"--","app":"PHP","method":"GET","url":"\/core\/cron.php","message":"Undefined index: size at \/var\/www\/core\/lib\/private\/Files\/Cache\/Scanner.php#419"}
{"reqId":"E1qVs3bsCjnSHqDjNdaz","level":3,"time":"2017-04-06T23:16:10+00:00","remoteAddr":"127.0.0.1","user":"--","app":"PHP","method":"GET","url":"\/core\/cron.php","message":"Undefined index: size at \/var\/www\/core\/lib\/private\/Files\/Cache\/Scanner.php#416"}
{"reqId":"E1qVs3bsCjnSHqDjNdaz","level":3,"time":"2017-04-06T23:16:10+00:00","remoteAddr":"127.0.0.1","user":"--","app":"PHP","method":"GET","url":"\/core\/cron.php","message":"Undefined index: size at \/var\/www\/core\/lib\/private\/Files\/Cache\/Scanner.php#419"}
{"reqId":"E1qVs3bsCjnSHqDjNdaz","level":3,"time":"2017-04-06T23:16:10+00:00","remoteAddr":"127.0.0.1","user":"--","app":"PHP","method":"GET","url":"\/core\/cron.php","message":"Undefined index: size at \/var\/www\/core\/lib\/private\/Files\/Cache\/Scanner.php#416"}

@PVince81
Copy link
Contributor

PVince81 commented Apr 7, 2017

Do you observe these log entries also when running on master ? I don't see how these relate to your changes. It could be another important bug you just discovered.

@noveens
Copy link
Contributor Author

noveens commented Apr 7, 2017

@PVince81 ,
These used to appear on the latest master after the changes of #26690
When you set the flag files_external_allow_local to false in config.php, these logs still show up.

And what about the red colored indicator?

Http::STATUS_UNPROCESSABLE_ENTITY
);
}
$canCreateNewLocalStorage = \OC::$server->getConfig()->getSystemValue('files_external_allow_create_new_local', false);
Copy link
Contributor

Choose a reason for hiding this comment

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

move this to function create.

here you are also preventing updates to happen, so existing local storages cannot be updated/saved (red indicator)

@PVince81
Copy link
Contributor

PVince81 commented Apr 7, 2017

  • BUG: need to find a way to remove "Local" from the selection list altogether, maybe filter it out when populating the list

On my local instance I didn't see any log entries related to scanner, so it might be specific to your env.

@noveens
Copy link
Contributor Author

noveens commented Apr 8, 2017

@PVince81 ,
Can you have a look?

@noveens
Copy link
Contributor Author

noveens commented Apr 8, 2017

@PVince81 ,

Ready for a merge?


$this->updateStorageStatus($newStorage);
$this->updateStorageStatus($newStorage);
Copy link
Contributor

Choose a reason for hiding this comment

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

use tabs, not spaces

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, will do.

Copy link
Contributor Author

@noveens noveens Apr 10, 2017

Choose a reason for hiding this comment

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

@PVince81 ,
These are tabs not spaces, I guess GitHub is just trying to show the changed indentation?

@PVince81
Copy link
Contributor

Tested, works 👍

@noveens please fix the tabs then we can merge this

@PVince81 PVince81 added this to the 10.0 milestone Apr 10, 2017
@PVince81 PVince81 merged commit 70dd098 into master Apr 10, 2017
@PVince81 PVince81 deleted the 26653 branch April 10, 2017 07:51
}
$canCreateNewLocalStorage = \OC::$server->getConfig()->getSystemValue('files_external_allow_create_new_local', false);

if ($canCreateNewLocalStorage === true) {
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately, this also triggers for smb. As it seems it prevents adding any external storage. Also it returns null and no meaningful message is shown in the web ui. Will add an issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

see #27621

@PVince81
Copy link
Contributor

Regression found, fixed here: #27621

@lock
Copy link

lock bot commented Aug 3, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make it possible to disable local storage mount type
3 participants