refactor: unify manager patterns and remove duplicated code#5
Conversation
- ScrapeManager now uses (agency_name, scraper) tuples like EBCScrapeManager, ensuring agencies_processed returns YAML keys consistently across both endpoints - Extract get_config_dir() to yaml_config.py, removing duplication from both managers - Delete empty test_scrape_manager.py (tests were moved to test_yaml_config.py) - Add tests for get_config_dir() Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
miguellsfilho
left a comment
There was a problem hiding this comment.
Code Review - PR #5
Revisei as mudanças propostas e aprovo este PR. Segue minha análise:
Pontos Positivos
-
Remoção de duplicação: A função
_get_config_dir()estava duplicada emscrape_manager.pyeebc_scrape_manager.py. Extrair parayaml_config.pycomoget_config_dir()é a abordagem correta. -
Unificação de padrões:
ScrapeManageragora usa tuplas(agency_name, scraper)consistente comEBCScrapeManager. Isso garante queagencies_processedretorne os mesmos tipos de valores em ambos os endpoints. -
Fonte de verdade: Usar a chave YAML (
agency_name) em vez descraper.agencyé mais confiável - evita dependência da estrutura da URL e mantém consistência com a configuração. -
Testes adequados: Os testes para
get_config_dir()cobrem os casos necessários. -
Limpeza de código: Remoção do arquivo de teste vazio é apropriada.
Ressalva Menor
O commit inclui Co-Authored-By: Claude Opus 4.6, mas o CLAUDE.md do projeto indica que commits não devem incluir co-autoria do Claude. Para PRs futuros, considere remover essa linha. Não é blocker para este PR.
Verificação
- Mudanças são tecnicamente corretas
- Base branch correta (
feat/active-field-agencies) - Testes passando (31 testes)
LGTM! 👍
Summary
Follow-up do PR #4 para resolver inconsistências identificadas na revisão:
ScrapeManageragora usa tuplas(agency_name, scraper)comoEBCScrapeManager, garantindo queagencies_processedretorne nomes das chaves YAML de forma consistente em ambos os endpointsget_config_dir()extraído parayaml_config.py, removendo duplicação de_get_config_dir()em ambos managerstest_scrape_manager.pyvazio removido (os testes foram movidos paratest_yaml_config.pyno commit anterior)get_config_dir()Arquivos modificados
src/govbr_scraper/scrapers/yaml_config.pyget_config_dir()src/govbr_scraper/scrapers/scrape_manager.pyget_config_dirsrc/govbr_scraper/scrapers/ebc_scrape_manager.pyget_config_dir, remove_get_config_dirtests/unit/test_scrape_manager.pytests/unit/test_yaml_config.pyget_config_dirTest plan
poetry run pytest tests/unit/ -v— 31 testes passando🤖 Generated with Claude Code