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

3.6.0 #450

Open
wants to merge 6 commits into
base: testing
Choose a base branch
from
Open

3.6.0 #450

wants to merge 6 commits into from

Conversation

ericgaspar
Copy link
Member

Problem

  • Description of why you made this PR

Solution

  • And how do you fix that problem

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)

@ericgaspar
Copy link
Member Author

!testme

@yunohost-bot
Copy link
Contributor

🐛
Test Badge

@yunohost-bot
Copy link
Contributor

Alrighty!
Test Badge

@MadMan247
Copy link

In the official peertube release notes, it indicates that we have to manually run a migration script for this release.

Classic installation: cd /var/www/peertube/peertube-latest && sudo -u peertube NODE_CONFIG_DIR=/var/www/peertube/config NODE_ENV=production node dist/scripts/migrations/peertube-6.3.js

Will the yunohost installation run that for us? If not, is the above script correct? We do not have the directory peertube-latest in /var/www/peertube currently, is this a problem?

@ericgaspar
Copy link
Member Author

!testme

@yunohost-bot
Copy link
Contributor

Alrighty!
Test Badge

@yunohost-bot
Copy link
Contributor

📖
Test Badge

@MadMan247
Copy link

Be careful! I could be misleading you unintentionally. Please test before you publish.
The wording is strange, but this migration may not be able to be done during the upgrade process.
"You need to manually execute a migration script after your upgrade while PeerTube is running and the database migration is complete..."
This suggests that peertube must be running. Is there a separate migration that must be completed before this migration? It is unclear.

Thank you dearly for all that you do! Your efforts are appreciated!

@ericgaspar
Copy link
Member Author

Thanks for pointing out the need for a migration for this release. I added this migration after to the previous migration scripts already implemented. to be tested then...

@MadMan247
Copy link

I notice the tests have failed (they say they're still running). If I may be so audacious, I think the solution would be to put the upgrade script after the #Start systemd service section.

Something like

#=================================================
# START SYSTEMD SERVICE
#=================================================
ynh_script_progression --message="Starting systemd service..."
ynh_systemd_action --service_name=$app --action="start" --log_path="systemd" --line_match="HTTP server listening on 127.0.0.1"

#====================
#START 6.3.0 DB MIGRATION
#====================

if ynh_compare_current_package_version --comparison lt --version 6.3.0~ynh1; then
	ynh_script_progression --message="Running Peertube 6.3.0 migration script..."
	pushd "$install_dir"
		ynh_exec_warn_less ynh_exec_as $app env $ynh_node_load_PATH NODE_CONFIG_DIR="$install_dir/config" NODE_ENV=production $ynh_node dist/scripts/migrations/peertube-6.3.js
	popd
fi

Would this be considered bad practice?

@ericgaspar
Copy link
Member Author

!testme

@yunohost-bot
Copy link
Contributor

✌️
Test Badge

@yunohost-bot
Copy link
Contributor

📖 🪱
Test Badge

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

Successfully merging this pull request may close these issues.

3 participants