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

Add downloads for nightlies and pull requests #351

Merged
merged 5 commits into from
May 18, 2023

Conversation

DomClark
Copy link
Member

@DomClark DomClark commented Nov 6, 2022

The main changes in this PR:

  • Add actual tip-of-master nightlies to the downloads page, and rename the alpha builds from "Nightly" to "Alpha".
  • Add a page for downloading pull request builds.
  • Add a redirect route for downloading GitHub Actions artifacts without the user needing to log in to GitHub (similar to https://nightly.link/). This is required for the nightly and pull request downloads.

Other changes:

  • Updated the library classes and download controller to use Symfony's autowiring dependency injection.
  • Moved the repository owner and name information into a config file.
  • Fixed various catch blocks to refer to Exception in the root namespace.
  • Added the jumbo into the download error page, so it looks a little less broken.
  • Added types to various functions.
  • Refactored the download templates to try and reduce code duplication slightly.

The artifact download feature requires authentication for the GitHub API, which also comes with the bonus of raising our rate limit from 60 to 3600 requests per hour. Obviously the credentials for this need to be set up separately from this PR. I've been testing this with a personal access token as it's the easiest to set up, but other options are available. I used the following in .env.local:

###> knplabs/github-api ###
GITHUB_AUTH_METHOD=access_token_header
GITHUB_USERNAME=[token redacted]
GITHUB_SECRET=
###< knplabs/github-api ###
Screenshots

image
image

@PhysSong
Copy link
Member

@tresf @liushuyu I think it's fine to merge this without my comments fully resolved, but I think the secrets must be set up on the server prior to merging this.

@tresf

This comment was marked as resolved.

@tresf tresf self-requested a review May 18, 2023 04:47
@tresf tresf self-requested a review May 18, 2023 04:57
@tresf

This comment was marked as resolved.

@tresf
Copy link
Member

tresf commented May 18, 2023

Happy to, but won't a the website deploy wipe out this file?

No response in two weeks, so I'm going to have to find out the hard way. 😅

I've deployed the necessary credentials to ~/lmms.io/.env.local. If they're wiped out, I can copy them manually, but I may need some help making the changes permanent. 🤞

@tresf tresf merged commit e50a052 into LMMS:master May 18, 2023
@tresf
Copy link
Member

tresf commented May 18, 2023

Happy to, but won't a the website deploy wipe out this file?

Yeah, it does. 🤔

@tresf

This comment was marked as resolved.

@tresf
Copy link
Member

tresf commented May 18, 2023

I've added .env.local to the bottom of deploy-config.php, but I can't confirm if this is still being used for our webhooks: https://github.com/LMMS/lmms.io/settings/hooks.

Update: It is. After another commit, the webhook is showing as delivered. Also, my EXCLUDE changes seem to have taken.

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.

4 participants