-
Notifications
You must be signed in to change notification settings - Fork 13
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
Feature/get current config #14
base: master
Are you sure you want to change the base?
Conversation
@PierreMesure Very curious to hear what you think about it, I've spent quite a bit of time on it spread over the last months. I think it is in a state where it warrents a serious review. Let me know how you want to approach this/what I can do to help. |
I see in api.py there seems to be some provisions for accepting an API key rather than relying on access to initialize.js and, by extension, authentication being disabled. This would be a super useful feature and would resolve my problem in #13. If I am wrong and it is fully implemented, how can this be used? I attempted to set a That aside, this looks like some amazing work (albeit mostly beyond my understanding) and seems to fix a number of my gripes with Flemmarr. Thank you for your efforts! |
Hi @tigattack you were on the right track indeed. In the beginning i'd added support to the api class to be able to take an api_key. Later on i'd forgotten about this and it wasn't passed through properly. With a small fix I just did, you should now be able to specify |
Excellent work @RynoM A couple comments/questions.
|
Thanks again, working great in most cases now! One thing I've noticed is when applying a set of quality profiles, if they already exist I see the following in Sonarr's events (haven't tested other arr's yet):
Flemmarr's output shows:
I imagine this is because the qualityprofile ID must be I'm not sure how easy this would be to work around, but I thought I'd mention it anyway. |
Thanks for the kind words!
Also whether Flemmarr should actively delete unconfigured resources is an open question imo, it might be kind of unexpected/aggressive if not stated clearly beforehand. |
I think setting a default numbers of backups (last
I personally think this feature is great and strongly believe it should be kept, but maybe it could be configurable via an environment variable for those who don't want it. |
@tigattack This is indeed true and the reason I had to re-write a lot haha. But this should be solved by getting and comparing with the current config when applying a new one (and using the id's of the current config). Also its kind of weird that its saying its updating qualityprofile/0 three times, I'd expect /0 /1 and /2 if there were three. Can you maybe share that part of your config? But good to hear it (mostly) works, let me know if you spot any other oddities. @gottsman Just updated the name of the example config and added a timestamp to the backup file. The 'last n' is a nice-to-have maybe for later :) |
I've been following the progress, this looked like a tricky one to crack for sure!
Yeah, I thought the same... I should note that I've added more profiles and amended some of the existing ones since my original comment, but it's not changed anything in regards to the original issue and snippets I posted. |
@tigattack It looks like the backup you posted was created using an older version, still including 'id' everywhere (these are now no longer added/needed for the config). And somehow there ended up being 5 with 'id: 0'. If you either make a new backup or remove the (nested) 'id' keys everywhere, I think it should work as expected. |
You're right, entirely my mistake, apologies. I built my config based on API requests and clearly got some bits wrong! I've amended my config to remove the IDs, and it matches the schema of the backup now. However, I now get essentially the same error, albeit with the expected profile IDs:
Diff'ing my config.yml and config_backup.yml shows some interesting results. It appears that the profile item are in a different order in the backup. If I copy/paste the I've chucked the Perhaps I'm misunderstanding how this works, or have made another mistake, but I'm a bit confused by this. |
No worries :). The logs you're seeing now aren't necessarily errors. I put this statement in when I got statuscode 400 when trying to update something like Lidarr's metadataprofile 'None' (default one), to which Lidarr replied I couldn't because it was a 'reserved' item (so some things cant be updated). Maybe theres other instances when this happens too. The ordering of the lists has changed because now before saving, the backup is sorted by 'id', such that when we retrieve the current settings and also sort them by 'id', there the biggest change that things line up such that we don't have to update much. But practically this is kind of an optimization and shouldn't matter much. But its kind of weird that you're seeing this log for multiple qualityprofiles, if it was qualityprofile/0 i'd maybe understand. I will add an actual logger with different loglevels soon and add some debug statements here so that we can see what the actuall request and response looks like here. |
Maybe too many changes to list, but most importantly: