Skip to content

Fixed bugs in api_generator #269

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

Conversation

nhtruong
Copy link
Collaborator

@nhtruong nhtruong commented Feb 6, 2025

Fixed: mustache file not found error
Fixed: Wrong type due to SpecHash parsing bug
Fixed: Extra space in method description

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.

Fixed: Wrong type due to SpecHash parsing bug
Fixed: Extra space in method description

Signed-off-by: Theo Truong <[email protected]>
@nhtruong nhtruong force-pushed the fix/api_generator/workflow branch from b036348 to f72b1f6 Compare February 6, 2025 23:14
@nhtruong nhtruong changed the title Fixed mustache file not found error Fixed bugs in api_generator Feb 6, 2025
@nhtruong nhtruong marked this pull request as ready for review February 6, 2025 23:18
end

def to_s
"<SpecHash: #{@hash.to_s}>"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"<SpecHash: #{@hash.to_s}>"
"<SpecHash: #{@hash}>"

Redundant.

@@ -59,8 +70,8 @@ def parse(value)
return value unless value.is_a?(Hash)
ref = value.delete('$ref')
value.transform_values! { |v| parse(v) }
return SpecHash.new(value) unless ref
SpecHash.new(parse(resolve(ref)).merge(value))
value.merge!(resolve(ref)) if ref
Copy link
Member

Choose a reason for hiding this comment

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

Generally modifying value in-place like this is not a good idea, duplicate and return value.

Copy link
Collaborator Author

@nhtruong nhtruong Feb 7, 2025

Choose a reason for hiding this comment

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

I agree. This was done intentionally. The SpecHash is my poor-man OpenAPI parser (though 10 times faster than the official tool for ruby) that replaces every object with $ref with the object it references. The original object will be discarded and not to be used by anything else. Hence the in-place update.

@dblock dblock merged commit 29567b4 into opensearch-project:main Feb 7, 2025
37 checks passed
@nhtruong nhtruong deleted the fix/api_generator/workflow branch February 7, 2025 15:42
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