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

Web Server Improvements #374

Merged
merged 7 commits into from
Feb 19, 2022
Merged

Web Server Improvements #374

merged 7 commits into from
Feb 19, 2022

Conversation

efrecon
Copy link
Contributor

@efrecon efrecon commented Feb 18, 2022

This PR focuses on 3 things (sorry!):

  • It fixes issue Unable to host under subpath  #39 by passing directly the configured root path.
  • It adds access logging to the serve sub-command. All accesses, including errors, are logged through logrus at the info (success), or warn (error) levels. Lines provide information about the requests/response from the server. Logging is on by default, and can be turned on using the --log flag at the command-line.
  • It uses dynamic information from the github context when building and testing in the GitHub workflows. This facilitates for forks to properly run the workflows, as the resulting packages will be hosted within the fork's account.

Docker images corresponding to the content of this PR can be found within my fork

path.Join trims the trailing slash if the path isn't /, use configured
root instead.
This makes the workflows agnostic of the repository root, making it
possible to build in forked repos.
This adds proper logging for all resources, including errors. Logging
is on by default, but can be turned off.
@efrecon
Copy link
Contributor Author

efrecon commented Feb 18, 2022

A side note, as this was mentioned in the original issue: I have tested this behind the Caddy v2 reverse proxy. Pointing to it with the handle directive performs as expected. Provided shiori is run with --webroot /shiori/. The following configuration snippet is all that is needed:

  redir /shiori /shiori/
  handle /shiori/* {
    reverse_proxy http://shiori:8080 {
    }
  }

internal/cmd/serve.go Outdated Show resolved Hide resolved
@fmartingr fmartingr linked an issue Feb 19, 2022 that may be closed by this pull request
@fmartingr
Copy link
Member

Awesome @efrecon, I think we're good to go!

Thanks a lot for your improvements :)

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.

Unable to host under subpath
2 participants