swagger-combine

[bwalsh]

This PR contains 1 new feature:

• download swagger dependencies, apply rules and combine APIs. #1
  • combines gen3[indexd, peregrine, sheepdog] APIs with leonardo and dockstore

From README

Overview

Downloads

• gen3.indexd (files)

• gen3.peregrine (graphql)

• gen3.sheepdog (submissions)

• terra.leonardo (cluster mgt)

• dockstore (workflow)

Generates

• Single, combined swagger doc output/combined-apis.yml
• Provenance for each source (git hash or md5sum) provenance/*.commit
• Aggregation is controlled by rules: see config/combined_apis.ymls for more see swagger-combine

Tests

• It should generate a combined file
• Each of the expected sources should be present in combined file
• A provenance record for each should be present
• Each source should have an expected number of endpoints

Assuming this effort proceeds, there are several questions to consider:

• Are we satisfied with the prefix use on each endpoint?
• How do we want to address the security scheme?
• What technologies or approaches make sense to implement the gateway?

[kellrott]

This seems like a good way to start the conversation about the full AnVIL API. The exact APIs may be debated, but at least this gives us the format to have the discussion.

👍

[geoffjentry]

Did a bit of research on this topic last week and this seemed like the best path to explore in terms of providing a single, maintainable swagger. 👍

[bwalsh]

@geoffjentry thanks. any objection to merging this PR?

[geoffjentry]

@bwalsh not from me. i don't know that i'm an authoritative voice though, not sure what the process is

[mtmorgan]

Should the terra api itself be part of the combined API?

There are obvious issues, e.g., duplicate operationIds

$ grep operationId api.yml |sort|wc -l
     238
$ grep operationId api.yml |sort|uniq | wc -l
     202

and (mildly) conflicting response values

$ grep -A 1 401 api.yml
        '401':
          description: The request is unauthorized.
--
        '401':
          description: The request is unauthorized.
--
        '401':
          description: unauthorized request
--
        '401':
          description: unauthorized request
--
        '401':
          description: Proxy connection unauthorized
--
        '401':
          description: Proxy connection unauthorized
--
        '401':
          description: Credentials not provided or incorrect

but presumably merging the APIs here is a step to resolving those.

Should these be introduced as (failing) tests?

[geoffjentry]

@mtmorgan my $0.02 would be to not let perfect (or even "works") be the benefit of the good here, and to treat this as a starting point. I'd love to see people start trying to use it to discover problems and/or digging into issues like you just did to move it forward.

However if people believe that we're more likely to move forward faster by iterating on this PR prior to merging that also works for me.

[bwalsh]

@mtmorgan @kellrott @geoffjentry : merging this PR. Creating issue to track martin's request