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.

Sign up for GitHub

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

Jump to bottom

feat: add user agent #57

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

feat: add user agent #57

wants to merge 3 commits into from

Conversation

DominicGBauer
Copy link
Contributor

@DominicGBauer DominicGBauer commented Sep 20, 2024 •
edited
Loading

Description

Add User-Agent header to requests to identify the sdk being used.

Note: Currently this will not differentiate based on the Kotlin or Swift SDK and will only show Kotlin SDK in the user agent. This is because we need to:

  1. Update publish plugin to publish individual modules
  2. Create a separate build and publish github workflow for the Swift SDK and add -Pbuildkonfig.flavor=swift-sdk as a flag.
  3. This flag will then change the Library to reflect powersync-swift instead of powersync-kotlin

Work Done

  • Added the User-Agent header to requests
  • Added BuildKonfig module which will be used to differentiate between Kotlin and Swift sdks.
  • Removed User-Id header

Testing

image

@DominicGBauer DominicGBauer self-assigned this Sep 20, 2024
rkistner
rkistner reviewed Sep 20, 2024
View reviewed changes
core/src/commonMain/kotlin/com/powersync/sync/SyncStream.kt Outdated
@@ -174,6 +180,7 @@ internal class SyncStream(
headers {
append(HttpHeaders.Authorization, "Token ${credentials.token}")
append("User-Id", credentials.userId ?: "")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While you're working on headers, could you also remove the "User-Id" one in both places? It is never used by the service, and not included by other SDKs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rkistner rkistner rkistner left review comments

@stevensJourney stevensJourney Awaiting requested review from stevensJourney

At least 1 approving review is required to merge this pull request.

Assignees

@DominicGBauer DominicGBauer

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DominicGBauer @rkistner