Config : migration syntaxe :system

[AntoineAugusti]

On a un warning pour la syntaxe actuelle, je la migre donc.

:http configuration containing {:system, env_var} tuples for TransportWeb.Endpoint is deprecated.

Configuration with deprecated values:

    config :transport, TransportWeb.Endpoint,
      http: [
        port: {:system, "PORT"}
      ]

Move this configuration into config/runtime.exs and replace the {:system, env_var} tuples
with System.get_env/1 function calls:

    config :transport, TransportWeb.Endpoint,
      http: [
        port: System.get_env("PORT")
      ]

Il ne me semble pas nécessaire que ceci soit placé dans runtime.exs.

[thbar]

Je commenterai demain, il y a des choses auxquelles il faudra faire attention là dessus.

[AntoineAugusti]

:port est documenté comme pouvant être géré au runtime, c'est pourquoi je pensais que ce changement était suffisant. Curieux d'avoir ton avis !

[thbar]

:port est documenté comme
pouvant être géré au runtime, c'est pourquoi je pensais que ce changement était suffisant. Curieux d'avoir ton avis !

Oui tout à fait, c'est gérable au runtime et c'est ce qui était fait avant la PR.

Le warning recommande:

Move this configuration into config/runtime.exs

pour la raison suivante: si tu le mets dans prod.exs comme fait sur la PR, et bien ça ne sera pas géré au runtime, mais à compile time, et c'est ce que le warning vise justement à éviter.

La seule vraie méthode recommandée pour faire cela est de suivre le warning et de déplacer le tout dans runtime.exs, ou bien (dans l'interim, et c'est pas bloqué en dur) de le laisser comme avant, pour que le warning soit là pour nous rappeler qu'il faut faire ce changement !

[thbar]

À ce stade je recommande de créer un ticket avec le warning dedans pour traiter ça un peu plus tard (ça ne va pas casser), et laisser le warning, c'est ce qui sera le plus simple.

(ou bien tu peux te chauffer dessus et le mettre dans runtime comme le warning le recommande, mais ça peut attendre à mon avis)

[AntoineAugusti]

@thbar Je me chauffre, j'ai commencé une PR 😅

J'ai fait du nettoyage dans le fichier de l'endpoint (la partie avec load_from_system_env), ceci semble avoir été utile dans de précédentes versions de Phoenix mais n'est plus d'actualité.

[AntoineAugusti]

C'est dans le bloc config_env() == :prod

[thbar]

Ça paraît bon, surtout conjointement à:

transport-site/Dockerfile

Lines 14 to 15 in c471d43

	ENV PORT 8080
	ENV MIX_ENV prod

À terme il faudra qu'on aie une configuration plus fortement typée ou avec des fetch_env! mais ça peut attendre (sinon on risque d'avoir des salts à nil ou autres choses dans ce genre, probablement vérifié mais pas tout le temps).

J'ai enlevé l'auto-merge pour préférer un déploiement surveillé.

Tu as déjà testé sur prochainement ?

[AntoineAugusti]

@thbar

Tu as déjà testé sur prochainement ?

Je viens de faire un test pour de vrai sur prochainement ✅ #safety 🦺

🚢

[thbar]

Je viens de faire un test pour de vrai sur prochainement ✅ #safety 🦺

Impec !