Skip to content

Add get_quota to Ollama provider#3396

Merged
hlohaus merged 2 commits intoxtekky:mainfrom
Desel72:fix/issue-3395
Mar 20, 2026
Merged

Add get_quota to Ollama provider#3396
hlohaus merged 2 commits intoxtekky:mainfrom
Desel72:fix/issue-3395

Conversation

@Desel72
Copy link
Contributor

@Desel72 Desel72 commented Mar 19, 2026

Summary

  • Add get_quota to Ollama provider to fetch session, hourly, and weekly usage limits from ollama.com/settings

Reference

https://github.com/Haehnchen/idea-de-espend-ml-llm/blob/main/src%2Fmain%2Fkotlin%2Fde%2Fespend%2Fml%2Fllm%2Fusage%2Fprovider%2FOllamaUsageProvider.kt

Closes #3395

@Desel72
Copy link
Contributor Author

Desel72 commented Mar 19, 2026

Hi @hlohaus could you review this PR and share any feedback?

@hlohaus hlohaus merged commit ccfca2f into xtekky:main Mar 20, 2026
1 check passed
@github-actions
Copy link

Review of PR Add get_quota to Ollama provider

Overall Impression

The PR adds a get_quota classmethod to Ollama.py, enabling the provider to fetch session, hourly, and weekly usage stats from the Ollama web interface. The intent is clear, but a few implementation details could be refined to improve readability, testability, and robustness.

Specific Feedback

1. Missing Import

  • The function uses Optional[str] and Optional[dict] but Optional is never imported.
    Fix: from typing import Optional

2. Cookie Retrieval Logic

  • The current logic tries an API key, then falls back to AuthManager, and finally to a cookie sourced from get_cookies.
    • The function should document the precedence clearly.
    • Consider adding a helper method ._load_cookies() to encapsulate this logic, which would also be unit‑testable.

3. debug.error Usage

  • The debug.error calls append the string “Failed to get Ollama quota:” followed by the exception object.
    • This results in a tuple rather than a string, which may print oddly.
    • Use debug.error(f"Failed to get Ollama quota: {e}").

4. HTML Parsing

  • The parsing is rudimentary, relying on string searches and regexes across possibly large HTML.
    • If performance is a concern, it’s fine, but consider using lxml or BeautifulSoup for robustness.
    • Also, the outer loop uses html.find(label) then slices +800. This could miss data if the label is near the end or if the snippet is longer than 800 characters.
    • A safer approach: search for the label and use the next few hundred chars, or use regex groups around the label.

5. Missing Type Hints on Return Value

  • The function returns a dict with keys such as "session_usage" that map to nested dicts.
    • Adding a @dataclass or a TypedDict (e.g., Quota: TypedDict) would make the return type explicit.

6. Edge Case Handling

  • If the website layout changes or the labels are localized, the method will silently return None.
    • Consider adding a debug.warn or raising a custom exception when quota fetching fails due to layout change.
    • Alternatively, expose a config flag that disables quota fetching.

7. Unused Imports

  • import re is used, but an implicit assumption exists that re is needed only for the width‑percentage extraction. This is fine.
    • However, ensure that all imports are actually needed; for instance json is imported but unused by this snippet.

8. Style & Consistency

  • The method’s name follows the get_ convention, but the cls parameter is unused except for accessing class attributes. That’s acceptable.
  • The documentation comment (docstring) is missing. Adding a brief description would aid maintainability.

Suggested Refactor Snippet

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.

Add get_quota to ollama

2 participants