Skip to content

Add encoding param to sync_item() and auto_run param to Syncer.__init__()#65

Merged
meeb merged 1 commit into
meeb:mainfrom
sariyamelody:sariyamelody/upstream-changes
Mar 21, 2026
Merged

Add encoding param to sync_item() and auto_run param to Syncer.__init__()#65
meeb merged 1 commit into
meeb:mainfrom
sariyamelody:sariyamelody/upstream-changes

Conversation

@sariyamelody
Copy link
Copy Markdown
Contributor

Summary

Two small additions to support external consumers of Syncer that need finer-grained control over the sync process:

  • auto_run=True on Syncer.__init__(): wraps the existing asyncio.run() and self.notify() calls in if auto_run:. Passing auto_run=False lets callers instantiate a Syncer to inspect purchases before deciding what to sync, without triggering a full sync immediately.

  • encoding=None on sync_item(): uses encoding or self.media_format throughout the method instead of self.media_format directly. This allows per-item format overrides and avoids a potential race condition when concurrent threads call sync_item() with different formats.

Both changes are backwards-compatible. Default behavior is unchanged.

Tests

Two new tests in tests/test_sync_item.py:

  • test_sync_item_uses_encoding_parameter -- verifies the encoding kwarg is passed through to the download URL call
  • test_sync_item_falls_back_to_media_format_when_no_encoding -- verifies default behavior is preserved

sync_item(): Accept an optional encoding parameter instead of always
using self.media_format. This fixes a race condition where concurrent
download threads could clobber each other's format setting via the
shared Syncer instance.

Syncer.__init__(): Accept an optional auto_run parameter (default True)
that controls whether sync starts immediately on construction. Setting
auto_run=False allows external consumers to instantiate a Syncer,
inspect purchases, and decide what to sync before starting.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@meeb
Copy link
Copy Markdown
Owner

meeb commented Mar 21, 2026

Thanks, I'll take a look at these. They sound fine on paper. There's been a few AI-based PRs lately and the internals are now a bit of a mess so this may need to wait until I get time to manually refactor things.

@sariyamelody sariyamelody marked this pull request as ready for review March 21, 2026 06:38
@sariyamelody
Copy link
Copy Markdown
Contributor Author

Thanks, I'll take a look at these. They sound fine on paper. There's been a few AI-based PRs lately and the internals are now a bit of a mess so this may need to wait until I get time to manually refactor things.

Super understandable - I'm in no big rush or anything, I just want to see the state of the art pushed, since god knows the world of self-hosting needs some real love - I appreciate you caring enough to be super responsive! Thank you for your time.

@meeb
Copy link
Copy Markdown
Owner

meeb commented Mar 21, 2026

Actually this is fine, I need to re-org the args to some of the functions a little but might as well bundle this in before.

@meeb meeb merged commit f7b8d42 into meeb:main Mar 21, 2026
2 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