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

Introduce new remoteInputSpec parameter #13727

Merged
merged 2 commits into from
Nov 7, 2022
Merged

Introduce new remoteInputSpec parameter #13727

merged 2 commits into from
Nov 7, 2022

Conversation

hdani9307
Copy link
Contributor

@hdani9307 hdani9307 commented Oct 18, 2022

To close #13726

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/utils/export_docs_generators.sh
    
    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*.
    For Windows users, please run the script in Git BASH.
  • File the PR against the correct branch: master (6.1.0) (minor release - breaking changes with fallbacks), 7.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.

@erichaagdev
Copy link
Contributor

👋 @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?

@hdani9307
Copy link
Contributor Author

Hi @erichaagdev

I wanted to generate the API from an URL like https://raw.githubusercontent.com/OpenAPITools/openapi-generator/master/modules/openapi-generator-gradle-plugin/src/test/resources/specs/petstore-v3.0.yaml in my project, but because of the @InputFile annotation in the GenerateTask, it is only accepting file urls, however the http url inputspec is supported by the generator.

@erichaagdev
Copy link
Contributor

@hdani9307 thanks for the explanation. Annotating file inputs with @InputFile is required to properly support incremental build functionality (e.g. UP-TO-DATE). You can read the Incremental Build section of the Gradle documentation to learn more.

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 inputSpec as is and instead add a new property (e.g. remoteInputSpec) which can be annotated with @Input and used only if inputSpec is not defined.

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 clean.

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.

@hdani9307
Copy link
Contributor Author

@erichaagdev If I create a new remoteInputSpec property, I will have to make the inputSpec parameter optional. Currently it is a required property. Would it cause any regression issue?

@erichaagdev
Copy link
Contributor

@hdani9307 I don't believe marking a previously required property with @Optional would break any backwards compatibility. Removing the @Optional annotation from a property and making it required would, but that's not what you would be doing.

I'd also like to propose an alternative solution - one which only requires you to modify your own build scripts. You could leverage the gradle-download-task plugin and add a task downloading the remote OpenAPI specification. You would then use the location of the downloaded spec file as the value for inputSpec.

Here is a sample showing how that could be done: https://github.com/erichaagdev/gradle-samples/blob/main/remote-openapi-spec/build.gradle.kts

@hdani9307 hdani9307 changed the title Remove @InputFile, use a simple @Input #13726 #13726 Introduce new remoteInputSpec parameter Oct 27, 2022
@hdani9307
Copy link
Contributor Author

@erichaagdev I have added the new remoteInputSpec property.

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.

@erichaagdev
Copy link
Contributor

Could you also update the configuration section of the README to mention the new property?

Copy link
Contributor

@erichaagdev erichaagdev left a 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.

@wing328
Copy link
Member

wing328 commented Nov 7, 2022

@hdani9307 thanks for the PR.

@erichaagdev thanks for reviewing the change.

@wing328 wing328 merged commit 3eec4eb into OpenAPITools:master Nov 7, 2022
@hdani9307 hdani9307 deleted the gradle_plugin_inputspec branch November 8, 2022 09:56
@wing328 wing328 changed the title #13726 Introduce new remoteInputSpec parameter Introduce new remoteInputSpec parameter Mar 1, 2023
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.

[BUG][GRADLE PLUGIN] Generate API from a remote URL
3 participants