Propriedade 'allowed_domains' dos raspadores: informação geralmente redundante?

[Gab0]

O Scrapy utiliza a propriedade allowed_domains para impedir que o raspador seja direcionado para domínios não desejados ou potencialmente maliciosos. Ela é opcional, e precisa ser definida em cada raspador. Nesse projeto, acredito que seja válido que ela seja definida em todos os raspadores.

A maior parte dos raspadores desse repositório declara allowed_domains manualmente e também declara alguma URL para direcionar o raspador. O problema é que em muitos casos, a propriedade allowed_domains é uma lista com um único domínio: o domínio da URL passada ao raspador, conforme mostrado no exemplo abaixo:

class CeCedroSpider(BaseAdiariosV1Spider):
    TERRITORY_ID = "2303808"
    name = "ce_cedro"
    allowed_domains = ["cedro.ce.gov.br"]
    BASE_URL = "https://www.cedro.ce.gov.br"
    start_date = date(2018, 1, 1)

Para evitar essa redundância, poderíamos empregar um método de gerar allowed_domains automaticamente, com base na URL fornecida. Isso simplificaria o código dos raspadores, permitindo omitir essa propriedade e garantindo que ela esteja sempre definida.

Existem exceções: alguns raspadores precisam de mais de um allowed_domains, para buscar informações em outros lugares. Nesses casos, bastaria declarar a propriedade manualmente.

Essa issue pode afetar mais de 100 raspadores neste repositório. Podemos discutir sobre a validade dessa issue, e se for considerada válida, proponho que seja resolvida em duas etapas:

1. Em BaseGazetteSpider.__init__, checar a existência da propriedade allowed_domains na instância. Se não existir, utilizamos a urllib para extrair o domínio encontrado em alguma propriedade conhecida como apontadoras da URL inicial. O nome dessa variável não é padronizado entre os raspadores: encontramos base_url, url_base e até mesmo BASE_URL . Se não forem encontrados meios de ter a allowed_domains definida, podemos emitir um alerta.
2. Remover as declarações redundantes de allowed_domains de todos os raspadores.

[trevineju]

O peso numérico dessa situação só acontece pois a maior parte dos raspadores usam alguma classe base.

Então, acho que, se for para adotar uma solução dessas, pode ser mais interessante atualizar as bases para preencher o allowed_domain. Assim como é o exemplo que você trouxe: BaseAdiariosV1Spider poderia ser incrementado, e não necessariamente BaseGazetteSpider. Só essa base já traria um reflexo de enxugar a presença de allowed_domains de 34 raspadores.

Esses casos, trazem um pouco mais de "confiança" pra essa alteração, visto que a URL segue um padrão conhecido e o nome da variável (url_base, base_url, etc) é forçadamente igual. Para além desse escopo, teria que pensar um pouco mais na proposta, visto que os contextos passam a ser mais diversos e imprevisíveis.

[Gab0]

Implementar a solução em cada classe base pode ser uma possibilidade, e já iria sim enxugar a allowed_domains de vários raspadores. Ainda assim, seria mais simples modificar apenas a própria BaseGazetteSpider, se for possível. O workflow poderia estar associado à checagem que você propôs em #1319, que é importante. Exemplo:

class BaseGazetteSpider(scrapy.Spider):
    # ...

    def __init__(self, start_date="", end_date="", *args, **kwargs):
        super(BaseGazetteSpider, self).__init__(*args, **kwargs)

        if not hasattr(self, "TERRITORY_ID"):
            raise NotConfigured("Please set a value for `TERRITORY_ID`")

        if not hasattr(self, "allowed_domains"):
            self.allowed_domains = self.infer_allowed_domains()
            if self.allowed_domains is None:
                raise NotConfigured("Please set a value for `allowed_domains`")

        # ...

    def infer_base_url(self) -> Optional[str]:
        # NOTE: O nome desse campo poderia ser padroniado.
        known_base_url_fields = ["base_url", "url_base", "BASE_URL"]

        for field in known_base_url_field:
            url = getattr(self, field, None)
            if url is not None:
                return url

        return None

    def infer_allowed_domains(self) -> Optional[List[str]]:
        base_url = self.infer_base_url()

        if base_url is None:
            return None

        return [urlparse(base_url).netloc]

[Gab0]

São 219 spiders que declaram allowed_domains de forma redundante, que poderia ter sido extraída de sua base_url. Acabei fazendo um experimento pra verificar isso, ele está aqui: https://github.com/Gab0/querido-diario-spider-checker

[trevineju]

Implementar a solução em cada classe base pode ser uma possibilidade, e já iria sim enxugar a allowed_domains de vários raspadores. Ainda assim, seria mais simples modificar apenas a própria BaseGazetteSpider, se for possível.

Simples e possível é, o problema é se estaria certo resolver assim. A origem dessa situação não é o BaseGazette estar incompleto, pq não é papel de BaseGazette preencher campos. O papel que BaseGazette cumpre é meio que garantir que os raspadores tenham os atributos obrigatórios (territory_id, datas, etc) -- e ter uma BASE_URL não é obrigatório, é contextual. É tão contextual que, além do que você mesmo já notou que pode ter várias variações de nomenclatura por questões de estilo de desenvolvimento (que por si só é outro problema), pode ser que alguém crie um atributo BASE_URL para fazer alguma coisa no processamento interno do raspador que não necessariamente terá o domínio. Nesse caso, estaríamos automatizando um bug aos fazer essa inferência.

O único contexto em que ter uma BASE_URL (ou nomenclatura variada) é obrigatório é o de spiders base <> spiders herdeiros da base. Nesse caso, sempre temos completo controle do uso da variável. Por isso disse que a solução seria por aí. Como é ali que surge esse problema, é ali que se resolve ele: corrigindo a generalização da classe base.

[Gab0]

Atualizei aquela ferramenta, e as 219 Spiders com allowed_domains redundantes (e que possuem base_url ou atributo similar para inferência) estão distribuidos em 11 classes intermediárias, lista completa aqui.

Seria problemático implementar a checagem e inferência das allowed_domains em cada uma dessas classes intermediárias separadamente. Colocar a checagem na BaseGazette não é uma opção porque vai sobrecarregar a classe? Existe uma solução mais chique: criar outra classe, por exemplo GazetteWithBaseUrl que seria subclasse de BaseGazette e superclasse de todas as classes intermediárias que declaram self.base_url (ou equivalente). Lá essa inferência automática de base_url para allowed_domains pode acontecer.

Outro problema é que ter considerar todas as diferentes maneiras de se referir a base_url que estão espalhadas pelo repositório seria complacência com uma coisa que já deveria estar padronizada. Então antes de tentar solucionar essa issue aqui poderíamos tentar solucionar a #1320.

[trevineju]

@Gab0, veja, o que eu entendo do que você está propondo é: ao invés de corrigir o problema, contornar ele. É isso que a ideia de inferência traz.

Seria problemático implementar a checagem e inferência das allowed_domains em cada uma dessas classes intermediárias separadamente.

Não disse por qual motivo.

Você diz que implementar meros urlparse(base_url).netloc (ou variações a depender do nome da variável) em algumas classes intermediárias seria "problemático", mas quer criar uma superclasse pra esses casos -- coisa que obrigaria a mexer nas classes intermediárias de qualquer forma --, e ainda quer renomear variáveis por toda base de código do repositório. Isso é muito mais do que corrigir apenas alguns arquivos.

Não vamos criar uma GazetteWithBaseUrl pois a família de classes do repositório existe para acomodar lógicas de raspagem de sites, coisa que uma classe desse tipo não faria. Para rotinas comuns a muitos raspadores, o caminho dentro da lógica do repositório é criar uma função no módulo utils.

Então, você pode adicionar uma função extract_domain nas utils do projeto e todas as spiders base serem modificadas para importar e usá-la. Mas veja que ainda assim cada spider base deverá ser modificado.