review notes

review_notes.md

Nate notes

general notes

• nice use of HttpException I didn't know that would return a proper error
• maybe add type hints if we have time
• no defensive programming around function/method parameters/arguments

files and git

• Add a readme.md to the root. Mine was a copy and paste of the requirements and a quick blurb of what I implemented.

• Add gitignore https://github.com/brandon-braner/torqata_example/blob/master/.gitignore

• Remove venv and __pycache__/ from git history

  • git rm -rf —cached venv __pycache__/

• try to rename content service again, it seems to work on my computer

• review the requirements.txt and delete anything from your virtual env you dont need.

  • You can do this either by pip uninstall package or going into the interpreter in pycharm and deleting the package.
  • Then recreate your requirements.txt pip freeze > requirements.txt

routing

I would suggest moving your routes into separate files using APIRouter

https://fastapi.tiangolo.com/tutorial/bigger-applications/

main.py

lines 8 and 9 why import as

import services.content_service as ContentService
import services.user_service as UserService

function defs should use snake case getcontent = get_content getwholerecord = get_whole_record updatetitlebyid = update_title_by_id

fix line space errors

auth.py

• remove () from AuthHandler not needed
• pycharm says we should put a init in the class with pass
• we should look into moving secret to an env variable

interview notes

• they will probably ask why you used bcrypt, they did me
• may ask you about jwt tokens, I did not use them

user_service.py

instantiate the user model on one line then get the data on the other

model = UserModel()
users = model.get_user_list()

login function

would it make more sense to make a get_user_by_name function on the model and query for the single user?

Create a PydanticSchema for your response. That way they are all formatted the same and they will be documented in the api documentation

content service

i am not sure what id the id variable is shadowing but maybe renaming it to content_id would clear that notice up.

create a pydantic model for the assemble whole record

line 43 contentdf should be content_df

• make one method for updating where take in a dict of the content you query for the content and merge two dicts together
• if you leave it like it is remove updated_content = as that isn't used

models

remove the s from the names user_model vs user_models there maybe a time where you need to query more than one model, having sqlmodel created in each model is bad, we should move that to a provider class and import it into the models https://github.dev/brandon-braner/torqata_example/blob/88f0addf4151ecb0815bd5fcd5e118a9a7f93edd/app/providers/db.py#L29

since these are classes you can remove the class name from the methods create_user -> create