Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat (WIP): use pydantic for model validation #65

Draft
wants to merge 30 commits into
base: main
Choose a base branch
from

Conversation

NiceAesth
Copy link
Contributor

@NiceAesth NiceAesth commented Feb 23, 2024

Status: this branch will not function atm

Description

Pydantic is the industry standard tool for model validation. It is a well trusted tool baked into other libraries (such as FastAPI).

This PR aims to shift the library into using it. It will be a very big one, but I will attempt to keep commit history simple. Also, the models will try to keep the same structure as the interfaces used in Lavalink itself.

Reasoning

  • It would be a massive gain towards stability and type safety
  • It would provide very simple ways of handling versioned models using smart unions meaning we could remove a large portion of if statements for lavalink versions
  • It would remove a massive amount of boilerplate we have to write
  • Pydantic is ""blazing fast"" as the rust devs would say it

To-do:

Models:

  • LavalinkVersion
  • Events
  • Enums
  • Filters
  • Payloads
  • Track
  • Playlist
  • Queue
  • Apple Music Song
  • Apple Music Playlist
  • Apple Music Album
  • Apple Music Artist
  • Spotify Track
  • Spotify Playlist
  • Spotify Album
  • Spotify Artist

Library

  • Pool
  • Player

Code Style:

  • Remove relative imports (this will be followed by a second PR to change the library to use poetry instead of pipenv, something which we arguably should have done in the first place as DX with pipenv for libraries simply sucks)
  • Address type: ignore comments
  • Address unused arguments in functions (e.g. force in disconnect())
  • Separate concerns in player/pool better, reduce nesting
  • Address inconsistencies between classmethod based defaults in Timescale filters and subclass based defaults with Frequency filters (e.g. Tremolo / Vibrato)

Documentation:

  • wip idk

@NiceAesth NiceAesth changed the title feat: use pydantic for model validation feat (WIP): use pydantic for model validation Feb 23, 2024
NiceAesth and others added 21 commits February 23, 2024 13:43
can be used as helpers for type unions
allows for more standard handling
arguably we shouldn't be using our own exceptions for cases like this in the first place
not sure about this one
@NiceAesth
Copy link
Contributor Author

Looking at the existing code the logic for tracks will become insanely simple by switching to typing based logic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant