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

Update Configuration.md to show SHIORI_HTTP_PORT deprecation. #844

Closed
wants to merge 1 commit into from

Conversation

tenpai-git
Copy link

@tenpai-git tenpai-git commented Feb 18, 2024

Data

  • Shiori version: v1.5.5
  • Database Engine: PostgreSQL
  • Operating system: Debian 12 Bookworm
  • CLI/Web interface/Web Extension: Command Line Interface (CLI)

Describe the bug / actual behavior

The configuration page has SHIORI_HTTP_PORT as a port you can assign as an environment variable for Linux distributions. However, even if you set the environment variable at the system level, the assignment will not work. With the introduction of the -p flag, it seems that Shiori will always default to 8080, no matter what you set SHIORI_HTTP_PORT to be as an environment variable, because the runtime variable is a higher priority. Instead you must set -p at runtime, or the hosted port will not change.

This is meant to be a simple pull request before the overall documentation revamp.

Proposed Change

Because other options are also deprecated, I recommend making another note in the configuration documentation about this for now in the same style as the other deprecation, as to reduce confusion for current users of Shiori. A future update for Shiori can address all deprecated configuration options and be easily found on this page for the developers.

To Reproduce

Steps to reproduce the behavior:

  1. Add an environment variable in /etc/environment for your Linux-based system (I used a Debian 12 Linux Container, location may slightly differ based on OS). This will make a global system variable accessible anywhere to your program, including Shiori.

root@host# echo SHIORI_HTTP_PORT=15000 >> /etc/environment

  1. Load your change to the file into the current running system environment. source /etc/environment

  2. Download, compile, and run Shiori on the same system. It will tell you the web server is running the HTTP protocol on port 8080.

In the directory you want to download Shiori:
git clone https://github.com/go-shiori/shiori

Use cd to enter that directory and compile and run the binary: go build main.go
./main server

The output will look like this:

INFO[2024-02-18T17:46:24Z] Starting Shiori vdev                         
INFO[2024-02-18T17:46:24Z] starting http server                          addr=":8080"

Setting a custom port.

The only way to change this behavior currently is to use the p option (for example, a custom port of 16000): ./main server -p 16000

The output should then become:

INFO[2024-02-18T17:47:54Z] Starting Shiori vdev                         
INFO[2024-02-18T17:47:54Z] starting http server                          addr=":16000"

Notes

This solution doesn't change the behavior of Shiori at all, it is only designed to be a temporary note in the same style as existing documentation to make addressing it easier later.

This documentation change will make it more clear to save time for users looking on how to set environment variables on manual or Infrastructure-As-Code automated installations (Ansible), and those unfamiliar with Golang.

Alternative solutions are to remove both deprecation notes and the options from the tables, and/or make a new table for deprecated options towards the bottom of the article in case additional deprecations are expected in upcoming development.

## Data
- **Shiori version**: v1.5.5 
- **Database Engine**: PostgreSQL
- **Operating system**: Debian 12 Bookworm
- **CLI/Web interface/Web Extension**: Command Line Interface (CLI)

## Describe the bug / actual behavior
The configuration page has `SHIORI_HTTP_PORT` as a port you can assign as an environment variable for Linux distributions. However, even if you set the environment variable at the system level, the assignment will not work. With the introduction of the `-p` flag, it seems that Shiori will always default to 8080, no matter what you set `SHIORI_HTTP_PORT` to be as an environment variable, because the runtime variable is a higher priority. Instead you *must* set `-p` at runtime, or the hosted port will not change.

## Proposed Change
Because other options are also deprecated, I recommend making another note in the configuration documentation about this for now in the same style as the other deprecation, as to reduce confusion for current users of Shiori. A future update for Shiori can address all deprecated configuration options and be easily found on this page for the developers.

## To Reproduce
Steps to reproduce the behavior:
1. Add an environment variable in `/etc/environment` for your Linux-based system (I used a Debian 12 Linux Container, location may slightly differ based on OS). This will make a global system variable accessible anywhere to your program, including Shiori.

`root@host# echo SHIORI_HTTP_PORT=15000 >> /etc/environment`

2. Load your change to the file into the current running system environment. 
`source /etc/environment`

3. Download, compile, and run Shiori on the same system. It will tell you the web server is running the HTTP protocol on port `8080`. 

In the directory you want to download Shiori:
`git clone https://github.com/go-shiori/shiori`

Use `cd` to enter that directory and compile and run the binary: 
`go build main.go`
`./main server` 

The output will look like this: 
```
INFO[2024-02-18T17:46:24Z] Starting Shiori vdev                         
INFO[2024-02-18T17:46:24Z] starting http server                          addr=":8080"
```

## Setting a custom port. 
The only way to change this behavior currently is to use the `p` option (for example, a custom port of 16000): 
`./main server -p 16000` 

The output should then become:
```
INFO[2024-02-18T17:47:54Z] Starting Shiori vdev                         
INFO[2024-02-18T17:47:54Z] starting http server                          addr=":16000"
```

## Notes
This solution doesn't change the behavior of Shiori at all, it is only designed to be a temporary note in the same style as existing documentation to make addressing it easier later. 

This documentation change will make it more clear to save time for users looking on how to set environment variables on manual or Infrastructure-As-Code automated installations (Ansible), and those unfamiliar with Golang.
@tenpai-git
Copy link
Author

tenpai-git commented Mar 17, 2024

Is this one good to go @fmartingr or is there a check or something I should do? Or do you just want to take it in a different direction?

@fmartingr
Copy link
Member

Hey @tenpai-git, this is probably a bug more than a deprecation, I had no intent of removing this flag. Let me take a look at this.

@fmartingr
Copy link
Member

Yeah this is obviously a bug, we are overriding the configuration from the flags without checking if the user actually set something in the flags. I'm going to patch this temporarily while we figure out a proper way to move forward, thanks for noticing.

@tenpai-git
Copy link
Author

tenpai-git commented Mar 23, 2024

Thank you @fmartingr - I'll close this pull request since the option is meant to remain. Would love a mention on the next release and glad I could contribute in some way. Happy to test.

@tenpai-git tenpai-git closed this Mar 23, 2024
@tenpai-git tenpai-git deleted the patch-1 branch March 23, 2024 19:52
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.

None yet

2 participants