Pull to refresh

Comments 24

То есть новичков вы сразу решили научить использованию глобальных переменных, причем неявно скрытых где-то в контексте? Не видите ли вы здесь каких-то недостатков?

Подскажи конкретнее о каких глобальных переменных речь? Довнесу в статью тогда. Из неявного только импорты хинтов и алхимии, все остальные переменные я попытался вынести в код, чтобы было видно людям.

contextvars - это, фактически, использование глобальной переменной. Примерно то же самое, что `def func(**kwargs)`. Из объявления функции непонятно с какими переменными она работает, что она изменяет. Это антипаттерн. Это может быть, иногда, удобно, но я бы советовал явно передавать аргументы.

хмм
у меня чуть-чуть другое мнение. Ключевое преимущество, когда много вложенных функций, куда проще работать с базой. Обернул в декоратор, получил сессию. И не надо передавать в функцию никаких объектов, и ничего не надо редактировать(условно понадобился объект сессии в функции, а она у вас 100 раз вызывается, везде редактировать надо будет). Конечно пример это крайний случай, но всё же.

И так же я бы не сказал что это глобальная переменная, в плане логики работы. У вас на каждый стек вызовов функций, свой контекст. То есть соседний вызов этой же функции не получил то значение, которое есть в этой функции

Но это лишь моё мнение)

А что изменится, если заменить ContextVar головной переменной или полем синглтона? Рефакторить проще, отлаживать проще. Минусов относительно ContextVar никаких. Зачем тогда ContexVar и чем он лучше обратной переменной или поля некоторого глобального класса?

В целом, никак не изменится, вы верно подчеркнули суть. Можно и с глобальной переменной провернуть, и с замыканием, наверное. Сам этот contextvars немного попахивает. Я бы понял если бы это была сторонняя библиотека, но оно в stdlib теперь.

В последниее время Python нехорошо скатывается в очень странные изменения не только в синтаксисе (привет, switch), но и в идеологии. Возможно, Гвидо раньше как-то сдерживал самые идиотские пулл-реквесты, но теперь вместо него решения принимают другие люди.

Не будет изоляции в асинхронном коде, как уже писали здесь

Мне кажется что в этом случае ContextVar выступает в роле ThreadLocal, только для питона AsyncContextLocal-переменной. Так как автор явно упомянул что пользуется этим кодом для Telegram-ботов и веба, а также учитывая что функции async, то "обернуть" сессию каждого запроса в ContextVar для меня кажется разумным решением чтобы сохранить их независимость друг от друга

Как бы да, но оно почему-то реализовано самым безобразным образом. Так, чтобы уж точно не поддерживаться никакими синтаксическими анализаторами IDE и не рефакториться толком.
Поверх этого можно было намазать синтаксического сахара, который бы позволил представить тоже самое в виде полей класса, или чего-то вроде NamedTupe или Dataclass. Если бы это было из коробки, пусть и ввиде ещё одного уровня абстракции, то нормально было бы. Но оно какое-то безобразное всё.

Ну, и сама идея класть соединение в контекст потока как-то дурно пахнет. Очень легко всё это забыть и потерять. Аналог с ThreadLocal хорош - никто же не кладёт туда в здравом уме соединение к БД, так как чуйкой чуют, что потом проблем больше чем пользы. Побочных эффектов массу можно придумать. А точно есть гарантия что совершенно никогда не будет открытия нового конекта в том же контексте? А если потом когда понадобится, забудут же где на самом деле этот коннект хранится и что он не реентерабельный.
Хранение потенциально изменяющихся или дополняющихся данных (а если потом несколько коннектов понадобиться или несколько баз? или в одном пуле несколько коннектов смешаются? а при тестировании точно не будет приключений?) в неявные и ни с чем несвязанных контексты это очень плохо. Какой-то нибудь явный объект контекста и то в 100 раз лучше было бы. Да, нужно было бы как-то это объект тащить с собой, но зато он явный, отслеживается и рефакторится.
Не говоря уже о том, что конечная цель всё же не просто конект к базе иметь, а всю пользовательскую сессию. Ну, так и решение нужно делать сразу под возможность хранения каких-то данных сессии. А так оно всё раскидано по разным местам будет.

Ну, и сама идея класть соединение в контекст потока как-то дурно пахнет.

Согласен, могут возникнуть проблемы. В частности, когда задачу в фон класть через create_task, то у основного хендлера сессия закроется, а у задачи в create_task он ведь тоже закроется. И будет неявная ошибка. Для этого можно добавить 1 параметр, условно new_session, который отвечает за то чтобы добавить отдельную сессию на функцию, чтобы не зависеть от главной функции. Но да, всё оно имеет свои плюсы и минусы

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

Да, гарантия есть. При каждом вызове декоратора идет проверка, есть или нет сессия, если есть, отдается "старая" сессия, если нет, создается новая сессия.

UFO just landed and posted this here

Кстати, вопрос про реентерабельность хороший. Необязательно даже иметь рекурсию чтобы вызвать foo() два раза - достаточно иметь две функции с таким декоратором foo() и bar() и вызвать одну из другой.

Что касается сессии - это contextvars, аналог threading.local, но для асинхронного кода. Если я правильно понял ваш вопрос - то нет, два разных запроса не должны смешаться.

UFO just landed and posted this here

Вижу что baidr вам уже ответил по поводу того что переменная изолирована в корутинах. Отвечу по нагрузочному тестированию

Мы с коллегой используем это в работе, проект небольшой, но при 100-1000 рпс проблем нет, вообще никаких. Возможно, это создаст какие-то проблемы при 100000рпс, но до этого пока далеко. Так как по сути мы создаём 1 транзакцию, на 1 хендлер(что важно), и далее в вызываемых функциях мы используем транзакцию хендлера, то не вижу даже предполагаемых проблем, где что может замедлять работу приложения

UFO just landed and posted this here

Почему, а главное зачем?

engine = create_async_engine(
   SQLALCHEMY_DATABASE_URL,
   **(dict(pool_recycle=900, pool_size=100, max_overflow=3))  # ??? 
)

В этом есть какой-то практический смысл?

В этом нет никакого смысла, просто мой коллега так сделал, решил и оставить

Но статья совсем не об этом же)

Она-то не о том, но я тоже запнулся на этом месте ища потаённый смысл.

А если у вас будет 2 бд, как вы разрулите эту ситуацию? В декораторе имя передавать, наверное, норм

Хмм, очень хороший вопрос
Если вы имеете в виду допустим шардированный постгре, то возможно сделать ещё один уровень абстракции, над транзакцией наверное.. хотя не особо уверен что поможет, тут надо подумать очень хорошо как это красиво реализовать
Если просто допустим ещё редис, то лично в питоне он вообще как глобал переменная используется, и там создается подключение на каждый запрос(да можно объединить запросы). А так для другой СУБД ничего не мешает сделать такой же декоратор
Вы задали очень правильный вопрос, пока не готов на него ответить!

Чем необязательный параметр session или connection не угодил? Ловишь его в декораторе, если он пустой или не передан — инициализируешь в декораторе и передаешь, если передан — pass through. Это позволит звать одну и ту же функцию как из другой функции над уже работающей транзакцией, так и звать ее как есть, представляя декоратору позаботиться о подготовке новой сессии.

Если я правильно понял ваш вопрос
Так в декораторе и так реализовано, если уже есть сессия, то он просто обрабатывает функцию. Если нет, то создает сессию и обрабатывает функцию

А если сессия есть, но мне надо, чтобы функция явным порядком создала себе отдельную сессию и отработала в ней?

Рекомендую автору почитать зачем нужен dependency injection, dependency inversion и зачем люди делают зависимости более явными (спойлер: чтобы код работал более очевидно). Вы соорудили какого-то монстра чтобы сэкономить на передаче сессии, зато потом непонятно как это поддерживать.

А два пробела вместо четырёх никого не смутило?

Sign up to leave a comment.

Articles