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

updated dockerfile dependencies to come from setup.py #1165

Merged
merged 1 commit into from
May 6, 2020

Conversation

jonnekaunisto
Copy link
Contributor

No description provided.

@coveralls
Copy link

coveralls commented Apr 28, 2020

Coverage Status

Coverage remained the same at 64.819% when pulling 503e670 on jonnekaunisto:docker into 850a40e on Zulko:master.

@tburrows13
Copy link
Collaborator

Hey thanks for this change. I was wondering if you'd actually tested moviepy on docker recently? I'd noticed that the dockerfile seemed out of date, but because I'd never used docker, I didn't feel qualified to fix it. If this change is all that is needed to make it work, that would be great.

@jonnekaunisto
Copy link
Contributor Author

Yeah this commit mostly just keeps the dependencies updated, but it would probably make more sense to use the alpine image instead of the debian image for the dockerfile, since alpine is basically the standard image to use.

Updating to alpine would require mostly just finding the packages from the alpine packages list, which sometimes have slightly different names than debian ones.

I'm happy to take a crack at that if it makes sense.

@tburrows13
Copy link
Collaborator

I've got no opinion on alpine vs debian/ubuntu. If you think that it would be an improvement, then go for it, but from my (limited) research, it seems like debian is a perfectly valid choice.

@tburrows13 tburrows13 added bug-fix For PRs and issues solving bugs. docker Anything to do with running MoviePy in Docker containers. labels Apr 29, 2020
@jonnekaunisto
Copy link
Contributor Author

Yes debian is perfectly fine. The dockerfile with these configurations fails for one test, which is test_matplotlib.

Error is: _tkinter.TclError: no display name and no $DISPLAY environment variable

Which doesn't seem too bad since the docker container doesn't have any display

@tburrows13 tburrows13 mentioned this pull request May 4, 2020
@tburrows13
Copy link
Collaborator

Is this PR finished? Are you happy for it to be merged?

@jonnekaunisto
Copy link
Contributor Author

Yes, please merge this pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-fix For PRs and issues solving bugs. docker Anything to do with running MoviePy in Docker containers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants