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

Docker: Add DATA_PATH to cron env #5531

Merged
merged 1 commit into from
Jul 19, 2023
Merged

Conversation

zhaofengli
Copy link
Contributor

@zhaofengli zhaofengli commented Jul 17, 2023

Changes proposed in this pull request:

  • Adds $DATA_PATH to the cron environment

This fixes automatic feed updating with non-standard data paths.

How to test the feature manually:

  1. Set a non-standard DATA_PATH and enable automatic feed updating with CRON_MIN
  2. cat /tmp/FreshRSS.log
  3. See successful result instead of Failed requirements!

Pull request checklist:

  • clear commit messages
  • code manually tested
  • unit tests written (optional if too hard)
  • documentation updated

Additional information can be found in the documentation.

@Alkarex Alkarex added the Docker Everything related to Docker label Jul 19, 2023
@Alkarex Alkarex added this to the 1.22.0 milestone Jul 19, 2023
@Alkarex
Copy link
Member

Alkarex commented Jul 19, 2023

Hello,
It probably does not hurt to pass this variable, so let's do it. However, the expected way in Docker is to map the data path to a volume, which can be where you want, so I am a bit unsure of your use-case. Could you provide a bit more info?
https://github.com/FreshRSS/FreshRSS/tree/edge/Docker#docker-compose

Please add a line for you in https://github.com/FreshRSS/FreshRSS/blob/edge/CREDITS.md

@Alkarex
Copy link
Member

Alkarex commented Jul 19, 2023

Related to #5423

@Alkarex Alkarex merged commit c35a9ee into FreshRSS:edge Jul 19, 2023
1 check passed
@zhaofengli
Copy link
Contributor Author

However, the expected way in Docker is to map the data path to a volume, which can be where you want, so I am a bit unsure of your use-case.

Sorry for the late reply. My use-case is to deploy FreshRSS on Kubernetes with Litestream1. Litestream runs as a sidecar container with a shared ephemeral directory mounted at /data. I could have used /var/www/FreshRSS/data, but DATA_PATH exists and I want to use a path consistent with other apps I deploy.

Please add a line for you in edge/CREDITS.md

Thanks, I'll add it the next time I contribute 😄

Footnotes

  1. I also patch FreshRSS downstream to use a single SQLite database with table prefixes, but this is obviously too cursed to be upstreamed.

@zhaofengli zhaofengli deleted the cron-data-path branch July 24, 2023 00:30
@Alkarex
Copy link
Member

Alkarex commented Jul 24, 2023

Thanks for the explanations.

Thanks, I'll add it the next time I contribute

You can just make a new PR for that

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

Successfully merging this pull request may close these issues.

3 participants