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

Add Ktor builder using Resource #39

Merged
merged 16 commits into from
Jan 19, 2023
Merged

Add Ktor builder using Resource #39

merged 16 commits into from
Jan 19, 2023

Conversation

gutiory
Copy link
Collaborator

@gutiory gutiory commented Dec 23, 2022

This PR creates a function that allows to build a Ktor engine as a Resource

Copy link
Member

@nomisRev nomisRev left a comment

Choose a reason for hiding this comment

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

LGTM so far @gutiory ☺️

suspendapp-ktor/src/main/kotlin/KtorServer.kt Outdated Show resolved Hide resolved
suspendapp-ktor/src/main/kotlin/KtorServer.kt Outdated Show resolved Hide resolved
suspendapp-ktor/build.gradle.kts Outdated Show resolved Hide resolved
suspendapp-ktor/build.gradle.kts Outdated Show resolved Hide resolved
suspendapp-ktor/build.gradle.kts Outdated Show resolved Hide resolved
@gutiory gutiory requested a review from nomisRev January 9, 2023 12:46
@nomisRev
Copy link
Member

nomisRev commented Jan 9, 2023

Remove js and mingwX64 from gradle

Is this not possible? 🤔

@gutiory
Copy link
Collaborator Author

gutiory commented Jan 9, 2023

Remove js and mingwX64 from gradle

Is this not possible? 🤔

Ktor-server-core does not have support for js and mingwX64 😞

@nomisRev
Copy link
Member

Hey @gutiory,
I checked out this branch and there are a couple things broken. So let's walk through the steps why we're getting a false-positive green build.

  1. When working with Kotlin Multiplatform main is not a valid sourceSet, so this code is currently not being picked up by gradle and is not being compiled. As a result IDEA also doesn't show any red lines, as analyse (or type checking) is not occurring on the code. To fix this we need to rename the main directory to commonMain.

  2. When doing so the build will turn red, and we'll find that ApplicationEngine cannot be resolved. Currently the module is depending on io.ktor:ktor-server-core, but this seems to be the wrong artefact for ApplicationEngine in a KMP setting. Instead we need to depend on ktor-server-host-common, and this will give us access to import io.ktor.server.engine.ApplicationEngine & co from commonMain.

  3. Finally, you'll find the build is still red since we're still returning Resource<ApplicationEngine> and we wanted to return ApplicationEngine directly. In Arrow 1.1.4 this can be done by using install({ }) { app, exitCase -> ... } but we don't have that syntax available yet in 1.1.3. To quick-fix this we can just use Resource({ }) { app, exitCase -> ... }.bind(). We're however currently in the process of releasing Arrow 1.1.4/5 so we can also postpone this PR/release 1 week and use install directly.

@gutiory
Copy link
Collaborator Author

gutiory commented Jan 16, 2023

Good catch @nomisRev !. Thanks a lot for your analysis.
I'm fine waiting until the next Arrow version is release to use the new Resource syntax.

@nomisRev
Copy link
Member

@gutiory Arrow 1.1.5 is released, and the version bump has been merged into main.
So you can make the changes, we can merge this PR and release a new version of SuspendApp with the new module 🥳

gutiory and others added 14 commits January 18, 2023 16:21
Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com>
Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com>
Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com>
Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com>
Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com>
`ktor-server-host-common` is the artefact to be used in a KMP, that provides `ApplocationEngine` to the project.
As the project is a KMP, `main` is not a valid `sourceSet`. The code must be placed under `commonMain`.
The `Resource` acquire and release process is changed after last `Arrow` update. We use now the new `install` method.
`Ktor` is published as hierarchical project structure, so we need to disable this feature.
Copy link
Member

@nomisRev nomisRev left a comment

Choose a reason for hiding this comment

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

🥳 Thank you for an awesome first contribution @gutiory

@nomisRev nomisRev merged commit 2b5a88c into main Jan 19, 2023
@nomisRev nomisRev deleted the feature/suspendapp-ktor branch January 19, 2023 08:55
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