Use new helpers, add backup/restore and uwsgi #6

Jibec · 2017-10-30T16:57:51Z

Hi, I wanted to add backup and restore, and I finally rewrote this package.

Some part I did not understood where removed (like swap) or I didn't like the easy_install ran as root.

Using gunicorn looked like to require a port, I decided to use uwsgi to have a socket instead.

I'm unsure it works, package_check gives weird results (upgrade doesn't work with package_check, but works fine on my VM). Please have a looks and play with it :)

Do not install this in production if you are not ready to loose your data.

Jibec · 2017-11-16T22:27:55Z

this work is now ready for testing, please come and try, but use it carefully, everything is in your database, to be safe, just dump it!

rigelk · 2017-12-29T13:39:15Z

Since I don't use YunoHost anymore in production, I can't test it fully right now. Look very nice though! I will be testing it and doing a proper review in the upcoming days if I manage to free some time for a proper test.

Jibec · 2018-01-18T21:58:02Z

I see time is hard to find!
Do you mind transfering this application to me?

Jibec · 2018-01-29T21:50:00Z

@maniackcrudelis I can't figure out the reason why upgrade from fd63504 and 267ccc2 fails as it works fine in my VM... I would appreciate it you may have a look.

@rigelk it also includes a maintainer change, (btw, beudbeud was written)

Josue-T · 2018-06-28T07:23:32Z

Hello,
Thanks @Jibec for your work.
I think it could be good to go forward with this PR because I have the same issue #7 and it might affect many user. This PR should really improve the package. I allow myself to just review a little bit the code.

Josue-T · 2018-06-28T07:26:55Z

conf/nginx.conf

+	uwsgi_read_timeout 3600;
+	uwsgi_param SCRIPT_NAME __PATH__;
+	uwsgi_modifier1 30;
+	uwsgi_pass unix:///tmp/__NAME__.socket;


Why do you store a socket in /tmp, is it not better in something like /run/uwsgi/app/searx as same as in searx ?

That's true, I'll improve that someday

Josue-T · 2018-06-28T07:28:20Z

manifest.json

@@ -1,25 +1,25 @@
 {
    "name": "Firefox Sync Server",
    "id": "ffsync",
-    "url": "https://github.com/abeudin/ffsync_ynh",
+    "version": "1.6.0",


Sorry, could you use the new syntax for versioning (1.6.0~ynh1) ?

yep, fixed.

Josue-T · 2018-06-28T07:30:23Z

scripts/_common.sh

+# usage: ynh_setup_source dest_dir [source_id]
+# | arg: dest_dir  - Directory where to setup sources
+# | arg: source_id - Name of the app, if the package contains more than one app
+ynh_setup_source_local () {


I think this helper is in the core so, we don't really need it here.

true, let's fix this

Josue-T · 2018-06-28T07:34:56Z

manifest.json

    "packaging_format": 1,
    "description": {
        "en": "Mozilla’s Sync-Server to host your Firefox account data",
        "fr": "Le serveur de synchronisation de Mozilla, pour héberger vos données Firefox"
    },
    "maintainer": {
-        "name": "beudbeud",
-        "email": "beudbeud@beudibox.fr",
-        "url": "https://github.com/balu-/FSyncMS"


Mybe you could add the previous_maintainers key.

Josue-T · 2018-06-28T07:37:47Z

scripts/install

-# Use 'FSyncMS' as database name and user
-db_user=ffsync
+# Check destination directory
+final_path="/var/www/$app"


Why do you use /var/www as final_path for an app which use python and wsgi. I think most the common for that is /opt/yunohost/$app

unless this changed recently, this looks wrong: https://github.com/YunoHost/example_ynh/blob/master/scripts/install#L45

Yes and no,

Many app (by example searx, or synapse) which use python use /opt/yunohost/$app.

Well, ok, I won't change it if there it isn't mandatory.

So, I don't really understand why you change that because before it was on the right place (see here). So why do you change that ?

I really think that /var/www/$app is more for php / web apps but not for uwsgi / python app and many app apply this concept.

Following the discussion here, I think that it could be good to change that. I could also do it if you don't have time to do it.

Josue-T · 2018-06-28T07:39:34Z

scripts/install

+
+# TODO: fix this css patch
+# ynh_replace_string "media\/img@$path_url\/media\/img@g" $final_path/syncserver/page/sync_files/firefox_sync-bundle.css
+# ynh_replace_string "media\/img@$path_url\/media\/img@g" $final_path/syncserver/page/sync_files/responsive-bundle.css


What is it ?

no idea, I did not understood either

me neither.

Josue-T · 2018-06-28T07:45:24Z

scripts/upgrade

+#=================================================
+
+# Backup the current version of the app
+ynh_backup_before_upgrade


Are we really able to do a backup if we upgrade the old installation style ?

Jibec · 2018-06-28T11:40:13Z

Thanks for your comments. I won't work on this package anymore until I'm sure it can be merged. Please let me know if you can get approval from rigelk or YunoHost Apps team.

rigelk · 2018-06-28T11:56:24Z

@Jibec I can approve and merge the request when you deem it ready.

Jibec · 2018-06-28T12:02:51Z

oh, good to know you answers :) Then I'll read and answer questions, but I expect you to give feedback from times to time if you're merging requests ;)

Jibec · 2018-07-05T21:25:27Z

It now looks like to works fine from old install and every basic scenario.

Please test upgrade: yunohost app install https://github.com/YunoHost-Apps/ffsync_ynh/ && yunohost app upgrade ffsync -u https://github.com/jibec/ffsync_ynh/

rigelk · 2018-07-05T23:27:51Z

Works for me!

Jibec · 2018-07-06T05:05:58Z

Thanks for the test. Can you confirm you did test with real data? I have to say I don't have time yet to make a real end user test. I assume using a dedicated Firefox profile would do the job (create the profile, sync data, remove the profile, upgrade app, create a new profile, sync data: should bring back previous data). Is it a good way to test or do you know other?

licryle · 2018-08-08T03:29:18Z

Well, f6b79fe will certainly help with one of the 3 issues I've raised, but not the 2 other ones.

Reminder:

Path issue: licryle@5ad912e
Dependencies (like your patch): licryle@10f63a2
Service setup: licryle@d762eab && licryle@2fab09f

licryle · 2018-08-08T03:31:15Z

I see this topic is closed, but I don't think it's working, are you tracking (1) and (3) independently?
Happy to help as mentioned before.

Josue-T · 2018-08-22T20:08:55Z

Hello,

I have pushed some fix, so it might be better now.

I have also tested the migration from the old package with my prod instance (on ARM) and it was good.

I think it should be ready for some other tests.

Josue-T · 2018-08-29T15:21:21Z

If nobody mind I purpose to merge this in 10 days

Jibec · 2018-08-29T17:23:57Z

I'm against this merge as you can't add me as maintainer (I canceled my pull request). I'm fine with the idea this apps is taken over by you, and that my work may help this app to move forward! I'm sorry I couldn't say yes to all requests and decided to stop my contribution (for multiple good and bad reasons, mostly unrelated to this package). Keep the hard work!

Josue-T · 2018-08-29T18:17:00Z

Ok,

I you want I can give you the access to this repos. It's you want.

Josue-T · 2018-09-02T13:32:04Z

Hello,

I discovered some issue (when I restart the server) with uwsgi. @Jibec what is the reason to not use the main uwsgi service (which automatically start all service in /etc/uwsgi/app-enabled) ?

Josue-T · 2018-09-06T21:15:59Z

Hello,
I have pushed some new fix which might fix some problem especially on the reboot. Without this fix ffsync didn't started correctly on the reboot.

If somebody can try this it could be good. And if it's ok for everybody I purpose to merge this in 10 days if I don't find some new issues.

About the maintainer who will be the maintainer of this app ?

Josue-T · 2018-12-05T17:09:05Z

Let's merge this.

I'll be the maintainer of this app (if nobody mind).

Jibec · 2018-12-05T18:21:23Z

I'm explicitly OK for you being maintainer of this app. -- Jean-Baptiste Holcroft

rigelk · 2018-12-05T18:44:15Z

Same for me.

Jibec added 4 commits October 30, 2017 13:53

Remove sources

8b28531

use helpers & uwsgi. Enable multi instance

745f424

upgrade to 1.6.0

21cf715

Fix upgrade from previous versions

333e097

Remove useless user rights

23645f9

Jibec mentioned this pull request Dec 26, 2017

Firefox Android fails to sync with TokenServerMalformedResponseException #7

Closed

maniackcrudelis requested a review from rigelk December 26, 2017 19:47

fix alias_traversal and change maintainer

3b3cabb

use local cache storage and pip instead of make

dd241fe

Josue-T reviewed Jun 28, 2018

View reviewed changes

Jibec added 3 commits June 28, 2018 23:09

Upgrade to 1.8.0

cf0f16a

Remove local download path and add previous maintainer

613527a

Rewrite uwsgi to use the helper

f6682e6

Jibec closed this Aug 4, 2018

Josue-T added 7 commits August 20, 2018 22:34

Merge commit 'licryle' into master

c46400e

Fix final path

d919dcc

Fix uwsgi helper

14e1be1

Store settings before to test the install

03944dd

Dont save path and domain it's automatically done

e346c9e

Restore home page

f8b2336

Fix permission in final_path

b88c7ad

Josue-T reopened this Aug 22, 2018

Josue-T mentioned this pull request Aug 29, 2018

Not working? #9

Closed

Josue-T approved these changes Aug 29, 2018

View reviewed changes

Josue-T added 4 commits September 5, 2018 21:34

Update uwsgi helper

e345831

Change order of instruction and fix uwsgi helper

87d4333

Fix backup - restore

04f9427

Fix nginx config

7a1958e

Josue-T added 3 commits September 25, 2018 21:32

Change maintener

d35b849

Remove logrotate, it's already managed by uwsgi package

ad64328

Remove start service by the socket

c8b399a

Josue-T merged commit f021251 into YunoHost-Apps:master Dec 5, 2018

Use new helpers, add backup/restore and uwsgi #6

Use new helpers, add backup/restore and uwsgi #6

Conversation

Jibec commented Oct 30, 2017

Jibec commented Nov 16, 2017

rigelk commented Dec 29, 2017 • edited

Jibec commented Jan 18, 2018

Jibec commented Jan 29, 2018

Josue-T commented Jun 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Josue-T Jun 28, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Josue-T Jun 28, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jibec commented Jun 28, 2018 via email

rigelk commented Jun 28, 2018

Jibec commented Jun 28, 2018 via email

Jibec commented Jul 5, 2018

rigelk commented Jul 5, 2018

Jibec commented Jul 6, 2018 via email

licryle commented Aug 8, 2018

licryle commented Aug 8, 2018

Josue-T commented Aug 22, 2018

Josue-T commented Aug 29, 2018

Jibec commented Aug 29, 2018 via email

Josue-T commented Aug 29, 2018

Josue-T commented Sep 2, 2018

Josue-T commented Sep 6, 2018

Josue-T commented Dec 5, 2018

Jibec commented Dec 5, 2018 via email

rigelk commented Dec 5, 2018

rigelk commented Dec 29, 2017 •

edited

Josue-T Jun 28, 2018 •

edited

Josue-T Jun 28, 2018 •

edited