How to become an author
Search
Write a publication
Pull to refresh

Comments 15

PinnedPinned comments
хорошо бы обрабатывать ответы с кодом отличным от 200. Добавляем проверку, строчку будет лучше поместить в функцию.

Во-первых, зачем ради одной самоочевидной строки заводить целую функцию? Во-вторых, в requests и других подобных библиотеках уже есть встроенная функция raise_for_status, которая выбрасывает исключение при статусах 4xx и 5xx


return False

Пхпшники покусали? Во-первых, выбрасывать исключения при ошибках в питоне абсолютно нормально, та же raise_for_status так делает. Во-вторых, если есть аллергия на исключения, то можно хотя бы None вернуть, это всё ещё менее странно чем False


Логгирование, контроль исполнения и отладка

Всю эту информацию можно достать напрямую из объекта response или из возникшего исключения, функция response_res в показанном здесь виде не имеет никакого практического смысла. Кроме того, response_res ломает статическую типизацию, потому что заменяет объекты с аннотациями типов на нетипизированный словарь — для производства и будущей поддержки кода это ОЧЕНЬ плохо


автоматический сбор данных не очень приветствуется, даже в таком вроде бы легальном поле как получение рсс-ленты.

Бред, RSS по сути своей предназначен именно для автоматизированного сбора данных


словит капчу на второй-третий раз

Капча на RSS — опять бред


fake_useragent.UserAgent().random

И здесь сразу два подозрительных момента. Во-первых, клиент, меняющий свой юзер-агент при каждом запросе — это в принципе странно, а во-вторых, браузеры не умеют читать RSS (за редким исключением в виде браузерных расширений и прочей мелочи) и значит браузерного юзер-агента здесь быть в принципе не должно


не хорошо слать запросы с пустыми cookies, referer и так далее

Ничего из перечисленного быть в принципе не должно, это же RSS


except:

… и получаем невозможность прервать работу программы из-за перехваченных KeyboardInterrupt и SystemExit. К счастью, в следующем примере кода это уже исправлено, хоть и без пояснений


а вот с исключением возникает, вопрос, как его правильно обработать.

Точно не подменой на фейковый Response. Опять аллергия на исключения?


Для одновременных запросов многопоточность вообще хороша

Для одновременных запросов многопоточность вообще не подходит, потому что наплодит кучу жрущих ресурсы потоков, которые при этом будут простаивать без дела в ожидании ответа от сервера. Для io-bound задач специально изобрели асинхронность, которая замечательно работает в одном потоке


logging.info(f"многопоточность сломалась {ex}")

info для критических ошибок, скрытие информации о стеке, скрытие информации о типе ошибки, ужас… Есть же специальный logging.exception для такого


Если запустить программу в режиме планировщика (например каждые 30 секунд)

Нормальный планировщик должен дожидаться, когда предыдущий запуск завершится


Как выглядит итоговый код

feedparser.parse зачем-то вызывается два раза, пустая трата процессора. Причём во второй раз не перехватываются возможные исключения (а вдруг внутри feedparser баги есть)


Ещё feedparser сам по себе плох тем, что возвращает результат парсинга в виде нетипизированного словаря, но так как я не в курсе, есть ли достойные альтернативы, то ладно, пусть пока будет


Итого: меняем многопоточность на асинхронность, избавляемся от аллергии на исключения, возвращаем нормальные объекты вместо нетипизированного словаря, выкидываем бесполезный юзер-агент — и получаем гораздо более адекватный и при этом более компактный код:


Как на самом деле выглядит итоговый код 
import asyncio
import logging

import aiohttp
import feedparser

async def harvest_all(url: str) -> None:
    try:
        timeout = aiohttp.ClientTimeout(total=10)
        async with aiohttp.ClientSession(timeout=timeout) as session:
            async with session.get(url) as response:
                response.raise_for_status()
                raw_feed = await response.text()
    except Exception:
        logging.exception("Failed to get feed %s", url)
        return

    try:
        feed = feedparser.parse(raw_feed)
        process_feed(url, feed)
    except Exception:
        logging.exception("Failed to process feed %s", url)

def process_feed(url: str, feed: feedparser.FeedParserDict) -> None:
    if not feed["entries"]:
        logging.info("Feed %s has no entries", url)
        return

    for entry in feed["entries"]:
        logging.info("%s", entry["title"])

async def main() -> None:
    logging.basicConfig(level=logging.INFO)

    rss_list = [
        "https://lenta.ru/rss",
        "https://habr.com/ru/rss/all/",
    ]

    while True:
        tasks = {asyncio.create_task(harvest_all(url)) for url in rss_list}
        await asyncio.wait(tasks)
        await asyncio.sleep(60)

if __name__ == "__main__":
    asyncio.run(main())
Total votes 13: ↑13 and ↓0+13

Кажется, в самом первом примере проверки 200-го ответа вы перепутали равно с неравно :)

This comment wasn’t rated yet0

Точно перепутал, поправил, спасибо!

This comment wasn’t rated yet0
хорошо бы обрабатывать ответы с кодом отличным от 200. Добавляем проверку, строчку будет лучше поместить в функцию.

Во-первых, зачем ради одной самоочевидной строки заводить целую функцию? Во-вторых, в requests и других подобных библиотеках уже есть встроенная функция raise_for_status, которая выбрасывает исключение при статусах 4xx и 5xx


return False

Пхпшники покусали? Во-первых, выбрасывать исключения при ошибках в питоне абсолютно нормально, та же raise_for_status так делает. Во-вторых, если есть аллергия на исключения, то можно хотя бы None вернуть, это всё ещё менее странно чем False


Логгирование, контроль исполнения и отладка

Всю эту информацию можно достать напрямую из объекта response или из возникшего исключения, функция response_res в показанном здесь виде не имеет никакого практического смысла. Кроме того, response_res ломает статическую типизацию, потому что заменяет объекты с аннотациями типов на нетипизированный словарь — для производства и будущей поддержки кода это ОЧЕНЬ плохо


автоматический сбор данных не очень приветствуется, даже в таком вроде бы легальном поле как получение рсс-ленты.

Бред, RSS по сути своей предназначен именно для автоматизированного сбора данных


словит капчу на второй-третий раз

Капча на RSS — опять бред


fake_useragent.UserAgent().random

И здесь сразу два подозрительных момента. Во-первых, клиент, меняющий свой юзер-агент при каждом запросе — это в принципе странно, а во-вторых, браузеры не умеют читать RSS (за редким исключением в виде браузерных расширений и прочей мелочи) и значит браузерного юзер-агента здесь быть в принципе не должно


не хорошо слать запросы с пустыми cookies, referer и так далее

Ничего из перечисленного быть в принципе не должно, это же RSS


except:

… и получаем невозможность прервать работу программы из-за перехваченных KeyboardInterrupt и SystemExit. К счастью, в следующем примере кода это уже исправлено, хоть и без пояснений


а вот с исключением возникает, вопрос, как его правильно обработать.

Точно не подменой на фейковый Response. Опять аллергия на исключения?


Для одновременных запросов многопоточность вообще хороша

Для одновременных запросов многопоточность вообще не подходит, потому что наплодит кучу жрущих ресурсы потоков, которые при этом будут простаивать без дела в ожидании ответа от сервера. Для io-bound задач специально изобрели асинхронность, которая замечательно работает в одном потоке


logging.info(f"многопоточность сломалась {ex}")

info для критических ошибок, скрытие информации о стеке, скрытие информации о типе ошибки, ужас… Есть же специальный logging.exception для такого


Если запустить программу в режиме планировщика (например каждые 30 секунд)

Нормальный планировщик должен дожидаться, когда предыдущий запуск завершится


Как выглядит итоговый код

feedparser.parse зачем-то вызывается два раза, пустая трата процессора. Причём во второй раз не перехватываются возможные исключения (а вдруг внутри feedparser баги есть)


Ещё feedparser сам по себе плох тем, что возвращает результат парсинга в виде нетипизированного словаря, но так как я не в курсе, есть ли достойные альтернативы, то ладно, пусть пока будет


Итого: меняем многопоточность на асинхронность, избавляемся от аллергии на исключения, возвращаем нормальные объекты вместо нетипизированного словаря, выкидываем бесполезный юзер-агент — и получаем гораздо более адекватный и при этом более компактный код:


Как на самом деле выглядит итоговый код 
import asyncio
import logging

import aiohttp
import feedparser

async def harvest_all(url: str) -> None:
    try:
        timeout = aiohttp.ClientTimeout(total=10)
        async with aiohttp.ClientSession(timeout=timeout) as session:
            async with session.get(url) as response:
                response.raise_for_status()
                raw_feed = await response.text()
    except Exception:
        logging.exception("Failed to get feed %s", url)
        return

    try:
        feed = feedparser.parse(raw_feed)
        process_feed(url, feed)
    except Exception:
        logging.exception("Failed to process feed %s", url)

def process_feed(url: str, feed: feedparser.FeedParserDict) -> None:
    if not feed["entries"]:
        logging.info("Feed %s has no entries", url)
        return

    for entry in feed["entries"]:
        logging.info("%s", entry["title"])

async def main() -> None:
    logging.basicConfig(level=logging.INFO)

    rss_list = [
        "https://lenta.ru/rss",
        "https://habr.com/ru/rss/all/",
    ]

    while True:
        tasks = {asyncio.create_task(harvest_all(url)) for url in rss_list}
        await asyncio.wait(tasks)
        await asyncio.sleep(60)

if __name__ == "__main__":
    asyncio.run(main())
Total votes 13: ↑13 and ↓0+13

Все-таки у оригинального кода есть одно преимущество: его легче понять и доработать. Обратите внимание, как много вашего кода было переиспользовано в предложенном варианте (одно название функции, если я не ошибаюсь?). Часто попытки разобраться как исправить/доработать такой вот продакшн-код занимают больше времени и сил, чем используя типовые паттерны добавить простую обработку ошибок к ничем незамутненному happy path.

Total votes 1: ↑1 and ↓0+1

Критика конструктивная, особенно по части нетипизированных словарей) и ряд подходов тоже интересные, определенно стоить рассмотреть внедрение. В тоже время предложенный в статье вариант (с учётом отмеченных недостатков) вполне рабочий. По части решения многопоточности, изучу подробнее, тут аргументы не самые убедительные приведены, но с предметом подробно не знаком. В любом случае, пользы от материала получилось много больше чем ожидалось)

This comment wasn’t rated yet0

Ну знаете, оригинальный вариант тоже будет вполне рабочим, если обернуть его в systemd-сервис с таймером. Оно и упавший процесс перезапустит, и зависший по таймауту прибьет, и несколько экземпляров с разными url можно организовать.


Если хотите разобраться с многопоточностью — попробуйте заставить вашу программу сколько-нибудь прилично реагировать на Ctrl-C, узнаете много нового.

This comment wasn’t rated yet0

Поюс неплохо было бы If-Modified-Since обрабатывать.

Total votes 1: ↑1 and ↓0+1

"Чтобы код использовался в рабочем проекте..." - наверно имеет смысл заранее составить чёткое ТЗ что от этого кода ожидается и какие ситуации нужно обрабатывать. Тогда программисту не придётся по ходу дела что-то выдумывать.

Total votes 3: ↑2 and ↓1+1

В начале материала задача ставится, и дальше идёт объяснение подхода к её решению

нам нужно получать несколько рсс-лент в плотном режиме (например каждую минуту)

This comment wasn’t rated yet0

Да. Объяснение похода к решению задачи интересное. Но почему этот код может не попасть в прод, если он решает задачу "нам нужно получать несколько рсс-лент в плотном режиме (например каждую минуту) " ? Потому что не совсем правильный подход не соответствует чьей-то жизненной позиции или опыту? Вед в ТЗ эти ньюансы не описаны. Код делает то что нужно.

This comment wasn’t rated yet0

Ещё можно так писать

try:

response = requests.get()

if response.ok:

...

except requests.RequestException as err:

...

This comment wasn’t rated yet0

Перечень фидов в коде, а не в отдельном файле, отсутствие самоминимального cli - такой хоккей нам не нужен...

This comment wasn’t rated yet0

а какой cli может быть для приведенного случая? (фиды в коде, а не в отдельном файле, для наглядности)

This comment wasn’t rated yet0

Гм. Самый-самый минимум - путь к тому файлу с фидами ). Чуть больше -h, элементарный help. У меня в шаблоне еще настройки логгирования - уровень болтливости, вывод в файл для отладки. Специфически для задачи - размер пула для параллельной обработки (С соответствующей логикой жевания чанками), а то окажется в файле с фидами миллион записей и как-то не хорошо получится. Если предполагаем, что фидов может оказаться таки миллион - еще и всякие таймауты\временное исключение сбойных урлов из списка появится. Для сетевки еще и поддержка прокси, по тому, что фиг знает кто\где\как это все запускать соберется. В случае, если код пойдет в прод, т.е. будет контейнеризован - получение этого из переменных окружения (в click "из коробки", но.).

This comment wasn’t rated yet0
Sign up to leave a comment.

Articles

Your account

Sections

Information

Services

Support
© 2006–2024, Habr