search

istresearch / scrapy-cluster

This Scrapy project uses Redis and Kafka to create a distributed on demand scraping cluster.
http://scrapy-cluster.readthedocs.io/
MIT License
1.18k stars 323 forks source link

Enable docker-compose build #142

Closed topiaruss closed 6 years ago

topiaruss commented 6 years ago

This allows a single docker-compose build to rebuild local images. If this is accepted, I'll do similar for py2.7

madisonb commented 6 years ago

I am curious as to why Travis seems stuck on this PR... anyways I have a number of comments:

1) The build: directive is great, however it then forces someone to use python3 or python2. What is your plan here to support both? It seems to me like that is better off in an docker-compose.override.yaml file, since it is custom to your particular setup, in which case I would not want to commit it here.

2) I don't see the reason behind moving the COPY command into /usr/src/app/utils when it gets deleted a couple lines later.

3) The requirements.txt file at the top of this project is deprecated and should not be used to build containers. It creates excess bloat by installing packages that are not needed by the individual components. Instead we should use only the bare minimum, which reverts your changes back to the original xxx/requirements.txt files inside each component.

Overall, I appreciate the PR but am going to recommend closing it, as I don't plan on merging any of the changes propsed here into the codebase, due to the reasons outlined above.

topiaruss commented 6 years ago

All good comments.

  1. Fair enough. Override is a good suggestion. I'll see how that would look.
  2. The COPY is to support the requirements.txt dependencies. Once PIP is done the dir can be deleted. It's exactly analogous to the code it replaces, but it works in this environment.
  3. OK. Understood. This was a misunderstanding. I support the smaller local requirements.

I'll close the request.

How can I help towards the next release?

madisonb commented 6 years ago

@topiaruss If you are interested in helping with the next release, lets move the conversation over to Gitter