fishjam-dev / fishjam

General purpose media server. Supports WebRTC, HLS, RTSP, SIP
https://fishjam-dev.github.io/fishjam-docs/
Apache License 2.0
197 stars 12 forks source link

[RTC-345] Introduce JF dedicated env vars for running in a distributed mode #92

Closed mickel8 closed 11 months ago

mickel8 commented 12 months ago

This PR introduces our own env variables for running JF in a cluster.

When we run erlang/elixir locally, we can setup distribution using flags like --sname --name --cookie etc. However, when we make a release, we are provided with special env variables e.g. RELEASE_NODE RELEASE_COOKIE etc. which results in multiple configuration options depending on environment we run Jellyfish in.

Intoducing our own env vars we:

codecov[bot] commented 12 months ago

Codecov Report

Merging #92 (27f163c) into main (5888e33) will decrease coverage by 0.90%. Report is 1 commits behind head on main. The diff coverage is 60.71%.

@@            Coverage Diff             @@
##             main      #92      +/-   ##
==========================================
- Coverage   84.27%   83.37%   -0.90%     
==========================================
  Files          44       44              
  Lines         744      764      +20     
==========================================
+ Hits          627      637      +10     
- Misses        117      127      +10     
Files Coverage Δ
lib/jellyfish/config_reader.ex 100.00% <100.00%> (ø)
lib/jellyfish/application.ex 36.84% <21.42%> (-43.16%) :arrow_down:

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5888e33...27f163c. Read the comment docs.

mickel8 commented 11 months ago

JF_DIST_ENABLED -> no change JF_NODE_NAME -> JF_DIST_NODE_NAME JF_COOKIE -> JF_DIST_COOKIE JF_NODES -> JF_DIST_NODES

@sgfn Can do that but what about WebRTC or RTSP? I think we should prefix them too

sgfn commented 11 months ago

@sgfn Can do that but what about WebRTC or RTSP? I think we should prefix them too

Yeah, I've no problem with that, we should go for it

roznawsk commented 11 months ago

+1 from for comments from @sgfn - other than that LGTM

mickel8 commented 11 months ago

Approved, with one small note:

Regarding the .env.sample file: I'm still firmly against making the core Jellyfish documentation spread out in two or more >places -- could you just confirm we will duplicate all of this info in the docs, and not have the docs say: "for more info, >refer to the .env.sample file"?

Sure, I will revisit this once I write official docs