Skip to content
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

Main fix test error + upd #1142

Merged
merged 37 commits into from
Dec 15, 2022
Merged

Conversation

SurfaceS
Copy link
Contributor

@SurfaceS SurfaceS commented Dec 13, 2022

fix error :

Seems that tmdb has no apikey set -> result null -> omdb fetch.
I put mocks on tmdb and code related to it (key provided = real fetch, no key or 'foo' key = mocks).

Omdb data seems to give a wrong series IMDbID (give episode IMDbID = series IMDbID).
I put some code to get series IMDbID from tmdb to temp fix that.
Updated the mocks with value given by the omdb server (with errored series IMDbID).

update yarn.
update deps.

@SubJunk
Copy link
Member

SubJunk commented Dec 13, 2022

@SurfaceS TMDB_API_KEY is set in GitHub so I'm not sure why that wouldn't be working:

image

image

@SurfaceS
Copy link
Contributor Author

on my code :

const apiKey = process.env.TMDB_API_KEY || 'foo';
const baseUrl = apiKey === 'foo' ? 'https://local.themoviedb.org/3/' : null;

So if it take the mocks url (I name it local.themoviedb.org), it mean TMDB_API_KEY is null or 'foo'.

@SurfaceS
Copy link
Contributor Author

Concerning the PR :
PR is ok for me.

The error in the check Series or season not found! is normal as omdb doesn't know the season 1 for 'Lost'.
OMDB seems to be a way less accurate than TMDB.

Concerning the api :

The api is indexed on imdb id's, but sometimes it is only tmdb id specially for series.
The api is english only request/reply.

I'll back on another PR about that.

@SubJunk
Copy link
Member

SubJunk commented Dec 14, 2022

We can probably just remove OMDb. Unfortunately we found it before TMDB. If we had found TMDB first, we probably wouldn't have added OMDb.
And yes I agree it would be good to fix for entries without IMDb IDs.

@SubJunk SubJunk merged commit 85f0332 into UniversalMediaServer:main Dec 15, 2022
@SubJunk
Copy link
Member

SubJunk commented Dec 15, 2022

Nice fixes thanks!

@SurfaceS SurfaceS deleted the main-test-error branch December 24, 2022 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants