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

talk recording - prefer local installation #3569

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Containers/talk-recording/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ RUN set -ex; \
echo "root:$(openssl rand -base64 12)" | chpasswd; \
git clone --recursive https://github.com/nextcloud/spreed --depth=1 --single-branch --branch "$RECORDING_VERSION" /src; \
mv -v /src/recording/pyproject.toml /src/recording/src/pyproject.toml; \
python3 -m pip install --no-cache-dir /src/recording/src; \
python3 -m pip install --no-cache-dir --no-index --find-links=file:///local/dir /src/recording/src; \
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I guess I need to adjust file:///local/dir ? question is to what but I suppose that is what I need to figure out myself 😅

Copy link
Member

Choose a reason for hiding this comment

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

Current dir I guess?

Copy link
Member

Choose a reason for hiding this comment

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

Speaking of which, the recording backend was moved to it's own repo:
https://github.com/nextcloud/nextcloud-talk-recording

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

right, thanks for the heads-up!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

@szaimen szaimen Oct 20, 2023

Choose a reason for hiding this comment

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

Actuallly we are installing the packages from the pyproject file so I am not sure we actually need the above code changes here? also https://github.com/nextcloud-releases/all-in-one/actions/runs/6563947941/job/17832323096 seems to show that. @danxuliu please clarify :)

Also see https://github.com/nextcloud/nextcloud-talk-recording/blob/main/pyproject.toml

Copy link
Member

Choose a reason for hiding this comment

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

Using --no-index disables getting the packages from PyPi. However, it does it not only for the package being installed, but also its dependencies.

However, to locally install a package it is enough to use an identifier that hints at being a local directory. So installing from /src/recording/src* should be fine; to err on the side of caution you could prepend file://, but it should not be necessary.

The problem in the recording server documentation was that it was installed from a relative directory named nextcloud-talk-recording, so pip tried to fetch it from PyPi too.

*Out of curiosity, why do you move the pyproject.toml file inside the src directory? It should be enough to keep it in the original place and install /src/recording, or what am I missing? :-)

rm -rf /src; \
touch /etc/recording.conf; \
chown recording:recording -R \
Expand Down