-
Notifications
You must be signed in to change notification settings - Fork 18
HTTP Client Config #229
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
HTTP Client Config #229
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks nice 👍
One thing I'm wondering about is if we can use this to remove the createClient
field in SyncStream
and PowerSyncDatabaseImpl
(since we now have a proper option for this). I've added those for tests, but it looks like we can now pass the test engine via connection options instead.
(and then as a fallback when the engine option is null, use defaultHttpClient
from the companion or just inline the HttpClient
call)
# Conflicts: # core/src/commonIntegrationTest/kotlin/com/powersync/testutils/TestUtils.kt # core/src/commonMain/kotlin/com/powersync/sync/SyncOptions.kt # core/src/commonMain/kotlin/com/powersync/sync/SyncStream.kt
# Conflicts: # core/src/commonMain/kotlin/com/powersync/sync/SyncStream.kt # gradle.properties
# Conflicts: # demos/hello-powersync/composeApp/build.gradle.kts # demos/hello-powersync/composeApp/composeApp.podspec # demos/hello-powersync/composeApp/src/commonMain/kotlin/com/powersync/demos/PowerSync.kt # demos/hello-powersync/iosApp/Podfile.lock
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me apart from some documentation nits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy with these changes, looks like a good addition to have 👍
Overview
This PR allows configuring the Ktor networking client we use for PowerSync connections.
This currently allows for a broad range of configuration, including:
Todos And Open Questions
Verify Websocket logging - this seems to currently be quite limitedWebsockets are no longer used.Gauge the impact of exposing and depending on Ktor in our public
SyncOptions
API for SwiftThe addition of this work increased the release XCFramework by 6MB. The header file increased by 6KB.
This is relatively small compared to the current sizes.
Main Branch
This Branch
The ability to use a provided client has been marked as experimental. We could mark this as a delicate API after an initial experimental release.
I could not find any issues with my current setup. I think Android Studio might apply formatting differently compared to manual
ktlint
invocations - currently using the Android studio ktlint plugin.Demo
The Ktor Logging plugin has been added to the
Supabase Todolist
demo. A sample log of the HTTP request headers is belowSwift
The Swift SDK now exposes a limited and controlled subset of plugin functionality by exposing logging PowerSync network requests. powersync-ja/powersync-swift#63