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

Adiciona a extensão correspondente no arquivo que está sendo baixado #1045

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

AlexJBSilva
Copy link
Contributor

Descrição

Adiciona a extensão correspondente no arquivo que está sendo baixado, quando o scrapy não consegue achar a extensão pela url de download. Essa parte é uma implementação (com ajustes) do código sugerido pelo @ogecece em #946 (review) que utiliza a biblioteca filetype.

Para resolver o problema de "nos casos onde forçamos a detecção da extensão, o arquivo sempre seria baixado novamente de forma desnecessária", o método stat_file foi sobrescrito, com a inclusão da busca por um arquivo com extensão quando o scrapy não consegue achar a extensão pela url de download para comparar se o arquivo já foi baixado.
Resolve #819
O PR #946 pode ser encerrado.

Checklist - Novo spider

  • Você executou uma extração completa do spider localmente e os dados retornados estavam corretos.
  • Você executou uma extração por período (start_date e end_date definidos) ao menos uma vez e os dados retornados estavam corretos.
  • Você verificou que não existe nenhum erro nos logs (log_count/ERROR igual a zero).
  • Você definiu o atributo de classe start_date no seu spider com a data do Diário Oficial mais antigo disponível na página da cidade.
  • Você garantiu que todos os campos que poderiam ser extraídos foram extraídos de acordo com a documentação.

está sendo baixado, quando o scrapy não consegue achar a extensão pela
url de download. Essa parte é uma implementação (com ajustes) do código
sugerido pelo @ogecece  em okfn-brasil#946 (review) .
Para resolver o problema de "nos casos onde forçamos a detecção da extensão, o arquivo sempre seria baixado novamente de forma desnecessária",
o método `stat_file` foi sobrescrito, com a inclusão da busca por um
arquivo com extensão quando o scrapy não consegue achar a extensão pela
url de download para comparar se o arquivo já foi baixado.
Resolve okfn-brasil#819
@ogecece
Copy link
Member

ogecece commented Apr 3, 2024

@rennerocha tu consegue dar uma olhada nesse PR? Eu comecei a revisar hoje com a @trevineju e me pareceu que o caminho que o @AlexJBSilva tá seguindo tá correto, mas precisaríamos que também fosse implementado o S3FilesStore.

Pensa em outro caminho de implementação?

@rennerocha
Copy link
Member

@ogecece eu já tinha dado uma olhada nisso a um tempo atrás. Vou ver com calma amanhã e sugiro algo (ou não)

@ogecece
Copy link
Member

ogecece commented Apr 17, 2024

@rennerocha conseguisse olhar?

@rennerocha
Copy link
Member

@rennerocha conseguisse olhar?

Estou analisando agora. Aparentemente é esse o caminho. Estou analisando para ver se é possível deixar mais simples, evitando mudar demais o padrão do Scrapy. Porém como você mesmo comentou, essa alteração só funciona para arquivos armazenados localmente. É necessário uma implementação do stat_file para o S3FilesStore, ou senão não irá funcionar em produção.

@rennerocha
Copy link
Member

rennerocha commented Apr 19, 2024

Acabei de analisar com mais calma e encontrei um problema com a solução proposta que pode não afetar o armazenamento do arquivo, mas cria uma inconsistência de resultados. Na chave path, retornamos o caminho onde o arquivo está armazenado. Porém se executarmos mais de uma vez para a mesma data, as execuções seguintes corretamente não fazem o download, adicionando o status com uptodate, mas o path fica sem a extensão. Isso ocorre porque como o arquivo não precisa ser baixado na segunda execução, não é necessário definir o filepath (que adiciona a extensão).

Primeira execução:

{'date': '2024-04-18',
 'file_urls': ['https://diariooficial.santos.sp.gov.br/edicoes/inicio/download/2024-04-18'],
 'files': [{'checksum': '09d4708a4d58287e88303f1719f88ba5',
            'path': '3548500/2024-04-18/52662a6f24c42ba582c341db6cc345eb9d78fe67.pdf',
            'status': 'downloaded',
            'url': 'https://diariooficial.santos.sp.gov.br/edicoes/inicio/download/2024-04-18'}],
 'is_extra_edition': False,
 'power': 'executive_legislative',
 'scraped_at': '2024-04-19T00:18:25.031138Z',
 'territory_id': '3548500'}

Segunda execução:

{'date': '2024-04-18',
 'file_urls': ['https://diariooficial.santos.sp.gov.br/edicoes/inicio/download/2024-04-18'],
 'files': [{'checksum': '09d4708a4d58287e88303f1719f88ba5',
            'path': '3548500/2024-04-18/52662a6f24c42ba582c341db6cc345eb9d78fe67',
            'status': 'uptodate',
            'url': 'https://diariooficial.santos.sp.gov.br/edicoes/inicio/download/2024-04-18'}],
 'is_extra_edition': False,
 'power': 'executive_legislative',
 'scraped_at': '2024-04-19T00:18:36.238569Z',
 'territory_id': '3548500'}

O que eu comentei acima pode ser solucionado provavelmente alterando o QueridoDiarioFSFilesStore e também a mesma versão do S3FileStore que precisa ser implementada.

@rennerocha
Copy link
Member

Estou achando essa solução com a implementação de FilesStores customizados mais complicada do que precisava ser. Considero solução mais simples, apenas definindo a extensão no método file_path e refazendo o download em algumas situações é aceitável pois:

  1. Rodamos cada raspador uma vez por dia, buscando apenas os dados do último dia, então se por acaso formos refazer o download, será no máximo de apenas 1-2 arquivos (sem impacto grande no tempo de execução)
  2. São poucos os raspadores que vão precisar definir a extensão dessa maneira.

Minha sugestão (e se concordarem eu faço o ajuste no PR) é usar o filetype para identificar o tipo do arquivo e definir a extensão, sem se preocupar se ele vai ser baixado de novo ou não em uma re-execução do raspador.

O código acabará muito mais simples de se manter e resolvemos esse problema imediato rapidamente. Se no futuro percebermos algum impacto grande no tempo de execução dos raspadores, aí podemos reconsiderar a mudança mais complexa.

O que acham? @ogecece @trevineju @AlexJBSilva

@trevineju
Copy link
Member

Oi, @rennerocha! Obrigada por revisar!

Como o @ogecece avançou mais do que eu nessa revisão, acho que ele pode comentar melhor sobre a solução pelo lado mais técnico. Mas pelo lado de impacto...

  1. São poucos os raspadores que vão precisar definir a extensão dessa maneira.

Integrados, ainda são poucos, realmente. Mas conhecidos são bastantes já, como os sistemas DOSP (~250 municípios) e SAIIO (~300 municípios), por exemplo, todos eles tem esse problema. Já foram identificados casos individuais também, mas não me recordo deles de cabeça agora.

Será que não vale a pena já pensarmos na solução mais ideal desde agora?
Pq nos daria mais liberdade de adicionar essas cidades sem agravar a situação (pensando que já temos esse problema em produção hoje e vamos ter que parar para corrigi-lo, em breve)

@rennerocha
Copy link
Member

Será que não vale a pena já pensarmos na solução mais ideal desde agora? Pq nos daria mais liberdade de adicionar essas cidades sem agravar a situação (pensando que já temos esse problema em produção hoje e vamos ter que parar para corrigi-lo, em breve)

O problema em produção que temos, de não ter a extensão correta, pode ser resolvido pela solução simples (que eu prefiro, apesar de baixar um Diário mais de uma vez SE o raspador for executado novamente naquela data, o que não acontece em produção) ou com a complexa (que vai exigir mais tempo para testar os casos). Então não vai agravar nenhum problema, e vai resolver ele rapidamente, com menos código.

@ogecece
Copy link
Member

ogecece commented Apr 24, 2024

@rennerocha, concordo. Se encontrou outro problema que ainda precisaríamos resolver e complexificar mais ainda a solução, melhor lidar com essa consequência que é contornável. Vai na mesma linha que eu havia comentado aqui, inicialmente.

Você consegue fazer os ajustes então pra gente mesclar?

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.

Adicionar extensão ao arquivo baixado
4 participants