Skip to content
This repository was archived by the owner on Jan 7, 2026. It is now read-only.

Conversation

@ildyria
Copy link
Member

@ildyria ildyria commented Feb 6, 2025

No description provided.

d7415
d7415 previously requested changes Feb 9, 2025
@ildyria
Copy link
Member Author

ildyria commented Feb 9, 2025

Yes because the docker env are passed to Lychee via the inject.sh

@ildyria ildyria requested a review from d7415 February 9, 2025 23:03
@ildyria ildyria requested a review from d7415 February 10, 2025 16:07
@ildyria ildyria dismissed d7415’s stale review February 12, 2025 17:32

Fixes have been applied per request.

#- APP_DEBUG=true
#- APP_FORCE_HTTPS=false
#- APP_URL=http://localhost
- APP_URL=${APP_URL:-http://localhost}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already the default - does this one not pass through?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it makes it easier to modify in .env without having to fiddle in lychee/config/.env as misconfiguration of this value is the first cause of failure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it's an env value, so wouldn't it work when modified in .env anyway?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you mean ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the idea was that setting APP_URL in .env set the environment variable in the container rather than in the shell? Which would mean this should pass through without adding that line.

Copy link
Member Author

@ildyria ildyria left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replied.

README.md Outdated
## Table of Contents
<!-- TOC depthFrom:1 depthTo:6 withLinks:1 updateOnSave:1 orderedList:0 -->
- [Image content](#image-content)
- [Notice: Dockerhub repository has been migrated to lycheeorg/lychee](#notice-dockerhub-repository-has-been-migrated-to-lycheeorglychee)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point we can probably drop this notice.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines -129 to -144
## Advanced configuration

Note that nginx will accept by default images up to 100MB (`client_max_body_size 100M`) and that PHP parameters are overridden according to the [recommendations of the Lychee FAQ](https://lycheeorg.github.io/docs/faq.html#i-cant-upload-large-photos).

You may still want to further customize PHP configuration. The first method is to mount a custom `php.ini` to `/etc/php/8.2/fpm/php.ini` when starting the container. However, this method is kind of brutal as it will override all parameters. It will also need to be remapped whenever an image is released with a new version of PHP.

Instead, we recommend to use the `PHP_VALUE` directive of PHP-FPM to override specific parameters. To do so, you will need to mount a custom `nginx.conf` in your container :

1. Take the [default.conf](./default.conf) file as a base
2. Find the line starting by `fastcgi_param PHP_VALUE [...]`
3. Add a new line and set your new parameter
4. Add or change any other parameters (e.g. `client_max_body_size`)
5. Mount your new file to `/etc/nginx/nginx.conf`

If you need to add (not change) nginx directives, files mounted in `/etc/nginx/conf.d/` will be included in the `http` context.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to object to this, but I guess these overrides aren't really needed any more (unless someone really knows what they're doing, and so can figure it out easily enough).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it, the main reason I am dropping it is because Lychee now supports upload by chunks, as a result the limit on post request is no longer an issue for large files. :)

ildyria and others added 2 commits February 14, 2025 08:32
@ildyria ildyria merged commit e047a4a into master Feb 14, 2025
1 check passed
@ildyria ildyria deleted the redis-cache branch February 14, 2025 07:57
Copy link

@ARUNRAJPUT234 ARUNRAJPUT234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Deleted by maintainer]

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants