search

shahargalon / book-store

0 stars 0 forks source link

Docker #2

Closed shahargalon closed 4 years ago

shahargalon commented 4 years ago

Hi, I changed the python version on the Pipfile and run this on my machine and its work

image

now I am tring to install everything on the official python docker and run it locally (right now i am experience some issues) and when I will manage to do this I will right the step on the Dockerfile

nirgn975 commented 4 years ago

@shahargalon

  1. Change the PR name to be the same as the issue name.
  2. Put me on "Reviewers", because otherwise I don't get any notification about this PR (you can also mention me with @ in the message, like I did above).
shahargalon commented 4 years ago

Hi, I generate the requirements.txt and wrote the Dockerfile (that lays on the official python Docker). the image is 920MB. and its run perffect image

nirgn975 commented 4 years ago

Looking good.

  1. in future PRs (Pull Requests), there is a "Linked issues" at the right menu (almost at the bottom of the menu), choose the issue that this PR is related to. This will create a link between the PR and the Issue (in the issue page and in the PR page, so we can go between them without searching). Also, it'll close the issue automatically once we'll close the PR (when we'll merge it). No need to do this now, I did it this time.
  2. Mark the tasks you finished in the issue as done.
  3. After reading a bit, we have 2 options: 3.1 We need to create the CI script (GitHub action workflow) so every time someone does a PR (Pull Request) with a change to the Pipfile it'll auto generate a new requirements.txt file. 3.2 I read that there is a way to use pipenv and tells it to install the dependencies (packages our project depend on, the ones in the Pipfile) right in the system (and not in the virtual environment). This way, we don't need the requirements.txt file anymore, we only need to install pipenv in the Docker image (don't shell into the virtual environment), and then to install the dependencies (look at this: https://stackoverflow.com/a/49705601/2040160).

So, which way to do want to go related to 3 ? it's your show, choose.

shahargalon commented 4 years ago

I wrote another Docker file who use the pipenv and the Pipfile (the size is little bit more then the fisrt one 965MB because the pipenv). do you want me do remove the requirements.txt file? and also replace the Dockerfile to the new one on the same brunch?

nirgn975 commented 4 years ago

@shahargalon Yeah. I mean, if you did that, we don't need the requirements.txt anymore.. And we'll use the new Dockerfile - no reason to use the one with the requirements.txt..

This branch and PR, should solve all the tasks in the issue, I don't care how, the implementation details are your problem (:

When you finish all the things, and the PR is ready to be merged, tag me and write that this PR is ready to review. I'll review it, and if everything is good I'll merge it to master and open a new issue (if there is something to fix, I'll write it here, when I review the code).

shahargalon commented 4 years ago

Hi @nirgn975 , I finished the Dockerfile, the PR should ready to be merged Please review it and let me know if all good

thanks

shahargalon commented 4 years ago

Hi @nirgn975 can you take a look on the Pipfile I added a note in this file on some issue that I cant understand..