search

pkkid / python-plexapi

Python bindings for the Plex API.
BSD 3-Clause "New" or "Revised" License
1.14k stars 196 forks source link

State of plexapi #454

Open Hellowlol opened 4 years ago

Hellowlol commented 4 years ago

Hello everybody.

plexapi haven't gotten the attention it need for quite some time. PRS have been unmerged, test suite is broken.

Going forward I would like to fix this. I would like to get some input from any stakeholders (or any other for that matter) regarding this. @pkkid @blacktwin @andrey-yantsen @jjlawren

Short time plan:

Some time in a not so distant future:

andrey-yantsen commented 4 years ago

Good to see that you're still interested in the project, awesome! The plan sounds great as well.

As for tests — one of the issues here is that we're downloading required content for the library for every run, and sometimes it's just failing. Some other times — we're unable to properly prepare the library and understand that it's basically ready... In my other plex-related project (plex-api.rs, the API written in Rust) I've gone in slightly another way: the Plex library (the database and all the required media) is stored inside the repo, hence bootstrap is much faster and stable, but I had to sacrifice some disk space to it.

It's just a suggestion, feel free to choose the better path :)

Hellowlol commented 4 years ago

@andrey-yantsen thanks for the feedback. Ill stub the files to skip the download. That’s a great idea. I wonder why picked so large files..

andrey-yantsen commented 4 years ago

I was young, sorry :D Feel free to steal the database from plex-api.rs, there is no copyright, the media — are just files with white noise. You can find an example on how to bootstrap plex-server in my github actions workflow.

pkkid commented 4 years ago

I like the plan.

To me, getting the tests more reliable is a top priority. If those are running consistently and reliably, we can be more confident and move faster on all other changes. -- I did try to take a stab at fixing some of the flakyness in the tests. It seems during test setup when it downloads and configures the temp Plex client, it doesn't always see or register all media we are giving it. I ended up scraping work in the area because it didn't end up helping anything. I like the idea of storing test media in the repo, it's one less variable we have to worry about not working. -- Testing the tests is an annoying area because we have to push the changes to get Travis to execute the scripts. Maybe it'll feel cleaner if we do this on another branch.

Not sure what the Black linter is so I have no comments. I always use flake8.

At this point, I am all for dropping Python2 support in 4.0. I feel like this will be pretty easy actually since we already support both. It'll be pretty much deleting the compat.py file.

Hellowlol commented 4 years ago

@pkkid Good news. We are really close regarding the test there are just two or three more that i haven't figured out why it fails on travis as the pass on my local tests

Next up after then is to change to local files then push a release.

Black is the psf formater. Basically it reformat the code for you so its always the consistent way in all projects. https://github.com/psf/black

pkkid commented 4 years ago

Ahh its a formatter (not a linter). I've always been against formatters myself. My reasons mostly include violating strict rules in certain circumstances to make things more readable than a machine does. However, I do realize not everyone agrees with my personal formatting style and would be willing to submit my opinion to the opinion of others for this project. -- Can I suggest we try this on a branch so we can visually inspect its output before changing master?

jjlawren commented 4 years ago

Black is a very good and consistent formatter. Home Assistant (a user of plexapi) uses it across the project and even enforces black formatting in a pre-commit hook. I think I've only had to override it when also annotating linting exceptions.

Hellowlol commented 4 years ago

@pkkid yep

Hellowlol commented 4 years ago

Test pass, and I just noticed the coverage hasn’t been updated since 2017... Travis seems to create the coverage report. Does anybody know why it isn’t being uploaded?

andrey-yantsen commented 4 years ago

@Hellowlol seems like it was me again: https://docs.coveralls.io/parallel-build-webhook, we need to add a web-hook to Travis' config.

jjlawren commented 4 years ago

Test pass, and I just noticed the coverage hasn’t been updated since 2017... Travis seems to create the coverage report. Does anybody know why it isn’t being uploaded?

Is this what we're looking for? https://coveralls.io/jobs/61530792.

It looks like a report is uploaded, but it's not being associated with the repo?

Hellowlol commented 4 years ago

Yep. Seems strange

andrey-yantsen commented 4 years ago

Nothing strange, actually - when opening the link you can see that the project was identified successfully, but the results just weren’t “merged”.

We have “parallel” builds enabled, but Travis doesn’t send any signals to coveralls, for the latter to identify that the build is finished and it’s actually the time to combine all the received results.

The fix is covered in the docs, but I’ve failed to add the webhook when was enabling the parallel builds in coveralls.

Hellowlol commented 4 years ago

@pkkid Can you take a look on how we build the docs? I would like them to update automatically.

jjlawren commented 4 years ago

I think the open issues could use a purge. Many have been sitting open with no activity for months (or years!).

Hellowlol commented 4 years ago

I can't figure out why new modules isnt included in the docs that gets released on readthedocs, they seems to be built when i look at the build history. Anybody wanna help me figure this out?

jjlawren commented 4 years ago

Things are looking far better now. I still see a few lingering issues related to the CI where I could really use some help to address:

  1. The tests still fail sporadically related to incomplete/wrong metadata. Recent example: https://travis-ci.org/github/pkkid/python-plexapi/jobs/726423384#L828-L862
  2. The Travis results are not pushed back consistently, so PRs show an empty "Checks" tab. Oddly some of the commits to master show test run results: https://github.com/pkkid/python-plexapi/commits/master

It seems like issue 1 is perhaps a side-effect of failed or slow metadata agents. These same tests work consistently on my local test server after manually refreshing & unmatching, so the upstream metadata still seems sane.

jjlawren commented 4 years ago

I created a canary test over in #569 and validated that we do sometimes start the test runs while the PMS docker instance is still getting settled. Here's the failed run which checks the /activities endpoint early in the tests and encountered a running metadata agent: https://travis-ci.org/github/pkkid/python-plexapi/jobs/727432667#L839-L846

I've updated the test to wait up to 60s for PMS activities to complete before continuing the runs. Doesn't add time to the test runs when things are running smoothly.