Enhanced Generator Code to generate Endpoints using OpenSearch API Specifications #194
Conversation
saimedhi
requested review from
VachaShah,
dblock,
harshavamsi and
shyim
as code owners
…ecifications Signed-off-by: saimedhi <saimedhi@amazon.com>
saimedhi
force-pushed
the
generator_changes
branch
from
3ae14bd
to
6e44615
Compare
|
@shyim, @dblock, @VachaShah please take a look. Thank you.
|
There was a problem hiding this comment.
This looks pretty good, I'd gladly merge it as is.
Generally try to get rid of any code that tries to edit code in-place. Generated files should be generated from a template and have a "THIS FILE IS DYNAMICALLY GENERATED" warning so nobody edits them. An ideal generator takes the spec, a bunch of templates, then dumps all output. No fixing, replacing, or editing of anything in-place.
| @@ -4,6 +4,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) | |||
|
|
|||
| ## [Unreleased] | |||
| ### Added | |||
| - Enhanced Generator Code to generate Endpoints using OpenSearch API Specifications ([#194](https://github.com/opensearch-project/opensearch-php/pull/194)) | |||
There was a problem hiding this comment.
Nit: "Enhanced Generator Code " is kinda redundant, simplify "Generate endpoints from OpenSearch OpenAPI spec" or something like that.
| @@ -164,6 +359,8 @@ | |||
|
|
|||
| printf("Copying the generated files to %s\n", $destDir); | |||
| cleanFolders(); | |||
| fix_license_header($outputDir . "/Namespaces"); | |||
| fix_license_header($outputDir . "/Endpoints"); | |||
There was a problem hiding this comment.
Is this a 1-time thing? Meaning once the headers are fixed, will this ever run?
Generally all code should be generated from a template that already has the license header, and be 1-way, overwriting whatever we had before, rather than trying to patch/merge existing files.
There was a problem hiding this comment.
-
If the endpoint exists before and have header I am keeping the same header to the newly generated corresponding endpoint. Relavant code is here.
- This is necessary because the existing files contain two types of headers. For example, security endpoints have an OpenSearch license header, while cat endpoints have an Elasticsearch license header.
-
At last after entire generation is done, checking if any files doesnot have header and adding opensearch license header for it by calling fix_license_header. Corresponding code is here.
-
Please let me know if any changes are needed.
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| * | ||
| * Elasticsearch PHP client |
There was a problem hiding this comment.
OpenSearch client.
Keep the licensed to ES part for all existing files, but it should not be added to new ones.
There was a problem hiding this comment.
Hey @dblock, after this PR gets merged, I'll be changing Elasticsearch PHP client to OpenSearch PHP client in all 279 files at once. This should make it easier for reviewers. Let me know if any more changes are needed for this PR to get merged. Thank you.
Description
Enhanced Generator Code to generate Endpoints using OpenSearch API Specifications.
Issues Resolved
Related to #97
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.