-
-
Notifications
You must be signed in to change notification settings - Fork 725
Introduce Pydantic data models and API type annotations #4560
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
base: develop
Are you sure you want to change the base?
Conversation
lists are not hashable and did cause an exception here
src/pyload/core/api/__init__.py
Outdated
@@ -1215,15 +1236,15 @@ def get_account_types(self): | |||
|
|||
@legacy("updateAccount") | |||
@permission(Perms.ACCOUNTS) | |||
def update_account(self, plugin, account, password=None, options={}): | |||
def update_account(self, plugin: str, account: str, password: str = None, options: dict[str: Any] = {}) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand the code analysis is throwing a warning here, but I think for now we should keep the default arguments as they were before.
except Exception: | ||
return jsonify(False), 500 | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I deleted this function because I don't think it actually worked.
it calls get_queue()
which returns PackageData
and then tries to access it id
- but PackageData
has no id
property/key.
Even if we assume that pid
was meant here, then it will call api.get_package_files()
which doesn't exist.
So something was very broken here. If you think we should keep it then please let me know how to fix this method to actually work as expected.
Describe the changes
As outlined in my comment on #4414 this is the first step to enable automatic API documentation.
So far, everything seems to work just fine.
With this, we can parse the existing methods into an OpenAPI specificaton, will submit this in a follow-up PR. I have a working draft, but it needs some polishing.
I've tried to encapsulate all changes to logical commits to make the review easier. In any case, I'd say this should probably stay on the develop branch for a couple of weeks before merging into main to make sure it doesn't cause any issues.
Additional references
Related issue #4414