repo-scripts icon indicating copy to clipboard operation
repo-scripts copied to clipboard

[script.plexmod] 0.7.6

Open pannal opened this issue 2 years ago • 21 comments

Description

PlexMod (for Kodi) This is a fork of the official open-source Plex client for Kodi "plex-for-kodi" (Plex4Kodi) maintained by me (pannal). The official client was abandoned years ago, this fork is over 800 commits ahead and supports all Kodi versions since Leia/18.

This submission has been agreed upon in advance with Plex Inc's legal team.

Source, Discussion

Checklist:

  • [x] My code follows the add-on rules and piracy stance of this project.
  • [x] I have read the CONTRIBUTING document
  • [x] Each add-on submission should be a single commit with using the following style: [script.foo.bar] 1.0.0

Ref: https://github.com/xbmc/repo-scripts/pull/2565

pannal avatar Jan 02 '24 13:01 pannal

@romanvm regarding your comment in the old PR and its size: You already have the base repository of this addon in your Kodi repository, script.plex. If it helps, you can significantly reduce the amount to review by just reviewing the changes between script.plex and script.plexmod.

pannal avatar Jan 02 '24 13:01 pannal

@romanvm how are we looking with merging this? The reason I ask is that 0.7.5 will be released tomorrow and will probably enter stable state immediately. Does it make sense to update this PR to 0.7.5, or would that significantly delay the merge?

Thanks!

pannal avatar Feb 09 '24 23:02 pannal

Spoke w/ @romanvm and he said he will review it when he has time piece by piece. Appreciate you continuing this! If you want an invite to our slack as a guest, email me at Keith-at-kodi.tv and I can send it over.

@ruuk sounds like we should remove the old one from the repo, if you haven't already?

keithah avatar Feb 13 '24 16:02 keithah

I started the review but the process is not very fast because GitHub PR review UI is painfully slow with such big number of files. And this is considering the fact, that I have a very good computer with AMD Ryzen 7 8 cores and 32MB RAM.

I see that you are using asyncio module. Please read this section of Kodi Wiki and implement the required workaround: https://kodi.wiki/view/Python_Problems#asyncio

romanvm avatar Feb 13 '24 19:02 romanvm

Spoke w/ @romanvm and he said he will review it when he has time piece by piece. Appreciate you continuing this! If you want an invite to our slack as a guest, email me at Keith-at-kodi.tv and I can send it over.

@ruuk sounds like we should remove the old one from the repo, if you haven't already?

I'll have to find out. This is the first I'm hearing about this. I don't know of any formal decision to abandon the app. (FWIW, I'm personally happy to see pannal's version in the repo.)

ruuk avatar Feb 13 '24 22:02 ruuk

I'm not aware of any official plans to remove the original addon, either. I went through a lengthy process to get this one approved, especially through the legal issues (naming). I'm sure it has been discussed internally but I haven't heard anything.

I see that you are using asyncio module. Please read this section of Kodi Wiki and implement the required workaround:

I'll check, but wouldn't the original addon have to do the same? I'm fairly certain I haven't added that dependency, but I'll double check.

@keithah @romanvm so should I go ahead and update this PR with the latest stable, or would that make your current review harder?

Thanks for taking care of this huge merge!

pannal avatar Feb 13 '24 23:02 pannal

I'll check, but wouldn't the original addon have to do the same? I'm fairly certain I haven't added that dependency, but I'll double check.

@pannal According to the diff you have provided imcplib package was added in this addon. And the original addon exists for a long time, probably since Python 2 times, so I doubt that it uses async code with asyncio library.

romanvm avatar Feb 14 '24 08:02 romanvm

@pannal According to the diff you have provided imcplib package was added in this addon. And the original addon exists for a long time, probably since Python 2 times, so I doubt that it uses async code with asyncio library.

Ah yeah, you're right.

pannal avatar Feb 14 '24 08:02 pannal

Updated with 0.7.5 and your asyncio suggestion. Diff to script.plex: https://github.com/plexinc/plex-for-kodi/compare/master...pannal:plex-for-kodi:develop_kodi21#files_bucket (the asyncio change is not in that diff yet, only in this PR)

pannal avatar Feb 14 '24 11:02 pannal

Updated with 0.7.5 and your asyncio suggestion.

I'd recommend to add the asyncio workaround at the very top level, that is, to both of your addon entrypoints: default.py and plugin.py before all other imports, unless you absolutely sure that one of them won't start asyncio event loop so that entrypoint can be left unchanged.

romanvm avatar Feb 14 '24 20:02 romanvm

None of them will, they both run the same entrypoint in main.py, and we don't use the asyncio features of icmplib.

pannal avatar Feb 15 '24 08:02 pannal

Ok then as long as it won't interfere with other addons using asyncio.

romanvm avatar Feb 15 '24 15:02 romanvm

@keithah I've sent you an email, did you receive it? Thanks!

pannal avatar Feb 16 '24 08:02 pannal

Appreciate you continuing this! If you want an invite to our slack as a guest, email me at Keith-at-kodi.tv and I can send it over.

Did you receive my email? Or wasn't this meant for me?

pannal avatar Feb 22 '24 01:02 pannal

@ruuk sounds like we should remove the old one from the repo, if you haven't already?

I've talked to people and the ball is rolling on officially sunsetting the add-on. Hopefully it's not an issue to leave it where it is and allow it to coexist for a bit. There's kind of a process here for handling this :slightly_smiling_face:.

ruuk avatar Feb 22 '24 01:02 ruuk

Appreciate you continuing this! If you want an invite to our slack as a guest, email me at Keith-at-kodi.tv and I can send it over.

Did you receive my email? Or wasn't this meant for me?

It was for you! I don't see the email. Can you try again? Keith@Kodi dot tv

keithah avatar Feb 22 '24 02:02 keithah

@romanvm would it be OK for you if I push 0.7.6 into this PR? It fixes one of the biggest issues that P4K first time users have - the necessity to disable DNS rebind protection in their DNS server for the plex.direct domain.

pannal avatar Mar 02 '24 14:03 pannal

Only if you provide a diff with changes so I won't have to go over all the files.

romanvm avatar Mar 02 '24 21:03 romanvm

Of course, here you go: https://github.com/pannal/plex-for-kodi/compare/v0.7.5...pannal:plex-for-kodi:tmp_0.7.6-rev2?expand=1

pannal avatar Mar 02 '24 22:03 pannal

Thank you so much!

pannal avatar Mar 03 '24 02:03 pannal

I won't. Thank you for sticking with me and the very frequent release-cycle I'm on right now.

pannal avatar Mar 09 '24 01:03 pannal

Thank you again!

pannal avatar Apr 05 '24 09:04 pannal