ZOMini / ugc_sprint_2

User Generated Content(UGC) + Elastic Logstash Kibana(ELK)
0 stars 0 forks source link

Code review #2

Open BigDeepBlue opened 1 year ago

BigDeepBlue commented 1 year ago

Здравствуйте, отлично поработали ! Молодцы, что сделали исследование БД. Есть небольшие рекомендации:

  1. Здесь ох сочувствую.
  2. Здесь можно раскомментить - пусть нотификации работают.
  3. Тут было бы неплохо написать модельки ответов https://fastapi.tiangolo.com/tutorial/response-model/ . Таким образом станет более понятно что возвращает ручка.
  4. Тут видимо случайно осталось, цикл с принтом.
  5. Тут наверное можно избавиться от append-ов, просто добавить это в обьявление pipeline выше.
  6. Здесь похожий совет на тот что дал уже по API - можно написать модельки данных которыми манипулируете, таким образом и код станет понятнее сразу и валидация данных появится.
  7. Тут с аннотациями немного нужно починить, вы используете motor асинхронную библиотеку и соответсвенно в ней содержатся типы поддерживающие async/await.
    
    uri = "mongodb://dev:dev@localhost:27017/?authSource=admin"
    connection_args = {
    "zlibCompressionLevel": 7,
    "compressors": "zlib"
    }
    client = AsyncIOMotorClient(uri, **connection_args)
    db = client.get_database("mydatabase")
    collection = db.get_collection("mycollection")
    collection
    AsyncIOMotorCollection(Collection(Database(MongoClient(host=['localhost:27017'], document_class=dict, tz_aware=False, connect=False, authsource='admin', zlibcompressionlevel=7, compressors=['zlib'], driver=DriverInfo(name='Motor', version='3.1.1', platform='asyncio')), 'mydatabase'), 'mycollection'))

как видите `AsyncIOMotorCollection` а не Collection тип из синхронной версии. Пересмотрите аннотации касательно motor и типов с помощью которых работаете с mongo.
Иначе await будут подсвечены подобно этому:
![image](https://user-images.githubusercontent.com/73478414/221146949-eff02fc4-88c4-420f-97f7-46a06248e966.png)
8. Молодцы [тут](https://github.com/ZOMini/ugc_sprint_2/blob/main/deploy/logstash.conf) что позаботились и не свалили все логи в одну кучу )
ZOMini commented 1 year ago
  1. Починил, спасибо спирту животворящему :) (медицинскому), засохшие кристаллики сахара замыкали где-то.
  2. Ок
  3. Ок. Сделал
  4. Ок. Удалил
  5. Ок. Сделал
  6. Ок. Сделал, пришлось валидаторы(сериализаторы) прикрутить, но так конечно правильнее.
  7. Ок. Спасибо, теперь все четко.
  8. Спасибо, старался.
BigDeepBlue commented 1 year ago

@ZOMini @isinyakin Молодцы ! Все поправили ! :+1: