[script.plexmod] 0.7.8 #2620
[script.plexmod] 0.7.8 #2620
Conversation
|
Addon checker fails weirldly. |
|
The checker is working again. Still an error, can you check that and fix it? |
|
Heyho, not to push anything, but how's the cadence of merging PRs in this repo? Is it random or organized? Thanks! |
Whenever someone has the free time. |
There was a problem hiding this comment.
This is a definite no go. We do not allow addons to write into Kodi config files or databases directly. All intractions with Kodi core must be done via official APIs.
There was a problem hiding this comment.
We only use this method to write the advancedsettings.xml when there's no official API available, such as for cache settings in Kodi older than 21, and <hosts> entries. The user has full control over this and we only touch the relevant areas and don't modify other unrelated entries in that xml file.
For anything above Kodi 20, this manages the <hosts> entries for user convenience to resolve plex.direct domains to avoid DNS rebind issues for novice users who don't know how to whitelist plex.direct in their routers.
Also I can't find any guideline stating that we can't write certain files. If that's in fact the case, why does xbmcvfs allow the write operation?
If you're referring to:
add-ons should store all their data in their own subfolder inside the addon_data directory. Access (read/write/delete) to any other files / folders is not allowed by default.
Exceptions to this rule may be granted in specific cases only. Please contact Team XBMC's add-on repository maintainers on github if your add-on needs access to such files.
In case we grant such an exception for your addon, access (read/write/delete) to other files/directories must be opt-in by the user, and be clear for the user to understand what is being accessed.
from https://kodi.wiki/view/Add-on_rules#Requirements_for_all_python_addons, I hereby request this addon to be whitelisted for this special case, please.
FYI: This functionality hasn't been added in that version, it was already in the accepted 0.7.6, just buried further in the code.
Edit: Corrections
There was a problem hiding this comment.
If this was added in a previous version this was obviously an ovelook from our (probably mine) side because of the size of the original PR. Regarding granting an exempt for your addon, I'll consult with other team members.
There was a problem hiding this comment.
To clarify: I can drop the cache management support for Kodi below 21, that's fine, if that's your main concern, as we're changing a very global setting.
We need the <hosts> management part of the advancedsettings.xml, though, as the plex.direct hosts change with every IP change and it's non-trivial for the user to find those hosts using the plex.tv API.
Telling the user to whitelist plex.direct in their router's DNS rebind protection configuration doesn't really solve the issue, either, because many provider-issued routers don't allow that.
There was a problem hiding this comment.
Any news on this? Can I do anything to make this mergable? Again, I can drop the cache management stuff.
There was a problem hiding this comment.
I applogize for this taking way too long. Unfortunately, currently I have little time because of various life stuff.
So let's agree on dropping the cache management stuff. As I understand, hosts management is Plex specific and should not affect other addons. On my side I will do my best fo finish my review by this weekend.
There was a problem hiding this comment.
Thank you so much for your understanding and the offer. Would it help me dropping the 0.7.8 PR and creating a new one for 0.7.9 in about 2-3 weeks? Maybe that takes some pressure away from you, as well.
There have been so many changes and fixes in the meantime.
If not, I'll gladly branch away and make the necessary adjustments for 0.7.8.
Edit: Corrections
There was a problem hiding this comment.
I guess it's better to make a new PR with all the fresh stuff.
There was a problem hiding this comment.
@romanvm this would be the change to disallow advancedsettings.xml writing for caching when installed from the official Kodi repository: pannal/plex-for-kodi@178038e
Please confirm that this suites what we've agreed upon, thank you!
|
Closing for new PR in a couple of weeks |
Description
0.7.8
Checklist: