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

Testing #260

Merged
merged 31 commits into from Mar 27, 2020
Merged

Testing #260

merged 31 commits into from Mar 27, 2020

Conversation

@maniackcrudelis
Copy link
Contributor

maniackcrudelis commented Mar 14, 2020

Problem

  • Version 15 is not supported anymore.
  • The onlyoffice builtin feature in 18 is very useful.
  • Install fails on buster because there's no php-mcrypt anymore

Solution

After a quick lookup on the internet, my understanding is that the stuff provided by php-mcrypt is old / deprecated since a while, and in fact Nextcloud doesnt need it anymore (it was added back to the jessie or maybe even squeeze era).

So let's remove it. (N.B. : I haven't tested anything though - but it should be okay to commit this for stretch as well)

PR Status

  • Code finished.
  • Tested with Package_check.
  • Fix or enhancement tested.
  • Upgrade from last version tested.
  • Can be reviewed and tested.

Validation


Minor decision

  • Upgrade previous version :
  • Code review : Maniack C
  • Approval (LGTM) : Maniack C
  • Approval (LGTM) : Kay0u
  • CI succeeded :
    Build Status
    When the PR is marked as ready to merge, you have to wait for 3 days before really merging it.
JimboJoe and others added 26 commits May 19, 2019
Co-Authored-By: Kayou <pierre.moltess@gmail.com>
Co-Authored-By: Kayou <pierre.moltess@gmail.com>
Upgrade to 18.0.1
@JimboJoe

This comment has been minimized.

Copy link
Contributor

JimboJoe commented Mar 14, 2020

Do we add #259 as well?

@maniackcrudelis

This comment has been minimized.

Copy link
Contributor Author

maniackcrudelis commented Mar 14, 2020

Missed that one
I resolve the conflict and I merge.
Could you update the first topic here meanwhile please ;)

@JimboJoe JimboJoe changed the title Upgrade to 18.0.1 Testing Mar 14, 2020
@maniackcrudelis

This comment has been minimized.

Copy link
Contributor Author

maniackcrudelis commented Mar 14, 2020

It was already done here c864e08

JimboJoe and others added 4 commits Mar 14, 2020
Upgrade to upstream version 18.0.2
@maniackcrudelis

This comment has been minimized.

Copy link
Contributor Author

maniackcrudelis commented Mar 17, 2020

@zamentur please update the forum and this pull request accordingly to the pull request you just merged.

Copy link

ahstro left a comment

Things seem to be running fine on fresh install. That's all the insight I can give though 😇

@zamentur

This comment has been minimized.

Copy link
Contributor

zamentur commented Mar 18, 2020

Hi,
I am running a nextcloud with this package exactly for 50 persons and tomorrow for 200 persons.

I had to tune some options like number of worker in nginx and number of simultaneous request (number of process) in php-fpm.

All things seems fine except the inherited permissions view feature which seems broken, could be a problem from upstream.

@lapineige

This comment has been minimized.

Copy link
Member

lapineige commented Mar 20, 2020

I can't upgrade: if I do it manually using yunohost app upgrade nextcloud -u https://github.com/YunoHost-Apps/nextcloud_ynh it takes no time and at the end I'm still using 15.0.14.
I'm on Debian Stretch. Is this available only for Buster ?

@kay0u

This comment has been minimized.

Copy link
Member

kay0u commented Mar 20, 2020

I can't upgrade: if I do it manually using yunohost app upgrade nextcloud -u https://github.com/YunoHost-Apps/nextcloud_ynh it takes no time and at the end I'm still using 15.0.14.

sudo yunohost app upgrade nextcloud -u https://github.com/YunoHost-Apps/nextcloud_ynh/tree/testing

with /tree/testing at the end

@lapineige

This comment has been minimized.

Copy link
Member

lapineige commented Mar 20, 2020

It is not supposed to be the default branch used now ? ^^

I did it, and… OHAY, it worked ! 🎉 Nextcloud 18.0.2 installed, from 15.0.14 ! 🎉 🎈

Great work everyone !

@lapineige

This comment has been minimized.

Copy link
Member

lapineige commented Mar 20, 2020

I have only one issue: in domain.tld/index.php/settings/admin/overview, I have this message:

La base de données a quelques index manquant. L'ajout d'index dans de grandes tables peut prendre un certain temps. Elles ne sont donc pas ajoutées automatiquement. En exécutant "occ db:add-missing-indices", ces index manquants pourront être ajoutés manuellement pendant que l'instance continue de tourner. Une fois les index ajoutés, les requêtes sur ces tables sont généralement beaucoup plus rapides.

I tried the command, but the command result was :

This version of Nextcloud requires at least PHP 7.2
You are currently running 7.0.33-25+020200225.32+debian91.gbpa11893. Please update your PHP version

For the record, the command should be sudo -u nextcloud php7.3 occ db:add-missing-indices. And it worked :)

@maniackcrudelis

This comment has been minimized.

Copy link
Contributor Author

maniackcrudelis commented Mar 20, 2020

The command was about to be executed 10 minutes after your upgrade.
https://github.com/YunoHost-Apps/nextcloud_ynh/blob/testing/scripts/upgrade.d/upgrade.last.sh#L15

@lapineige

This comment has been minimized.

Copy link
Member

lapineige commented Mar 20, 2020

Well, that's great if it was planned ^^ - I suppose it will create no issue ?

@lapineige

This comment has been minimized.

Copy link
Member

lapineige commented Mar 20, 2020

Maybe it's an off-topic question : for those we are using this Nextcloud 18, does OnlyOffice works for you ?
I can't find a way to activate the integration in Nextcloud 18. Is Yunohost version supporting it ?

Edit: I found this article https://nextcloud.com/blog/how-to-install-onlyoffice-in-nextcloud-hub-and-new-integration-feature/

But during the install of the app, I reloaded the page as it looked like i was frozen, and now I have this:

An exception occurred while executing 'ALTER TABLE oc_documentserver_changes ADD change_index BIGINT NOT NULL': SQLSTATE[42S21]: Column already exists: 1060 Duplicate column name 'change_index'

I guess the install failed part way 🤔

edit 2 : deactivated it, reactivated it, waited a while… it works !

edit 3: and know… loading a odt file gives me an error ONLYOFFICE est indisponible. Veuillez contacter votre administrateur

@kay0u

This comment has been minimized.

Copy link
Member

kay0u commented Mar 20, 2020

update is_url_handled
@maniackcrudelis

This comment has been minimized.

Copy link
Contributor Author

maniackcrudelis commented Mar 21, 2020

Guys, do you realize that we were supposed to declare this testing ready to be reviewed today, after 7 days of testing, and you did merge commits in the meantime without advertising the community, nor updating that very PR.

So, first, update when you add commits, tell people as well.
And, I extend this testing for 3 more days to have feedback on those new commits. (3 days considering the commits will be added to the forum and this PR today...)

@kay0u kay0u mentioned this pull request Mar 21, 2020
3 of 8 tasks complete
@lapineige

This comment has been minimized.

Copy link
Member

lapineige commented Mar 21, 2020

Regarding OnlyOffice integration: should we add more information about it (link to a tutorial, information about ARM support...) in the readme/forum ?
I guess it's expected by a lot of users who might be confused if it doesn't work out-of-the-box (additionally that would mean a lot of support work "for nothing") without further information.

@lapineige

This comment has been minimized.

Copy link
Member

lapineige commented Mar 21, 2020

Upgrade tested again, this time again from 15.0.14 -> 18, but on a Rasberry Pi 3 (so ARM platform).
Upgrade went fine, everything seems to be ok, I only saw that error message:

Process AsynchronousFileReader-3:
Traceback (most recent call last):
File "/usr/lib/python2.7/multiprocessing/process.py", line 258, in _bootstrap
self.run()
File "/usr/lib/python2.7/dist-packages/moulinette/utils/stream.py", line 47, in run
data += os.read(self._fd, 50)
OSError: [Errno 11] Resource temporarily unavailable

@maniackcrudelis

This comment has been minimized.

Copy link
Contributor Author

maniackcrudelis commented Mar 24, 2020

Looks ok to me.

@kay0u
kay0u approved these changes Mar 24, 2020
Copy link
Member

kay0u left a comment

Let's merge and pray in 3 days

@maniackcrudelis maniackcrudelis merged commit d7cabe4 into master Mar 27, 2020
@lapineige

This comment has been minimized.

Copy link
Member

lapineige commented Mar 27, 2020

And… it begins ! :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.