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

Enhanced API generator to generate imports and headers #444

Closed
wants to merge 2 commits into from

Conversation

saimedhi
Copy link
Collaborator

Description

Enhanced API generator to generate imports and headers

Issues Resolved

Closes #348

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.

Signed-off-by: saimedhi <saimedhi@amazon.com>
Signed-off-by: Sai Medhini Reddy Maryada <117196660+saimedhi@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Jul 20, 2023

Codecov Report

Merging #444 (3984fd9) into main (4dba35d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #444   +/-   ##
=======================================
  Coverage   71.49%   71.49%           
=======================================
  Files          81       81           
  Lines        7668     7668           
=======================================
  Hits         5482     5482           
  Misses       2186     2186           

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

This is way too hacky ;) Suggestions below?

@@ -15,7 +15,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Added `Search#collapse` ([#409](https://github.com/opensearch-project/opensearch-py/issues/409))
- Added support for the ISM API ([#398](https://github.com/opensearch-project/opensearch-py/pull/398))
- Added `trust_env` to `AIOHttpConnection` ([#398](https://github.com/opensearch-project/opensearch-py/pull/438))

- Enhanced API generator to generate imports and headers ([#444](https://github.com/opensearch-project/opensearch-py/pull/444))
Copy link
Member

Choose a reason for hiding this comment

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

Change to "Added generating imports and headers to API generator"


# "CODE IS GENERATED" header is added below the license header in each generated module

header_separator = "# -----------------------------------------------------"
Copy link
Member

Choose a reason for hiding this comment

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

I don't quite understand what's going on here ;) If it's a license header, just load it from a file and add it? There are only 2 possible license headers: new files with SPDX only, existing files with ES license grant.

with open(self.filepath, "w") as f:
f.write(self.header)
if update_header is True:
f.write(self.header[:License_position])
Copy link
Member

Choose a reason for hiding this comment

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

Load this from a file and write it.

for api in self._apis:
f.write(api.to_python())

# Added "delete point in time" API manually to prevent breaking changes
Copy link
Member

Choose a reason for hiding this comment

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

Yikes, this seems like a huge hack. We need to find a better way, for example by putting generated APIs into generated/ and adding patches/ folder that includes those APIs that should not be generated. You can use file names to decide which one needs to be which.

@saimedhi saimedhi marked this pull request as draft July 31, 2023 18:31
@saimedhi saimedhi closed this Aug 2, 2023
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.

[FEATURE] Enhance the existing code generator to use the OpenAPI spec as the input format
2 participants