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

Add docker support #242

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add docker support #242

wants to merge 3 commits into from

Conversation

volkan
Copy link

@volkan volkan commented Nov 21, 2020

Without any special installation we will be able to download a book with the following command

docker-compose run safaribooks python3 safaribooks.py --cred "account_mail@mail.com:password01" XXXXXXXXXXXXX

@rufanov
Copy link

rufanov commented Jan 17, 2021

I really don't think it's convenient way to run this project. Yes, it's possible to docker'ize everything in the world, but docker designed for horizontal application scaling and CI. It's overcomplicated to use it for running just simple "single-use" python scripts.

Also both docker and docker-compose project do require python to be installed - so this doesn't solve anything at all.

@volkan
Copy link
Author

volkan commented Jan 17, 2021

I really don't think it's convenient way to run this project. Yes, it's possible to docker'ize everything in the world, but docker designed for horizontal application scaling and CI. It's overcomplicated to use it for running just simple "single-use" python scripts.

Also both docker and docker-compose project do require python to be installed - so this doesn't solve anything at all.

Frankly, I was having a problem with my python installation, I used docker rather than fixing the python installation. Then, I thought, it maybe can help someone else too and I did MR.

@siddhantvirus
Copy link

Honestly, I was revisiting this project , and I too was considering to dockerize the project and add a volume mount to make sure that the books get saved in our host machine.
The whole point of using docker, in this case, is to ensure that there are no install steps required.
I agree we do need python for docker-compose, but what if we just skip docker-compose and add the volume mount directly to the docker run command? it's harmless in my opinion.
Plus your requirements file mentions python 3.6 specifically, and those with a different version might not want the entire hassle of multiple versions or using venv

In any case, kudos for making this project! And I'm sure people would use it only for educational purposes :'3

Copy link

@PaulDMooney PaulDMooney left a comment

Choose a reason for hiding this comment

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

I recommend adding a .dockerignore file. It can contain the following:

.git/
.gitignore
.README.md
Dockerfile
docker-compose.yml
LICENSE.md

Dockerfile Show resolved Hide resolved
docker-compose.yml Show resolved Hide resolved
@PaulDMooney
Copy link

I'm on a Mac, using Docker Desktop. I don't believe I have the correct version of Python to run this project so yes, this docker option is nice. And no, I don't think being able to run docker-compose means I have the correct Python on my host-OS already since docker is running in a VM (though I may be wrong).

@ukazap ukazap mentioned this pull request Jun 23, 2023
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.

4 participants