feat(server): add env to define the path of build-lock.json #10810

martabal · 2024-07-03T16:02:59Z

This PR adds an env to define the path of build-lock.json

jrasm91

Is there a reason why you can't just read this from the CWD?

martabal · 2024-07-03T16:06:43Z

Is there a reason why you can't just read this from the CWD?

Yes, we don't use relative path, only absolute paths

bo0tzz · 2024-07-03T16:14:19Z

server/src/constants.ts

@@ -34,6 +34,7 @@ export const geodataDatePath = join(GEODATA_ROOT_PATH, 'geodata-date.txt');
 export const geodataAdmin1Path = join(GEODATA_ROOT_PATH, 'admin1CodesASCII.txt');
 export const geodataAdmin2Path = join(GEODATA_ROOT_PATH, 'admin2Codes.txt');
 export const geodataCities500Path = join(GEODATA_ROOT_PATH, citiesFile);
+export const lockfilePath = process.env.IMMICH_BUILD_LOCK_PATH || './build-lock.json';


Maybe this should just always use env, same as the other build-metadata stuff, with the base-image Dockerfile setting this particular one?

IMO, it would be great to group www/, build-lock.json and geodata/ into one folder and specify its path with an env. But I don't know if that would limit other packages like the Immich package from the AUR.

I kind of really rate having separate env for every file that we read from disk. I'd be perfectly happy with bundling all these files into a single, folder and having a single env for it. Something like IMMICH_BUILD_FILES or something.

I would be really happy too. Are you ok to have all these files/folders to /usr/src/resources ?
We would have:

/usr/src/resources/www

/usr/src/resources/geodata

/usr/src/resources/build-lock.json

bo0tzz · 2024-07-03T16:14:46Z

docs/docs/install/environment-variables.md

@@ -51,6 +51,7 @@ Regardless of filesystem, it is not recommended to use a network share for your
 | `CPU_CORES`                         | Amount of cores available to the immich server  | auto-detected cpu core count | server                   |                    |
 | `IMMICH_API_METRICS_PORT`           | Port for the OTEL metrics                       |            `8081`            | server                   | api                |
 | `IMMICH_MICROSERVICES_METRICS_PORT` | Port for the OTEL metrics                       |            `8082`            | server                   | microservices      |
+| `IMMICH_BUILD_LOCK_PATH`            | Path of build-lock.json                         |     `./build-lock.json`      | server                   | api                |


I don't think we documented the other build metadata env vars right?

(As in, I don't think we need to document this one either)

What do you mean by 'build metadata env'? We documented IMMICH_WEB_ROOT and IMMICH_REVERSE_GEOCODING_ROOT

I don't think we need to or should document build env related stuff on this page, which should just be user facing env settings.

jrasm91 · 2024-07-03T16:15:40Z

Is there a reason why you can't just read this from the CWD?

Yes, we don't use relative path, only absolute paths

What are you going to set it to?

martabal · 2024-07-03T16:25:17Z

What are you going to set it to?

Probably /app/immich/server/build-lock.json

jrasm91 · 2024-07-03T16:35:23Z

docs/docs/install/environment-variables.md

@@ -51,6 +51,7 @@ Regardless of filesystem, it is not recommended to use a network share for your
 | `CPU_CORES`                         | Amount of cores available to the immich server  | auto-detected cpu core count | server                   |                    |
 | `IMMICH_API_METRICS_PORT`           | Port for the OTEL metrics                       |            `8081`            | server                   | api                |
 | `IMMICH_MICROSERVICES_METRICS_PORT` | Port for the OTEL metrics                       |            `8082`            | server                   | microservices      |
+| `IMMICH_BUILD_LOCK_PATH`            | Path of build-lock.json                         |     `./build-lock.json`      | server                   | api                |


I don't think we need to or should document build env related stuff on this page, which should just be user facing env settings.

jrasm91 · 2024-07-03T16:36:35Z

server/src/constants.ts

@@ -34,6 +34,7 @@ export const geodataDatePath = join(GEODATA_ROOT_PATH, 'geodata-date.txt');
 export const geodataAdmin1Path = join(GEODATA_ROOT_PATH, 'admin1CodesASCII.txt');
 export const geodataAdmin2Path = join(GEODATA_ROOT_PATH, 'admin2Codes.txt');
 export const geodataCities500Path = join(GEODATA_ROOT_PATH, citiesFile);
+export const lockfilePath = process.env.IMMICH_BUILD_LOCK_PATH || './build-lock.json';


I kind of really rate having separate env for every file that we read from disk. I'd be perfectly happy with bundling all these files into a single, folder and having a single env for it. Something like IMMICH_BUILD_FILES or something.

alextran1502 · 2024-07-03T17:44:33Z

Hello, have you tested this in the production build?

martabal · 2024-07-03T18:23:52Z

Hello, have you tested this in the production build?

Hi! I built the base-image locally and ran the production build. It's running fine.

root@d98d644d1c6e:/usr/src/resources# ls -la
total 24
drwxr-xr-x 1 root root 4096 Jul  3 20:19 .
drwxr-xr-x 1 root root 4096 Jul  3 20:11 ..
-rw-r--r-- 1 root root  694 Jun 30 13:08 build-lock.json
drwxr-xr-x 2 root root 4096 Jul  3 20:13 geodata
drwxr-xr-x 3 root root 4096 Jul  3 20:19 www

jrasm91 · 2024-07-08T18:44:15Z

Closing in favor of #10958.

martabal requested a review from danieldietzler as a code owner July 3, 2024 16:03

github-actions bot added documentation Improvements or additions to documentation 🗄️server labels Jul 3, 2024

jrasm91 reviewed Jul 3, 2024

View reviewed changes

bo0tzz approved these changes Jul 3, 2024

View reviewed changes

bo0tzz reviewed Jul 3, 2024

View reviewed changes

jrasm91 approved these changes Jul 3, 2024

View reviewed changes

martabal force-pushed the feat/add-env-for-build-lock-path branch from 2ceef13 to 8bad84a Compare July 3, 2024 17:11

martabal mentioned this pull request Jul 3, 2024

refactor: build files immich-app/base-images#77

Merged

martabal force-pushed the feat/add-env-for-build-lock-path branch from 8bad84a to b13ef06 Compare July 3, 2024 17:18

fix: mv build-lock

c0309cd

martabal force-pushed the feat/add-env-for-build-lock-path branch from b13ef06 to c0309cd Compare July 3, 2024 20:41

jrasm91 closed this Jul 8, 2024

jrasm91 deleted the feat/add-env-for-build-lock-path branch July 8, 2024 18:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(server): add env to define the path of build-lock.json #10810

feat(server): add env to define the path of build-lock.json #10810

martabal commented Jul 3, 2024

jrasm91 left a comment

martabal commented Jul 3, 2024

bo0tzz Jul 3, 2024

martabal Jul 3, 2024

jrasm91 Jul 3, 2024

martabal Jul 3, 2024

jrasm91 Jul 3, 2024

bo0tzz Jul 3, 2024

bo0tzz Jul 3, 2024

martabal Jul 3, 2024

jrasm91 Jul 3, 2024

jrasm91 commented Jul 3, 2024

martabal commented Jul 3, 2024

jrasm91 Jul 3, 2024

jrasm91 Jul 3, 2024

alextran1502 commented Jul 3, 2024

martabal commented Jul 3, 2024

jrasm91 commented Jul 8, 2024

feat(server): add env to define the path of build-lock.json #10810

feat(server): add env to define the path of build-lock.json #10810

Conversation

martabal commented Jul 3, 2024

jrasm91 left a comment

Choose a reason for hiding this comment

martabal commented Jul 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrasm91 commented Jul 3, 2024

martabal commented Jul 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alextran1502 commented Jul 3, 2024

martabal commented Jul 3, 2024

jrasm91 commented Jul 8, 2024