-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
Introduce new remoteInputSpec parameter #13727
Introduce new remoteInputSpec parameter #13727
Conversation
👋 @hdani9307 could you briefly explain why you would like to make this change? What issue are you experiencing that you are trying to address with these changes? |
Hi @erichaagdev I wanted to generate the API from an URL like |
@hdani9307 thanks for the explanation. Annotating file inputs with Introducing these changes as is would cause a regression for functionality that was previously added by #4492, #6472, and #6716. With that being said, you may still be able to implement the functionality you want without causing a regression or impacting backwards compatibility. What I recommend is to leave Since Gradle has no way to know if the contents of the remote spec have changed, you would need to conditionally disable caching if the new property is in-use. Proper incremental building would not be supported, but to generate a new set of classes caused by changes to the remote specification you can always run a Another option is to conditionally register an additional non-cacheable/non-incremental task which would first download the remote specification as a pre-requisite step to the generate task. This would allow the generate task to still fully support build caching and incremental building. |
@erichaagdev If I create a new |
@hdani9307 I don't believe marking a previously required property with I'd also like to propose an alternative solution - one which only requires you to modify your own build scripts. You could leverage the Here is a sample showing how that could be done: https://github.com/erichaagdev/gradle-samples/blob/main/remote-openapi-spec/build.gradle.kts |
@erichaagdev I have added the new Thank you for your alternative suggestion, we are already using the same plugin as a workaround. It works well, I just wanted to fix this bug in the lib. |
...lin/org/openapitools/generator/gradle/plugin/extensions/OpenApiGeneratorGenerateExtension.kt
Show resolved
Hide resolved
...gradle-plugin/src/main/kotlin/org/openapitools/generator/gradle/plugin/tasks/GenerateTask.kt
Show resolved
Hide resolved
...gradle-plugin/src/main/kotlin/org/openapitools/generator/gradle/plugin/tasks/GenerateTask.kt
Outdated
Show resolved
Hide resolved
...gradle-plugin/src/main/kotlin/org/openapitools/generator/gradle/plugin/tasks/GenerateTask.kt
Outdated
Show resolved
Hide resolved
modules/openapi-generator-gradle-plugin/src/test/kotlin/GenerateTaskDslTest.kt
Outdated
Show resolved
Hide resolved
modules/openapi-generator-gradle-plugin/src/test/kotlin/GenerateTaskDslTest.kt
Outdated
Show resolved
Hide resolved
Could you also update the configuration section of the README to mention the new property? |
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.
I think this looks good, but I don't have approval to merge.
@hdani9307 thanks for the PR. @erichaagdev thanks for reviewing the change. |
To close #13726
PR checklist
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*
.For Windows users, please run the script in Git BASH.
master
(6.1.0) (minor release - breaking changes with fallbacks),7.0.x
(breaking changes without fallbacks)