Skip to content

[Kotlin Server] Update Ktor to latest version; move config to kts #19727

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

Merged
merged 4 commits into from
Oct 11, 2024

Conversation

e5l
Copy link
Contributor

@e5l e5l commented Sep 30, 2024

  • Update the Ktor version in the Kotlin Server template to 2.3.12
  • Migrate Ktor gradle configuration to Kotlin Script

technical committee: @stefankoppier

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh ./bin/configs/*.yaml
    ./bin/utils/export_docs_generators.sh
    
    (For Windows users, please run the script in Git BASH)
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
  • File the PR against the correct branch: master (upcoming 7.x.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@wing328
Copy link
Member

wing328 commented Oct 1, 2024

thanks for the PR

cc
@dr4ke616 (2018/08) @karismann (2019/03) @Zomzog (2019/04) @andrewemery (2019/10) @4brunu (2019/11) @yutaka0m (2020/03) @stefankoppier (2022/06)

@wing328
Copy link
Member

wing328 commented Oct 1, 2024

shall we keep modules/openapi-generator/src/main/resources/kotlin-server/libraries/ktor/build.gradle.mustache as some users may still prefer build.gradle (e.g. they've customized it with additional dependencies only available in their environment) ?

@e5l
Copy link
Contributor Author

e5l commented Oct 1, 2024

Hey @wing328, thank you for the notice.

According to our stats, less than 0.5% of users use the Groovy script, so we're considering it a legacy and not recommending it for new projects. (We can still add it if necessary.)

By the way, in Ktor support, we received feedback about the generator's confusing naming: while it's named Kotlin, it generates a Ktor server. At the same time, people are struggling to find the Ktor generator in the list. Could you tell me if we can rename it with the deprecation cycle?

@wing328 wing328 modified the milestones: 7.9.0, 7.10.0 Oct 8, 2024
@wing328
Copy link
Member

wing328 commented Oct 9, 2024

@e5l
Copy link
Contributor Author

e5l commented Oct 9, 2024

Hey @wing328, thanks for the check.
It looks like the Gradle version was outdated.

Bumped to the latest. Could you please check?

@wing328
Copy link
Member

wing328 commented Oct 10, 2024

@e5l please review the errors when you've time

maybe update gradle to 7.x instead which is still supported (not yet EOL)

@e5l
Copy link
Contributor Author

e5l commented Oct 10, 2024

Let me try to fix the errors first! It would be nice if we manage to have the latest one :)

@wing328
Copy link
Member

wing328 commented Oct 10, 2024

It would be nice if we manage to have the latest one :)

not against using the latest version

but we usually test with old versions (not yet EOL) as well to ensure it still works with the auto-generated code

@e5l
Copy link
Contributor Author

e5l commented Oct 10, 2024

Sure, let me try to stick with the latest 7.* minor so

@e5l e5l force-pushed the kotlin-ktor-server branch from ecf0985 to 042517e Compare October 10, 2024 07:12
@e5l
Copy link
Contributor Author

e5l commented Oct 10, 2024

Force pushed an update to Gradle 7.3. Could you check?

@e5l
Copy link
Contributor Author

e5l commented Oct 10, 2024

Some samples are using 6.* API, let me update them

@e5l
Copy link
Contributor Author

e5l commented Oct 10, 2024

Fixed, please check it out

@e5l
Copy link
Contributor Author

e5l commented Oct 10, 2024

Bump to fix: gradle/gradle#24390

@wing328
Copy link
Member

wing328 commented Oct 10, 2024

what about filing a separate PR just to update Ktor to latest version and we can merge that one first?

@e5l e5l force-pushed the kotlin-ktor-server branch from 117b89e to 42d103d Compare October 10, 2024 09:07
@e5l
Copy link
Contributor Author

e5l commented Oct 10, 2024

Sure, done!

@wing328
Copy link
Member

wing328 commented Oct 10, 2024

https://app.circleci.com/pipelines/gh/OpenAPITools/openapi-generator/31893

that can be ignored

if no one has further feedback, i'll merge it tomorrow (Fri)

thanks again for the PR

@e5l
Copy link
Contributor Author

e5l commented Oct 10, 2024

No problem! Feel free to add me to the Kotlin reviewers if you need volunteers :)

@wing328
Copy link
Member

wing328 commented Oct 11, 2024

No problem! Feel free to add me to the Kotlin reviewers if you need volunteers :)

Sure, will do. Thanks for joining us 👍

@wing328
Copy link
Member

wing328 commented Oct 11, 2024

According to our stats, less than 0.5% of users use the Groovy script, so we're considering it a legacy and not recommending it for new projects. (We can still add it if necessary.)

I'm totally ok to decommission it but my guess is that there will be users who still prefer it.

If that occurs, we can add it back.

@wing328 wing328 merged commit 7f899df into OpenAPITools:master Oct 11, 2024
28 of 29 checks passed
@wing328
Copy link
Member

wing328 commented Oct 11, 2024

By the way, in Ktor support, we received feedback about the generator's confusing naming: while it's named Kotlin, it generates a Ktor server. At the same time, people are struggling to find the Ktor generator in the list. Could you tell me if we can rename it with the deprecation cycle?

it's named kotlin-server:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants