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.

Sign up for GitHub

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

Jump to bottom

add talk-recording container #2645

Merged
merged 21 commits into from
Jun 6, 2023
Merged

add talk-recording container #2645

merged 21 commits into from
Jun 6, 2023

Conversation

Zoey2936
Copy link
Collaborator

@Zoey2936 Zoey2936 commented May 31, 2023 •
edited by szaimen
Loading

For #2527

I've added recording.conf to the update script to see, when there are changes done to it, also backend.secret needs to be changed in the config and added to all other configs in aio
@szaimen If you want just add all other recording changes to this PR

@Zoey2936 Zoey2936 requested a review from szaimen May 31, 2023 17:08
@Zoey2936
Copy link
Collaborator Author

no idea why the spellcheck crashes while building

@szaimen
Copy link
Collaborator

szaimen commented Jun 1, 2023

no idea why the spellcheck crashes while building

It fails because we now have some self-hosted github action runners that run very fast into rate limitations with docker hub.

@szaimen
Copy link
Collaborator

szaimen commented Jun 1, 2023

@Zoey2936 Thank you very much for this!

Actually I was unfortunately not clear enough with this. I imagined to create a separate container for the recording server since based on @danxuliu it takes a lot of ressources to run. For smaller instances to still run the talk HPB and turnserver, I would split things up. Another solution would be to still deliver it with this one container and allowing to disable the recording server via environmental variable. WDYT? :)

@Zoey2936
Copy link
Collaborator Author

Zoey2936 commented Jun 1, 2023

it seems like recording depends on signaling, thats the reason why I added it here, but an env sounds good

@szaimen
Copy link
Collaborator

szaimen commented Jun 1, 2023

it seems like recording depends on signaling

Yes indeed so I would just limit the recording server to get chosen only if the talk hpb is enabled as well.

WDYT about a separate container? Or do you think that having everything in one container would be the better way here?

@szaimen
Copy link
Collaborator

szaimen commented Jun 1, 2023 •
edited
Loading

Actually since the recording server also needs many dependencies putting it into another container maybe makes it easier to maintain?

@wedreamer

This comment was marked as off-topic.

Sign in to view
@Zoey2936
Copy link
Collaborator Author

Zoey2936 commented Jun 1, 2023 •
edited
Loading

Actually since the recording server also needs many dependencies putting it into another container maybe makes it easier to maintain?

then lets move it to its own container

@szaimen szaimen added this to the next milestone Jun 1, 2023
@szaimen szaimen added 2. developing Work in progress enhancement New feature or request labels Jun 1, 2023
@Zoey2936 Zoey2936 force-pushed the recording branch 4 times, most recently from 4a96539 to dc18680 Compare June 2, 2023 15:43
@Zoey2936
Copy link
Collaborator Author

Zoey2936 commented Jun 2, 2023

done, I think

@Zoey2936
Copy link
Collaborator Author

Zoey2936 commented Jun 2, 2023

I've pushed talk v16.0.3 instead of .4 to check if the update workflow works and also to check if the config gets updated

@szaimen
Copy link
Collaborator

szaimen commented Jun 3, 2023

Thanks a lot Zoey! I'll have a look probably on monday! :)

@Zoey2936 Zoey2936 changed the title add recording/update to alpine v3.18 add recording container Jun 3, 2023
szaimen
szaimen reviewed Jun 5, 2023
View reviewed changes
Copy link
Collaborator

@szaimen szaimen left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thank you very much for tackling this! :)

Some comments

.github/workflows/recording-update.yml Outdated Show resolved Hide resolved
.github/workflows/recording-update.yml Outdated Show resolved Hide resolved
Containers/recording/Dockerfile Outdated Show resolved Hide resolved
Containers/recording/Dockerfile Outdated Show resolved Hide resolved
Containers/recording/Dockerfile Outdated Show resolved Hide resolved
Containers/recording/start.sh Outdated Show resolved Hide resolved
Containers/apache/Caddyfile Outdated Show resolved Hide resolved
Containers/recording/Dockerfile Outdated Show resolved Hide resolved
Containers/recording/recording.conf Outdated Show resolved Hide resolved
Containers/recording/start.sh Outdated Show resolved Hide resolved
szaimen
szaimen reviewed Jun 5, 2023
View reviewed changes
Copy link
Collaborator

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

Some comments

@szaimen szaimen mentioned this pull request Jun 5, 2023
Merged
@szaimen szaimen added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jun 5, 2023
@szaimen szaimen marked this pull request as ready for review June 5, 2023 11:38
Zoey2936 and others added 12 commits June 5, 2023 15:26
@Zoey2936
fix username
51f49ca 
Signed-off-by: Zoey <zoey@z0ey.de>
@szaimen @Zoey2936
add dependabot updates
b24d3b7 
Signed-off-by: Simon L <szaimen@e.mail.de>
Signed-off-by: Zoey <zoey@z0ey.de>
@Zoey2936
fix workflow path
9e7ac84 
Signed-off-by: Zoey <zoey@z0ey.de>
@Zoey2936
fix latest commit
1509bdf 
Signed-off-by: Zoey <zoey@z0ey.de>
@Zoey2936
use /tmp as homedirectory for recording user
698d3c5 
Signed-off-by: Zoey <zoey@z0ey.de>
@Zoey2936
add workflow for signaling conf
c716438 
Signed-off-by: Zoey <zoey@z0ey.de>
@Zoey2936
improve clone command (1/2)
7f2ed01 
Signed-off-by: Zoey <zoey@z0ey.de>
@Zoey2936
improve clone command (2/2)
dcb51f9 
Signed-off-by: Zoey <zoey@z0ey.de>
@Zoey2936
change recording conf
bb6bdb5 
Signed-off-by: Zoey <zoey@z0ey.de>
@Zoey2936
openssl is required while building
d4f376b 
Signed-off-by: Zoey <zoey@z0ey.de>
@Zoey2936
combine workflows
daafb02 
Signed-off-by: Zoey <zoey@z0ey.de>
@Zoey2936
fix docker lint
1db3ad1 
Signed-off-by: Zoey <zoey@z0ey.de>
@Zoey2936
add firefox/geckodriver/xvfb
6a0a6ae 
Signed-off-by: Zoey <zoey@z0ey.de>
@Zoey2936
remove ip of talk container
6244a51 
Signed-off-by: Zoey <zoey@z0ey.de>
@Zoey2936 Zoey2936 requested a review from szaimen June 5, 2023 13:49
@szaimen szaimen mentioned this pull request Jun 5, 2023
Closed
7 tasks
szaimen
szaimen reviewed Jun 5, 2023
View reviewed changes
Containers/talk-recording/start.sh Outdated Show resolved Hide resolved
Containers/talk-recording/start.sh Outdated Show resolved Hide resolved
@szaimen
Adjust secrets
e945e30 
Signed-off-by: Simon L <szaimen@e.mail.de>
@szaimen
adjust ids
7931ec8 
Signed-off-by: Simon L <szaimen@e.mail.de>
szaimen
szaimen reviewed Jun 5, 2023
View reviewed changes
Containers/apache/Caddyfile Outdated Show resolved Hide resolved
@szaimen
dont expose it publicly
8e52a73 
Signed-off-by: Simon L <szaimen@e.mail.de>

Signed-off-by: Simon L. <szaimen@e.mail.de>
@szaimen szaimen changed the title add recording container add talk-recording container Jun 5, 2023
@szaimen szaimen mentioned this pull request Jun 6, 2023
Closed
30 tasks
@szaimen szaimen merged commit 216f8a1 into main Jun 6, 2023
@delete-merged-branch delete-merged-branch bot deleted the recording branch June 6, 2023 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@szaimen szaimen szaimen approved these changes

Assignees
No one assigned
Labels
3. to review Waiting for reviews enhancement New feature or request
Projects
None yet
Milestone
v6.1.0
Development

Successfully merging this pull request may close these issues.
3 participants
@Zoey2936 @szaimen @wedreamer