Skip to content

fix: Pour avoir des variables ===null dans l'appli, il faut leur rajouter default::#1197

Open
njean42 wants to merge 1 commit intomasterfrom
njean42/fix-variables-env-nullable
Open

fix: Pour avoir des variables ===null dans l'appli, il faut leur rajouter default::#1197
njean42 wants to merge 1 commit intomasterfrom
njean42/fix-variables-env-nullable

Conversation

@njean42
Copy link
Copy Markdown
Collaborator

@njean42 njean42 commented Mar 17, 2026

Et ne pas définir du tout la variable d'environnement dans le fichir .env.

…uter `default::` dans le parser `env()`.

Et ne pas définir du tout la variable d'environnement dans le fichir `.env`.
Comment thread config/services.yaml
max_nb_of_past_cycles_to_display: '%env(MAX_NB_OF_PAST_CYCLES_TO_DISPLAY)%'
max_time_at_end_of_shift: '%env(MAX_TIME_AT_END_OF_SHIFT)%'
max_time_in_advance_to_book_extra_shifts: '%env(MAX_TIME_IN_ADVANCE_TO_BOOK_EXTRA_SHIFTS)%'
max_time_in_advance_to_book_extra_shifts: '%env(default::MAX_TIME_IN_ADVANCE_TO_BOOK_EXTRA_SHIFTS)%'
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

À Scopéli on définissait dans parameters.yml :

max_time_in_advance_to_book_extra_shifts: null

Mais le définir de la même façon dans .env ne fonctionne pas :

MAX_TIME_IN_ADVANCE_TO_BOOK_EXTRA_SHIFTS=null
# on se retrouve avec la chaîne "null" dans l'appli…

Il faut donc :

  • indiquer dans services.yaml qu'il y a un default:: vide (cette PR)
  • et commenter ou retirer complètement la variable d'environnement correspondante de son fichier .env.

@njean42 njean42 requested review from PaulienM and raphodn March 17, 2026 21:49
@njean42 njean42 added the Bug Fonctionnalité ne se comportant pas comme anticipé label Mar 17, 2026
@njean42 njean42 added this to the 2.0.0 (Symfony 4) milestone Mar 17, 2026
@PaulienM
Copy link
Copy Markdown
Collaborator

PaulienM commented Mar 30, 2026

Pour toutes ces variables qui font plein de choses et qui ne sont pas faciles à configurer je pense que l'idéal serait vraiment de stocker ça en base et d'avoir un formulaire de parametre d'instance. Ca me parait une vraie source d'erreur et de complexité de configurer correctement les choses (problème de type, oublis, pas de doc, ...)
Ca pourrait couter un tout petit peu de perf en allant chercher les choses en base plutot qu'en utilisant un .env préchargé dans un fichier php. Mais déjà on pourrait limiter ce problème avec du cache et on y gagnerait beaucoup a avoir un .env avec beaucoup moins de ligne.
Et c'est d'autant plus le cas pour les nouvelles épicerie pour lesquelles ça doit être extrêmement compliqué de config tout ça.

Et je me dit même que ça pourrait être la dernière maj de la version 1.x : une commande qui permet de migrer tous ces paramètres en BDD pour ne pas avoir besoin de se taper toute la migration parameters.yml -> .env (ou du moins de manière très limitée)

Qu'en pense tu ?

Désolé pour le pavé, c'est un peu hors sujet mais en même temps ça répondrait à cette PR

@njean42
Copy link
Copy Markdown
Collaborator Author

njean42 commented Apr 3, 2026

Ca pourrait couter un tout petit peu de perf en allant chercher les choses en base plutot qu'en utilisant un .env préchargé dans un fichier php. Mais déjà on pourrait limiter ce problème avec du cache

Oui, dans l'idée, aucun souci pour charger des paramètres depuis la base de données.
Comme tu dis, si ça ralentit vraiment (j'en doute), un cache est assez facile à mettre en place.

et on y gagnerait beaucoup a avoir un .env avec beaucoup moins de ligne

Oui !

Et c'est d'autant plus le cas pour les nouvelles épicerie pour lesquelles ça doit être extrêmement compliqué de config tout ça.

En effet, même si on a la version par défaut dans .env.dist, ça fait un peu peur tout ce qu'on peut paramétrer.

Et je me dit même que ça pourrait être la dernière maj de la version 1.x : une commande qui permet de migrer tous ces paramètres en BDD pour ne pas avoir besoin de se taper toute la migration parameters.yml -> .env (ou du moins de manière très limitée)
Qu'en pense tu ?

À Scopéli au moins, on voudrait pouvoir tout configurer avec un fichier .env.
Et même plus précisément, avec des variables d'environnement dans notre docker-compose.env.
(ping @sebastienbianco pour être sûr que je rapporte bien notre position)

L'idéal pour nous serait :

Fichier config/services.yaml Fichier .env (.env.dist) Fichier docker-compose.env
Définit les valeurs par défaut pour tous les paramètres de l'appli
(surchargeables par variable d'env)
Transfère les valeurs des variables d'environnement.
Vraiment utile ?
Surcharge uniquement les paramètres dont la valeur ne convient pas à la coopérative

(On est en train de se dire que le .env de l'appli ne devrait rien faire d'autres que reprendre les valeurs des variables d'environnement définies dans docker-compose.env.)

Désolé pour le pavé, c'est un peu hors sujet mais en même temps ça répondrait à cette PR

Aucun souci !
Très intéressant. Ça rejoint nos discussions (surtout à venir) autour des modes d'installation de l'appli, docker, docker-compose, paramètres…

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

Labels

Bug Fonctionnalité ne se comportant pas comme anticipé

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants