Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jicofo & jvb: Fix OCTO + SCTP behaviour #1791

Merged
merged 5 commits into from
May 7, 2024

Conversation

spijet
Copy link
Contributor

@spijet spijet commented Apr 24, 2024

Hello everyone!

As a follow-up to #1787, here's the PR that:

  1. Removes the bogus cont-init check;
  2. Enables SCTP by default in Jicofo config template.

I'll leave the PR as a draft for now, until @saghul confirms that all that needed to be done is done. :)

@saghul
Copy link
Member

saghul commented Apr 24, 2024

@damencho can you PTAL too?

@damencho
Copy link
Member

@damencho can you PTAL too?

I asked @bgrozev and he said it is fine :)

@spijet spijet marked this pull request as ready for review May 2, 2024 08:32
@spijet
Copy link
Contributor Author

spijet commented May 2, 2024

I updated the config template according to @saghul's comment and switched this PR from draft to open. Please let me know if there's anything else that needs to be added. :)

@@ -4,10 +4,10 @@
{{ $AUTH_TYPE := .Env.AUTH_TYPE | default "internal" -}}
{{ $JICOFO_AUTH_TYPE := .Env.JICOFO_AUTH_TYPE | default $AUTH_TYPE -}}
{{ $JICOFO_AUTH_LIFETIME := .Env.JICOFO_AUTH_LIFETIME | default "24 hours" -}}
{{ $ENABLE_SCTP := .Env.ENABLE_SCTP | default "0" | toBool -}}
{{ $ENABLE_SCTP := .Env.ENABLE_SCTP | default "1" | toBool -}}
Copy link
Member

Choose a reason for hiding this comment

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

Upon further inspection I think we should keep this defaulting to disabled, for the time being. With the rest of the changes inplace you'll still be able to enable OCTO over SCTP though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in b2ecc27. :)

@saghul saghul merged commit 76ffaa7 into jitsi:master May 7, 2024
1 check passed
@spijet spijet deleted the fixes/20240424-jvb-octo branch May 8, 2024 07:10
@spijet
Copy link
Contributor Author

spijet commented May 8, 2024

Yay! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants