Skip to content

Support session_factory arg for BaseNetboxClient#65

Merged
azryve merged 4 commits intomainfrom
feature/client-sync-session-factory
Oct 27, 2025
Merged

Support session_factory arg for BaseNetboxClient#65
azryve merged 4 commits intomainfrom
feature/client-sync-session-factory

Conversation

@azryve
Copy link
Copy Markdown
Contributor

@azryve azryve commented Oct 27, 2025

The size of different options in BaseNetboxClient is growing.
Instead of passing down every single possible option its better to provide a factory.

Passing internal session instance to a callable allows to:

  • wrap default session in some mixin (requests_cache for example)
  • override default behaviour in some other way

@azryve azryve force-pushed the feature/client-sync-session-factory branch from 7a81b19 to d1745e7 Compare October 27, 2025 14:11
@azryve azryve changed the title Draft: Support session_factory arg for client_sync.BaseNetboxClient Draft: Support session_factory arg for BaseNetboxClient Oct 27, 2025
The size of different options in BaseNetboxClient is growing.
Instead of passing down every single possible option its better to provide a factory.
@azryve azryve force-pushed the feature/client-sync-session-factory branch from d1745e7 to 3312c79 Compare October 27, 2025 14:21
The format fix is too ugly imo:

         ssl_context: SSLContext | None = None,
-        session_factory: Callable[[ClientSession], ClientSession] | None = None,
+        session_factory: Callable[[ClientSession], ClientSession]
+        | None = None,
@azryve azryve changed the title Draft: Support session_factory arg for BaseNetboxClient Support session_factory arg for BaseNetboxClient Oct 27, 2025
@azryve azryve marked this pull request as ready for review October 27, 2025 14:38
Comment thread .ruff.toml Outdated
@@ -1,4 +1,4 @@
line-length = 79
line-length = 80
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@azryve azryve merged commit 118a462 into main Oct 27, 2025
3 checks passed
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.

2 participants