Skip to content
This repository has been archived by the owner on Nov 12, 2024. It is now read-only.

Migrate to moviebaseapp/tmdb-api #1135

Merged
merged 13 commits into from
Mar 8, 2023
Merged

Migrate to moviebaseapp/tmdb-api #1135

merged 13 commits into from
Mar 8, 2023

Conversation

chrisbanes
Copy link
Owner

@chrisbanes chrisbanes commented Mar 2, 2023

Fixes #1092

@ChrisKruegerDev
Copy link
Contributor

ChrisKruegerDev commented Mar 3, 2023

Two things to add in the TMDB lib:

For TMDB, there isn't a request limit anymore. So we don't need an extra retry mechanism. See https://developers.themoviedb.org/3/getting-started/request-rate-limiting

However, you may want to handle exceptions from the ktor request (ResponseException).

@chrisbanes
Copy link
Owner Author

For TMDB, there isn't a request limit anymore. So we don't need an extra retry mechanism. See https://developers.themoviedb.org/3/getting-started/request-rate-limiting

Nice. The retry handling was more for Trakt iirc.

Another thing I was thinking about was re-using my OkHttp client. Currently the creation of the Ktor client is private in Tmdb3, so I can't set the http engine. Maybe we could add a (userConfig: HttpClientConfig) -> HttpClient constructor param?

@chrisbanes chrisbanes changed the title Try out @MoviebaseApp/tmdb-api Migrate to moviebaseapp/tmdb-api Mar 4, 2023
@chrisbanes chrisbanes marked this pull request as ready for review March 4, 2023 08:30
@chrisbanes chrisbanes enabled auto-merge (rebase) March 4, 2023 08:31
@ChrisKruegerDev
Copy link
Contributor

Another thing I was thinking about was re-using my OkHttp client. Currently the creation of the Ktor client is private in Tmdb3, so I can't set the http engine. Maybe we could add a (userConfig: HttpClientConfig) -> HttpClient constructor param?

I thought something similar. For the entire OkHttp engine support, we need to open the library more. We can pass your own HTTP client instance and add the TMDB config on top.

https://ktor.io/docs/http-client-engines.html#okhttp

@chrisbanes
Copy link
Owner Author

For the entire OkHttp engine support, we need to open the library more. We can pass your own HTTP client instance and add the TMDB config on top.

Not necessarily. We can just rely on the implicit engine discovery (https://ktor.io/docs/http-client-engines.html#default), which is what I'm using right now. The important part is being able to configure the client underneath, through the engine block.

It should work with ChrisKruegerDev/tmdb-kotlin#27 like so (although I haven't tested it):

val tmdb = Tmdb3(apiKey) {
    engine {
        if (this is OkHttpConfig) {
            //  okhttp stuff
        } else ...
    }
}

@ChrisKruegerDev
Copy link
Contributor

ChrisKruegerDev commented Mar 4, 2023

Yes, this should work 👍 It feels a bit risky to rely on the selected engine from the current source set. Ktor looks for the first engine:

public actual fun HttpClient(
    block: HttpClientConfig<*>.() -> Unit
): HttpClient = engines.firstOrNull()?.let { HttpClient(it, block) } 

I added the following config ChrisKruegerDev/tmdb-kotlin#28. I haven't tested the code, but it should work:

val tmdb = Tmdb3 {
            httpClient(OkHttp) {
               engine {
                    // for own client
               }
            }

            httpClient {
                // for custom config
            }
        }

auto-merge was automatically disabled March 8, 2023 14:32

Rebase failed

@chrisbanes chrisbanes enabled auto-merge (squash) March 8, 2023 14:59
# Conflicts:
#	data/test/src/test/java/app/tivi/data/DatabaseTest.kt
@chrisbanes chrisbanes merged commit 354c7f0 into main Mar 8, 2023
@chrisbanes chrisbanes deleted the cb/tmdb-kmm branch March 8, 2023 16:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate from tmdb-java
2 participants