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

V2 #426

Merged
merged 95 commits into from
Mar 8, 2024
Merged

V2 #426

merged 95 commits into from
Mar 8, 2024

Conversation

Josue-T
Copy link

@Josue-T Josue-T commented Nov 1, 2023

Problem

Solution

  • Rework package with packaging v2 practice
  • Rework config panel with settings instead of sed directly in config file

TODO

PR Status

  • Code finished and ready to be reviewed/tested
  • The fix/enhancement were manually tested (if applicable)

Automatic tests

Automatic tests can be triggered on https://ci-apps-dev.yunohost.org/ after creating the PR, by commenting "!testme", "!gogogadgetoci" or "By the power of systemd, I invoke The Great App CI to test this Pull Request!". (N.B. : for this to work you need to be a member of the Yunohost-Apps organization)

Josue-T and others added 4 commits October 31, 2023 22:19
- Adapt all script with packaging v2
- Rework cleanly control pannel and remove all replace in file as it's breaken after each update
- Cleanup
@Thatoo
Copy link

Thatoo commented Nov 2, 2023

I tried to install it but it has failed : https://paste.yunohost.org/raw/uwetisotec

@Josue-T
Copy link
Author

Josue-T commented Nov 2, 2023

I tried to install it but it has failed : https://paste.yunohost.org/raw/uwetisotec

Well, as it's written, it's a draft, so it's not finished. 😉

@Thatoo
Copy link

Thatoo commented Nov 15, 2023

  • Install works now : happy v2!!!
  • I could login : CAS works again
  • but when I tried to change some settings, I got this error :
2023-11-15 21:28:33,296: DEBUG - + ynh_die '--message=Variable $data_dir wasn'\''t initialized when trying to replace __DATA_DIR__ in /etc/matrix-synapse/homeserver.yaml'
2023-11-15 21:28:33,390: WARNING - Variable $data_dir wasn't initialized when trying to replace __DATA_DIR__ in /etc/matrix-synapse/homeserver.yaml

https://paste.yunohost.org/raw/asuwowopuj

- for mail stack we use the system user with password provided by yunohost settings and for LDAP filter we still be waiting on matrix-org/matrix-synapse-ldap3#186
@csolisr
Copy link

csolisr commented Nov 21, 2023

* [ ]  but when I tried to change some settings, I got this error :
2023-11-15 21:28:33,296: DEBUG - + ynh_die '--message=Variable $data_dir wasn'\''t initialized when trying to replace __DATA_DIR__ in /etc/matrix-synapse/homeserver.yaml'
2023-11-15 21:28:33,390: WARNING - Variable $data_dir wasn't initialized when trying to replace __DATA_DIR__ in /etc/matrix-synapse/homeserver.yaml

Similar issue here - and it's even more complicated now, because I can't even force upgrading to the latest version:

sudo yunohost app upgrade synapse -u https://github.com/YunoHost-Apps/synapse_ynh/tree/v2 -F
Info: Now upgrading synapse...
Info: Creating a safety backup prior to the upgrade
Info: Collecting files to be backed up for synapse...
Warning: It's hightly recommended to make your backup when the service is stopped. Please stop synapse service with this command before to run the backup 'systemctl stop matrix-synapse.service'
Info: Declaring files to be backed up...
Warning: /var/cache/yunohost/app_tmp_work_dirs/app_1sckfx0q/scripts/backup: line 52: synapse_db_name: variable unassigned
Error: Could not back up synapse
Info: The operation 'Create a backup archive' could not be completed. Please share the full log of this operation using the command 'yunohost log share 20231121-174305-backup_create' to get help
Error: Nothing to save

Is there some way of manually setting the synapse_db_name variable in YNH so I can upgrade?

@Josue-T
Copy link
Author

Josue-T commented Nov 21, 2023

Hello,

PLEASE, don't upgrade to this version. This PR is just for information but is NOT ready. There still be a lot of work before to release it !! It could eventually be tested but NOT on production.

@csolisr
Copy link

csolisr commented Nov 21, 2023

Hello,

PLEASE, don't upgrade to this version. This PR is just for information but is NOT ready. There still be a lot of work before to release it !! It could eventually be tested but NOT on production.

Yup, just noticed it the hard way. Guess I'll have to wipe and reinstall the stable version later.

@csolisr
Copy link

csolisr commented Nov 21, 2023

Oh and if you stubbornly tried this PR before it was baked, and now you're stuck on the update part like I did, you can try setting the uninitialized values by hand:

sudo yunohost app setting synapse synapse_db_name -v $(sudo yunohost app setting synapse db_name);
sudo yunohost app setting synapse data_path -v $(sudo yunohost app setting synapse data_dir);

@Josue-T Josue-T mentioned this pull request Nov 22, 2023
2 tasks
@Josue-T Josue-T mentioned this pull request Nov 22, 2023
2 tasks
@Josue-T
Copy link
Author

Josue-T commented Mar 6, 2024

!bookwormtestme

@yunohost-bot
Copy link

📚
Test Badge

@Josue-T
Copy link
Author

Josue-T commented Mar 6, 2024

!bookwormtestme

@yunohost-bot
Copy link

Living in the future, are we?
Test Badge

@Josue-T
Copy link
Author

Josue-T commented Mar 6, 2024

!testme

@yunohost-bot
Copy link

🚀
Test Badge

@Josue-T
Copy link
Author

Josue-T commented Mar 7, 2024

!testme !bookwormtestme

@yunohost-bot
Copy link

📚 🪱
Test Badge

@yunohost-bot
Copy link

🌻
Test Badge

@@ -1,7 +1,7 @@
[Unit]
Description=Coturn
Documentation=man:coturn(1) man:turnadmin(1) man:turnserver(1)
After=syslog.target network.target
After=syslog.target network-online.target

Choose a reason for hiding this comment

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

This is supposed to fix issue #313 but it is not always working.
On my server, after making the change, the synapse-coturn service is still failing to start at boot time.
One way to fix the issue that I have tested and that is working is to add the parameter -L 0.0.0.0 to the ExecStart line, as in this commit on my fork of the app : Lab-8916100448256@d211419

Copy link
Author

Choose a reason for hiding this comment

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

Well are you really sure about that this parameter -L 0.0.0.0 will fix the issue ?

@Josue-T
Copy link
Author

Josue-T commented Mar 7, 2024

!testme !bookwormtestme

@yunohost-bot
Copy link

✌️
Test Badge

@yunohost-bot
Copy link

📚 🐛
Test Badge

@Josue-T
Copy link
Author

Josue-T commented Mar 8, 2024

Let's merge as we don't know when YunoHost/package_linter#136 will be merged.

@Josue-T Josue-T merged commit 84aca30 into testing Mar 8, 2024
@Josue-T Josue-T deleted the v2 branch March 8, 2024 11:05
@lapineige lapineige mentioned this pull request Mar 8, 2024
7 tasks
nathanael-h added a commit to nathanael-h/synapse_ynh that referenced this pull request Apr 17, 2024
Since the migration to the packaging v2 in YunoHost-Apps#426 the system user for this app is $app and not matrix-$app anymore.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment