8000 Integrate typing into source by rmartin16 · Pull Request #345 · rmartin16/qbittorrent-api · GitHub
[go: up one dir, main page]
Skip to content

Integrate typing into source #345

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 15, 2023
Merged

Integrate typing into source #345

merged 1 commit into from
Oct 15, 2023

Conversation

rmartin16
Copy link
Owner
@rmartin16 rmartin16 commented Oct 9, 2023 •
edited
Loading

Changes

  • Moves typing from the pyi files in to the py source files
  • This was primarily afforded by dropping support for versions of Python prior to 3.8
  • This also revealed that typing from pyi is seemingly much less robust as mypy was much more vigilant about issues
  • Typing the decorators was proving impossible after hours of trying to find the proper dark magic...so i replaced them
    • This also means that the legacy hash and hashes arguments for API methods is now unsupported and any uses must be updated to use torrent_hash and torrent_hashes, respectively.
  • A final missing piece, as inspired by types: add TypedDict for Tracker #337, is to add types to the JSON returned from qBittorrent
    • Since these are all dictionaries, the obvious way to do this is with TypedDict....unfortunately, you cannot use TypedDict with Generic until Python 3.11.
    • Although, even then, it's unclear if the AttrDict base will still cause conflicts...and I'm quite hesitant to rip that out...even if I almost certainly wouldn't have included this if I rewrote all this.

@codecov
Copy link
codecov bot commented Oct 9, 2023 •
edited
Loading

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (5a988e6) 100.00% compared to head (f655ea7) 100.00%.
Report is 12 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##              main      #345    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           15        14     -1     
  Lines         1957      1927    -30     
  Branches       577       351   -226     
==========================================
- Hits          1957      1927    -30
Files Coverage Δ
src/qbittorrentapi/_version_support.py 100.00% <100.00%> (ø)
src/qbittorrentapi/app.py 100.00% <100.00%> (ø)
src/qbittorrentapi/auth.py 100.00% <100.00%> (ø)
src/qbittorrentapi/client.py 100.00% <100.00%> (ø)
src/qbittorrentapi/definitions.py 100.00% <100.00%> (ø)
src/qbittorrentapi/exceptions.py 100.00% <100.00%> (ø)
src/qbittorrentapi/log.py 100.00% <100.00%> (ø)
src/qbittorrentapi/request.py 100.00% <100.00%> (ø)
src/qbittorrentapi/rss.py 100.00% <100.00%> (ø)
src/qbittorrentapi/search.py 100.00% <100.00%> (ø)
... and 3 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rmartin16 rmartin16 force-pushed the integrate-typing branch 14 times, most recently from 0a7c981 to 576dc8b Compare October 14, 2023 22:39
github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 14, 2023
View reviewed changes
src/qbittorrentapi/request.py
resp_logger("Request Headers: %s", response.request.headers)
resp_logger("Request HTTP Data: %s", {"data": data, "params": params})
if "auth/login" not in url:
resp_logger("Request HTTP Data: %s", {"data": data, "params": params})

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information

This expression logs [sensitive data (password)](1) as clear text. This expression logs [sensitive data (password)](2) as clear text. This expression logs [sensitive data (password)](3) as clear text. This expression logs [sensitive data (password)](4) as clear text. This expression logs [sensitive data (password)](5) as clear text. This expression logs [sensitive data (password)](6) as clear text. This expression logs [sensitive data (password)](7) as clear text. This expression logs [sensitive data (password)](8) as clear text. This expression logs [sensitive data (password)](9) as clear text. This expression logs [sensitive data (password)](10) as clear text.
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

False positive. Disabled by default and sensitive information from auth/login won't ever be printed.

@rmartin16 rmartin16 force-pushed the integrate-typing branch 4 times, most recently from 75ffd0d to a2b15f1 Compare October 15, 2023 21:31
@rmartin16 rmartin16 marked this pull request as ready for review October 15, 2023 22:06
@rmartin16 rmartin16 merged commit 193b994 into main Oct 15, 2023
@rmartin16 rmartin16 deleted the integrate-typing branch October 15, 2023 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@rmartin16
0