Skip to content
This repository has been archived by the owner on Oct 2, 2023. It is now read-only.

Refactors file pipeline to add queue #60

Closed
wants to merge 32 commits into from
Closed

Refactors file pipeline to add queue #60

wants to merge 32 commits into from

Conversation

cuducos
Copy link
Contributor

@cuducos cuducos commented Mar 18, 2020

Como discutido em #59, esse PR inicia uma tentativa de utilizar filas para reduzir o uso de memória – mas ainda é um rascunho, algumas coisas que sei que faltam:

  • Flag para baixar os arquivos pelo pipeline ou pela fila
  • Escolher o "modelo" no banco de dados a partir do tipo de item do objeto coletado no Scrapy
  • Salvar checksum
  • Testar se não quebrei nada no refactor do ExtractFileContentPipeline (tentei deixar o código mais legível, usando variáveis com nomes significativos para humanos e menos índices numéricos)
  • Configurar RabbitMQ em produção
  • Configurar RabbitMQ em desenvolvimento
  • Testar se a chamada no send do ator a partir do Scrapy funciona (sendo que a tarefa depende do Django; creio que sim, pois é o worker quem executa mas preciso testar)
  • Criar o worker com acesso ao ORM do Django (alguém já usou o django_dramatiq?)

E mais umas coisas que eu não sei ainda : )

Coisas importantes, agora:

  • Abri esse PR para ser mergeado na branch em salva-diarios (e não em master), era essa a ideia?
  • Claro que estou curioso para qualquer feedback até aqui, mas especialmente:
    • O que acham do # TODO que deixei no datasets/tasks.py (linha 16)?
    • O que acham do # TODO que deixei no scraper/pipelines.py (linha 33)?

Copy link
Contributor

@anapaulagomes anapaulagomes left a comment

Choose a reason for hiding this comment

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

Cuducos, obrigada pela sua primeira contribuição! 🎉

Abri esse PR para ser mergeado na branch em salva-diarios (e não em master), era essa a ideia?

Acredito que seria interessante ter uma feature flag aqui pra manter o funcionamento dos spiders que ainda não possuem dados sendo salvos no banco. Acho bom manter pra quem quiser rodar os spiders localmente fora do comando crawl e ainda assim ter o conteúdo dos arquivos. O que você acha?
Eu, por exemplo, ainda rodo os coletores locais pra fazer pesquisa nos arquivos e atualizar o Kaggle. Ainda é um passo manual que ainda está longe de ser automatizado.

De um jeito ou de outro, esse PR poderia ser mergeado na master mesmo. O salva-diarios é que não pode ser mergeado antes. 🙏

setup.cfg Show resolved Hide resolved
datasets/tasks.py Outdated Show resolved Hide resolved
datasets/tasks.py Outdated Show resolved Hide resolved
datasets/tests/test_tasks.py Outdated Show resolved Hide resolved
scraper/pipelines.py Show resolved Hide resolved
scraper/pipelines.py Outdated Show resolved Hide resolved
scraper/pipelines.py Outdated Show resolved Hide resolved
@anapaulagomes anapaulagomes added this to In progress in MVP - banco de dados Mar 20, 2020
@anapaulagomes anapaulagomes changed the base branch from salva-diarios to master March 21, 2020 16:28
@cuducos
Copy link
Contributor Author

cuducos commented Mar 28, 2020

Atualizando a lista de afazeres do post inicial com outros items:

UPDATE Movi para o post inicial.

@cuducos
Copy link
Contributor Author

cuducos commented Mar 28, 2020

Pronto! Avencei um pouco e agora já não considero mais draft, yay!

Para testarmos em ambiente de staging ou produção no Heroku, precisamos que alguém com acesso ative o CloudAMQP. Deve funcionar normalmente, pois adicionei o worker no Procfile e usei a variável de ambiente padrão do Heroku no core/settings.py.

@cuducos cuducos marked this pull request as ready for review March 28, 2020 22:31
Copy link
Contributor

@anapaulagomes anapaulagomes left a comment

Choose a reason for hiding this comment

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

Caramba, essa é uma bela feature pra MQ! 🎉 Deixei alguns comentários.
Não pude deployar pra staging pq tua branch não pertence a esse repo mas adicionei o addon e te coloquei como colaborador. Fiquei com preguiça de configurar uma review app, hoje tá corrido (malz ae haha).

README.md Show resolved Hide resolved
core/settings.py Outdated Show resolved Hide resolved
datasets/tasks.py Outdated Show resolved Hide resolved
datasets/tasks.py Show resolved Hide resolved
scraper/pipelines.py Show resolved Hide resolved
scraper/pipelines.py Outdated Show resolved Hide resolved
cuducos and others added 4 commits March 29, 2020 13:41
Apenas direciono ao site oficial pois as instruções de instalação variam
muito de acordo com o sistema operacional (sendo que antes disso a
própria documentação recomenda Docker).
Co-Authored-By: Ana Paula Gomes <apgomes88@gmail.com>
Co-Authored-By: Ana Paula Gomes <apgomes88@gmail.com>
@anapaulagomes anapaulagomes temporarily deployed to maria-quiter-queue-for-qdhhodg March 29, 2020 19:14 Inactive
Copy link
Contributor

@anapaulagomes anapaulagomes left a comment

Choose a reason for hiding this comment

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

Boa, @cuducos! 🏆
Amanhã vou deployar para staging, assim podemos testar antes de mergear.

@anapaulagomes anapaulagomes temporarily deployed to staging-maria-quiteria March 29, 2020 19:23 Inactive
datasets/tasks.py Outdated Show resolved Hide resolved
Copy link
Contributor

@anapaulagomes anapaulagomes left a comment

Choose a reason for hiding this comment

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

Depois de muitas lágrimas (risos), consegui identificar os motivos pelos quais não estava funcionando:

  • Precisamos ter configurado o limite do broker (BROKER_POOL_LIMIT=1) e o número de processos do worker - assim respeitamos os limites do plano gratuito e evitamos o famigerado connection refused
  • No método que salvava os diários estávamos aceitando o valor padrão dos itens (um conteúdo vazio) mas estávamos checando se ele é nulo ou não pra atualizar. Por isso nenhum diário tinha seu conteúdo atualizado.
  • No pipeline não estávamos retornando o item de volta

Fora isso:

  • Adicionei as atas ao rolê, já que o pipeline quebra sem suporte a elas

Commitei essas mudanças nessa branch de testes (baseada na tua). Lá dá pra ver as diferenças.

No momento estou rodando toda a coleta de diários e atas. O pico de mensagens na fila chegou a 11 mil. Três filas, 7 conexões abertas (máximo de 20 conexões). O consumo de memória no worker se manteve em 128MB - not bad.

Infelizmente uma parte ainda teremos que resolver: o arquivo não é encontrado (acredito que pelo fato do heroku não persisti-los), logo não é lido. :(

scraper/settings.py Outdated Show resolved Hide resolved
if ASYNC_FILE_PROCESSING:
content_from_file.send(**kwargs)
else:
item["file_content"] = content_from_file(**kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

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

Na real, não está funcionando como esperado. Retorna um gerador e nada acontece feijoada. Primeiro pq temos que retornar o item e lá em cima retornamos None e embaixo um generator. Além disso, não dá pra retornar vários itens (o yield está dentro do loop). Consegui pôr pra funcionar assim:

    def item_completed(self, results, item, info):
        if not results:
            return item

        content_from_file_urls = []
        for result in results:
            ok, file_info = result
            if not ok:
                continue

            kwargs = {
                "item_name": item.__class__.__name__,
                "url": file_info["url"],
                "path": f"{FILES_STORE}{file_info['path']}",
                "checksum": file_info["checksum"],
                "save_to_db": ASYNC_FILE_PROCESSING,
                "keep_file": KEEP_FILES,
            }
            if ASYNC_FILE_PROCESSING:
                content_from_file.send(**kwargs)
            else:
                content_from_file_urls.append(content_from_file(**kwargs))
        item["file_content"] = content_from_file_urls
        return item

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Putz! Falha minha. Como results era plural, meu instinto foi criar um gerador, mas acho que (a) não faz sentido, e, com certeza, (b) justifica o erro.

Procfile Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
datasets/tasks.py Outdated Show resolved Hide resolved
datasets/tasks.py Outdated Show resolved Hide resolved
cuducos and others added 8 commits April 1, 2020 12:03
Co-Authored-By: Ana Paula Gomes <apgomes88@gmail.com>
Co-Authored-By: Ana Paula Gomes <apgomes88@gmail.com>
Co-Authored-By: Ana Paula Gomes <apgomes88@gmail.com>
Co-Authored-By: Ana Paula Gomes <apgomes88@gmail.com>
Assim ele é reconhecido globalmente

Co-Authored-By: Ana Paula Gomes <apgomes88@gmail.com>
Assim ele é reconhecido globalmente

Co-Authored-By: Ana Paula Gomes <apgomes88@gmail.com>
Co-Authored-By: Ana Paula Gomes <apgomes88@gmail.com>
Co-Authored-By: Ana Paula Gomes <apgomes88@gmail.com>
@cuducos
Copy link
Contributor Author

cuducos commented Apr 1, 2020

Infelizmente uma parte ainda teremos que resolver: o arquivo não é encontrado (acredito que pelo fato do heroku não persisti-los), logo não é lido. :(

Poder ser por isso, mas mesmo se persistisse: são processos diferentes, ou seja, Docker containers diferentes. Precisamos de um volume compartilhado.

Acho que se a gente usar um S3, fica barato, ou quase de graça. Vai ser só uma escrita, uma leitura por arquivo (depois apagamos). O que acha?

@anapaulagomes
Copy link
Contributor

Fechando esse PR em favor do #101. Obrigada, @cuducos! 🏅

MVP - banco de dados automation moved this from In progress to Done Apr 20, 2020
@cuducos cuducos deleted the queue-for-tika branch January 1, 2021 16:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants