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.
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
Add tag sync support for Sonarr and Radarr #1332
base: develop
Are you sure you want to change the base?
Conversation
Are using the indexers' tags the best option for this? That can mean a lot of unecessary tags going to an app depending on how one users tags. App Tags should probably be a different option maybe, no? |
Do you mean to have a separate setting on each indexer which are the tags to transfer? I like the idea to keep them split, but I can see terminology getting very confusing. I already find the tagging to link indexer to proxy and app a bit confusing. I'm not sure it would help matters if indexer tags in prowlarr don't result as indexer tags in apps. As for unnecessary tags, I'm not sure how many tags people usually have. It should be minimised somewhat as sync is off by default and will only create tags which it needs to sync. Personally I wouldn't care about some unused tags. They're not very visible in the apps and don't affect functionality. That's just my opinion though. |
Changed to WIP I believe there is a bug noticed in thread at #500 (comment). When tag sync is off but fully indexer sync is on, the tags will be deleted from the indexer in the app. They should be left as is. |
Updated to preserve existing tags when tag sync is off. PR description updated. |
Also some tests would be nice if you'll add for these changes. |
Good catch, but I believe it is not quite that simple now. If I've worked this out correctly, that line will mean that a tag being removed in prowlarr won't be removed in the other application as that line will re-add it after the new indexer is built.
Potential options I can see are:
- Leave the retain tags line as is so that Prowlarr tags are only ever added and never removed.
- Remove the retain tags line to have it controlled by BuildSonarrIndexer. This would provide full sync, or retain tags if the sync tags setting is off (the code for this can be made more efficient to avoid multiple API calls).
- Update the code to consider tags which Prowlarr is aware of. For tags existing in both apps, perform a full sync (add and remove), for tags within applications which are NOT available in Prowlarr, leave them as they are.
- Update the tag sync setting so that it can be switched between the above options. This may get confusing as there will then be 2 "sync level" settings.
Given the discussions around this PR and the linked issue, the last option may be best to offer the most flexibility.
I started working on the tests but this and the tests might take a while to resolve so I'll put this back to WIP for now.
…On Sun, 5 Mar 2023, at 9:23 PM, Bogdan wrote:
***@***.**** requested changes on this pull request.
In src/NzbDrone.Core/Applications/Sonarr/Sonarr.cs <#1332 (comment)>:
> @@ -194,7 +203,7 @@ private SonarrIndexer BuildSonarrIndexer(IndexerDefinition indexer, DownloadProt
Implementation = indexer.Protocol == DownloadProtocol.Usenet ? "Newznab" : "Torznab",
ConfigContract = schema.ConfigContract,
Fields = new List<SonarrField>(),
- Tags = new HashSet<int>()
+ Tags = Settings.SyncIndexerTags ? GetAndCreateApplicationTagIdsForIndexer(indexer) : GetExistingIndexerTags(id)
Revert this change back and you should combine tags from prowlarr and apps if you want to here:
https://github.com/Prowlarr/Prowlarr/blob/e211436eb5375646e83daa3814d555b6120a0664/src/NzbDrone.Core/Applications/Sonarr/Sonarr.cs#L140
With `remoteIndexer.Tags` being available, `GetExistingIndexerTags` is not needed.
Same for Radarr.
—
Reply to this email directly, view it on GitHub <#1332 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZUIAIUCQTQFPNLD4WUXKTW2T76LANCNFSM6AAAAAAT4AKNKY>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
Database Migration
NO
Description
Adds tag sync support to Sonarr and Radarr.
Screenshot (if UI related)
Todos
Issues Fixed or Closed by this PR