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

Refatoração para o uso de um spider base para DIONET #745

Merged
merged 8 commits into from
Mar 27, 2024

Conversation

ayharano
Copy link
Contributor

@ayharano ayharano commented Oct 19, 2022

AO ABRIR um Pull Request de um novo raspador (spider), marque com um X cada um dos items do checklist
abaixo. NÃO ABRA um novo Pull Request antes de completar todos os items abaixo.

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/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.

Descrição

PR com proposta de resolução parcial do item #724, refatorando o código comum a dois spiders já em produção:

Refatorando um spider que ainda não está em produção (aparentemente não é possível acessar os dados anteriores ao start_date da refatoração):

  • es_associacao_municipios

Adicionando um spider novo:

  • ms_corumba

@trevineju trevineju added the hacktoberfest-accepted Pull Requests aprovados na Hacktoberfest label Oct 19, 2022
@trevineju trevineju linked an issue Oct 19, 2022 that may be closed by this pull request
@ayharano ayharano mentioned this pull request Dec 1, 2022
5 tasks
@ayharano ayharano marked this pull request as draft December 18, 2022 12:18
@ayharano ayharano marked this pull request as ready for review December 18, 2022 20:31
@ayharano
Copy link
Contributor Author

TODO: antes de refatorar usando a main de 2023-10-10, verificar as modificações realizadas já mescladas na main comparada às modificações propostas nesse PR

@trevineju
Copy link
Member

Oi, @ayharano! Obrigada pela PR 😍 (e sempre, também pela paciência 🫂)

Revisei a PR, já adicionando os ajustes pq a contribuição não só atualiza o que temos em produção (Rio de Janeiro e Serra), mas também permite adicionar outras cidades importantes pra gente (Jaru é uma das maiores cidades da Amazônia Legal e São José dos Campos uma das maiores do país, conforme listado em #1086). Já adicionei elas aqui.

Histórico

TODO: antes de refatorar usando a main de 2023-10-10, verificar as modificações realizadas já mescladas na main comparada às modificações propostas nesse PR

Já atualizei o histórico da main aqui na branch, também 😉

Não dei squash no histórico de commits para ficar transparente pra você as mudanças que fiz (61d593b)

Mudanças

Só teve uma questão principal. Teve dois trechos do spider base que você usou .format

# 1a ocorrência
url = self.json_list_url.format(
                year=date_.year,
                month=month,
                day=day,
            )
# 2a ocorrência
gazette_url = self.gazette_id_url.format(gazette_id=gazette_id) 

E isso criou uma demanda nas classes-filhas de ter os atributos (pegando apenas RJ de exemplo, mas se repetiu nas outras):

json_list_url = (
        "https://doweb.rio.rj.gov.br"
        "/apifront/portal/edicoes/edicoes_from_data/{year}-{month}-{day}.json"
    )
gazette_id_url = "https://doweb.rio.rj.gov.br/portal/edicoes/download/{gazette_id}"

Aqui, o que entendi da decisão foi: em prol de deixar a classe base mais enxuta, as classes-filhas ficaram mais complexas. Revisei em função do oposto: é melhor que as classes filhas fiquem tão simples quanto for possível, deixando a complexidade na classe dionet. Por isso...

  • json_list_url foi substituído por BASE_URL, que é mais aderente ao padrão de projeto que adotamos.
  • O caminho da API (/apifront/portal...), justamente por ser uma generalização comum a todos os casos, foi movido para a classe base. E a URL final é construída com .join ao invés de .format
api_path = f"/apifront/portal/edicoes/edicoes_from_data/{date_.year}-{month}-{day}.json"
url = "".join([self.BASE_URL, api_path, self.url_subtheme])
  • esse url_subtheme apareceu aí porque entendi também que você estava construíndo a URL com .format pq a quantidade de campos das URLs muda e você quis construir uma solução flexível. No caso, para Rio de Janeiro e Corumbá, a URL acaba ali em api_path, mas Serra e Associação de Municípios tinham mais partes depois disso ("?subtheme=diariodaserra" e "?subtheme=dom").
    Adotei a mesma ideia que você: deixei um atributo de classe vazio na classe dionet. Se a classe-filha precisar, sobreescreve, se não, fica vazio.
    image
  • E gazette_id_url também subiu pra classe base, pelo mesmo motivo de ser uma generalização comum e dá para construir essa string a partir da BASE_URL. Ficando:
gazette_url = f"{self.BASE_URL}/portal/edicoes/download/{gazette_id}"

@trevineju
Copy link
Member

trevineju commented Mar 27, 2024

Logs:
Para os municípios que estavam sendo refatorados, coletei um período menor (março/2024) apenas pra validação das minhas modificações.
ms_corumba.log | ms_corumba.csv
rj_rio_de_janeiro.log | rj_riodejaneiro.csv
es_serra.log | es_serra.csv

Para os dois municípios que adicionei, foi a coleta completa:
sp_sao_jose_dos_campos.log | sp_sao_jose_dos_campos.csv
ro_jaru.log | ro_jaru.csv

@trevineju
Copy link
Member

Ah! E joguei fora o código anterior de São José dos Campos pq confirmei que o site não existe mais.

Copy link
Member

@trevineju trevineju left a comment

Choose a reason for hiding this comment

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

Muito muito muito obrigada, @ayharano! ❤️

Pelo cuidado em manter nossa base de código com mais qualidade, saudável, notando padrões e refatoramentos possíveis. E por fazer parte do projeto!

@trevineju trevineju linked an issue Mar 27, 2024 that may be closed by this pull request
@trevineju trevineju merged commit e581f0d into okfn-brasil:main Mar 27, 2024
1 check passed
@ayharano ayharano deleted the dionet_gazette_spider branch March 28, 2024 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Pull Requests aprovados na Hacktoberfest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Mapeamento] Lista de municípios que usam DIONET [Novo spider base] DIONET
2 participants