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

[FEATURE] Generate API code from OpenSearch API Spec #139

Closed
nhtruong opened this issue Jan 27, 2023 · 4 comments · Fixed by #177
Closed

[FEATURE] Generate API code from OpenSearch API Spec #139

nhtruong opened this issue Jan 27, 2023 · 4 comments · Fixed by #177
Assignees
Labels
enhancement New feature or request

Comments

@nhtruong
Copy link
Collaborator

Is your feature request related to a problem?

opensearch-project/opensearch-clients#19

What solution would you like?

Write a generator that generates API Code that retains the features and interfaces of the current api gem from OpenSearch API Spec

What alternatives have you considered?

OpenAPI Generator could generate the entire client from scratch but the end result looks nothing like the current client.

Do you have any additional context?

This OpenSearch-Ruby API Generator will also be written in Ruby so that Ruby contributors can also contribute to the generator code.

@nhtruong nhtruong added the enhancement New feature or request label Jan 27, 2023
@nhtruong nhtruong self-assigned this Jan 27, 2023
@nhtruong
Copy link
Collaborator Author

Started working on this on my personal repo here using Mustache Template and OpenApi Parser.

@nhtruong
Copy link
Collaborator Author

nhtruong commented Feb 8, 2023

Progress Update: A Proof of Concept

The Generator at the time of this writing can generate most existing API methods (given that we have the OpenAPI Spec for each of them).

Here's what the GET _cat/indices/{index} method looks like when generated:

# SPDX-License-Identifier: Apache-2.0
#
# The OpenSearch Contributors require contributions made to
# this file be licensed under the Apache-2.0 license or a
# compatible open source license.
#
# Modifications Copyright OpenSearch Contributors. See
# GitHub history for details.

# This code was generated from OpenSearch API Spec.
# Update the code generation logic instead of modifying this file directly.

# frozen_string_literal: true

module OpenSearch
  module API
    module Cat
      module Actions
        INDICES_PARAMS = Set.new(%i[
          master_timeout
          cluster_manager_timeout
          bytes
          expand_wildcards
          health
          include_unloaded_segments
          pri
          time
          format
        ]).freeze
      
        # Returns information about indices: number of primaries and replicas, document counts, disk size, etc.
        #
        # @option arguments [String] :index - * Required * A comma-separated list of index names to limit the returned information
        # @option arguments [String] :master_timeout - DEPRECATED Explicit operation timeout for connection to master node
        # @option arguments [String] :cluster_manager_timeout - Explicit operation timeout for connection to cluster_manager node
        # @option arguments [Integer] :bytes - The unit in which to display byte values (options: b, k, kb, m, mb, g, gb, t, tb, p, pb)
        # @option arguments [String] :expand_wildcards - Whether to expand wildcard expression to concrete indices that are open, closed or both. (options: open, closed, hidden, none, all)
        # @option arguments [String] :health - A health status ("green", "yellow", or "red" to filter only indices matching the specified health status (options: green, yellow, red)
        # @option arguments [Boolean] :include_unloaded_segments - 
        # @option arguments [Boolean] :pri - If set to true segment stats will include stats for segments that are not currently loaded into memory
        # @option arguments [String] :time - The unit in which to display time values (options: d, h, m, s, ms, micros, nanos)
        # @option arguments [String] :format - A short version of the Accept header, e.g. json, yaml
        def indices(arguments = {})
          raise ArgumentError, "Required argument 'index' missing" unless arguments[:index]
      
          arguments = arguments.clone
      
          _index = Utils.__listify(arguments.delete(:index))
      
          headers = arguments.delete(:headers) || {}
          body    = arguments.delete(:body)
          params  = Utils.__validate_and_extract_params arguments, INDICES_PARAMS
          path    = Utils.__pathify '_cat', 'indices', _index
          method  = OpenSearch::API::HTTP_GET
      
      
          perform_request(method, path, params, body, headers).body
        end
      end
    end
  end
end

For reference, here's the existing method: https://github.com/opensearch-project/opensearch-ruby/blob/36e5d1dbd4a45aea504fd40284232ea30494d3dd/opensearch-api/lib/opensearch/api/actions/cat/indices.rb

A few things to note:

  • Instead of using ParamsRegistry to store an array of valid params, the generated method uses a CONSTANT to store a set of these params instead. It does the same thing (storing valid params) but more performant (set lookup vs array lookup)
  • The generated code has less params. This is due to the incomplete OpenAPI Spec used to generate it.
  • The path building logic has been streamlined using Utils.__pathify:
          path    = Utils.__pathify '_cat', 'indices', _index`

VS

          path   = if _index
                     "_cat/indices/#{Utils.__listify(_index)}"
                   else
                     "_cat/indices"
                   end

What's Next?

The main goal right now is a generator that can generate API methods for new API endpoints. So, we can put aside solving the edge cases seen in the method files for now.

Mechanism to add client-specific directives to the API Spec:

Take the GET _cat/indices/{index} endpoint above, for example. The implementation of this endpoint is a method named indices inside the cat module. These 2 bits of info are not parts of standard OpenAPI and Smithy. This POC assumed that the endpoint stores this info in x-action and x-namespace OpenAPI extensions. However, the current Smithy-to-OpenAPI converter has no way of converting a Smithy Custom Traits to OpenAPI Extensions. @Xtansia is working on a solution for this.

Generate Unit Test Specs

To reach parity with the current client, each new method must be accompanied by a simple spec like:

require 'spec_helper'

describe 'client.cat#indices' do

  let(:expected_args) do
    [
        'GET',
        '_cat/indices',
        {},
        nil,
        {}
    ]
  end

  it 'performs the request' do
    expect(client_double.cat.indices).to eq({})
  end
end

This kind of tests does not really test anything, esp for generated methods, in my opinion. However, Until the entire client is generated, we need to have one for each new method.

Generate Module Files for New Namespaces

Each module must be represented by a module file. For example, the cat namespace has this:

module OpenSearch
  module API
    module Cat
      module Actions; end

      # Client for the "cat" namespace (includes the {Cat::Actions} methods)
      #
      class CatClient
        include Common::Client, Common::Client::Base, Cat::Actions
      end

      # Proxy method for {CatClient}, available in the receiving object
      #
      def cat
        @cat ||= CatClient.new(self)
      end
    end
  end
end

Update the api.rb file with the new namespaces

The new namespaces should be programmatically added to the this list in api.rb file without affecting existing namespaces:

    def self.included(base)
      base.send :include,
                OpenSearch::API::Common,
                OpenSearch::API::Actions,
                OpenSearch::API::Cluster,
                OpenSearch::API::Nodes,
                OpenSearch::API::Indices,
                OpenSearch::API::Ingest,
                OpenSearch::API::Snapshot,
                OpenSearch::API::Tasks,
                OpenSearch::API::Cat,
                OpenSearch::API::Remote,
                OpenSearch::API::DanglingIndices,
                OpenSearch::API::Features,
                OpenSearch::API::Shutdown
    end

@dblock
Copy link
Member

dblock commented Feb 13, 2023

One reason to have rudimentary tests is to have code coverage metrics not be skewed by auto-generated code, another is to have a placeholder which can then be extended with custom specs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants