Create background Hub clients only if necessary#337
Create background Hub clients only if necessary#337atdrendel wants to merge 1 commit intohuggingface:mainfrom
Conversation
| ) | ||
| ) : self.foregroundUncachedClient | ||
| #endif | ||
| NetworkMonitor.shared.startMonitoring() |
There was a problem hiding this comment.
We do not need to manually call startMonitoring() because NetworkMonitor.init() calls it.
There was a problem hiding this comment.
This seems like the correct fix — especially if we're keeping shared instance around. Related discussion in #259.
There was a problem hiding this comment.
Pull request overview
This PR adjusts HubApi initialization so background URLSession-backed Hub clients are only created when useBackgroundSession: true, avoiding system log noise in apps lacking the required entitlement. It also refactors NetworkMonitor usage to a singleton to prevent duplicate initialization.
Changes:
- Conditionally build background Hub clients only when
useBackgroundSessionis enabled; otherwise reuse the foreground clients. - Switch
NetworkMonitorto a shared singleton with a private initializer and remove the redundant externalstartMonitoring()call.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
This PR reduces unwanted system log errors by preventing background URLSession creation unless explicitly requested, and consolidates network monitoring to a single shared instance.
Changes:
- Conditionally build background
HubClientinstances only whenHubApi.init(useBackgroundSession: true)is used; otherwise reuse foreground clients. - Remove redundant
NetworkMonitorinitialization by usingNetworkMonitor.sharedand making its initializer private.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks for opening this, @atdrendel! There's a formatting issue that's failing early in CI. I tried to apply the change, but it looks like your fork isn't configured to allow changes from downstream, so I opened #338 with that. |
|
Just merged #338 with these changes. Thanks again, @atdrendel! |
|
This is now available in swift-transformers 1.2.1 |
This pull request does two things:
HubApi.initis called withuseBackgroundSession: true, otherwise reuse the foreground sessionsNetworkMonitorand remove its duplicate initializationWhen background URL sessions are initialized in applications that do not include the com.apple application-identifier entitlement, the system logs an error.
After applying this fix, the error is no longer logged.