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

Add openAPI spec for Connector Builder Server #17535

Merged
merged 11 commits into from
Oct 18, 2022
246 changes: 246 additions & 0 deletions connector-builder-server/src/main/openapi/openapi.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,246 @@
openapi: 3.0.0
info:
description: |
Connector Builder Server API

version: "1.0.0"
title: Connector Builder Server API
contact:
email: contact@airbyte.io
license:
name: MIT
url: "https://opensource.org/licenses/MIT"
externalDocs:
description: Find out more about Connector Builder
url: "https://docs.airbyte.com/connector-development/config-based/overview/"

paths:
/v1/stream/read:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@girarda @brianjlai see this commit for the changes to this endpoint coming from our Backlog Grooming discussion: 2b52fb0
Here's a screenshot of the updated Swagger view of this spec since it's a little easier to see what the final product looks like there:
Screen Shot 2022-10-04 at 1 13 16 PM

Copy link
Contributor Author

@lmossman lmossman Oct 4, 2022

Choose a reason for hiding this comment

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

I tried a nested grouping here of pages nested inside of slices, with a sliceId and pageId in each nested object.

What do you think of this structure? I thought it made sense because if a user is doing both stream slicing and pagination, then seeing each page broken down by slice makes sense. If they are not using one of those, then there will just be one element in that level, e.g. if they are not using pagination then there will just be a single element in pages.

I also wasn't super sure about sliceId and pageId -- I figured we wanted some way to identify which slices and which pages are which, but I wasn't sure if we have something to use as the "IDs" here. Since they are just strings, maybe we can just put in the URL parameter / header that changes for each slice/page?

Copy link
Contributor

Choose a reason for hiding this comment

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

in practice, stream slices correspond to states, so we could identify them using the state object, for example:

{
"repository": "airbytehq/integration-test",
"created_at": "2021-06-29T03:44:45Z"
}

we don't currently persist or keep track of the number of pages within a slice, but each page corresponds to a single request/response so we could identify them using a counter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, I think that makes sense as a way to differentiate them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided to just remove the sliceId and pageId fields from this. I didn't feel like they would add much value, because the slice "state" can be extracted from the results, and the page number is basically just the index of the page in the pages array

Copy link
Contributor

Choose a reason for hiding this comment

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

the slice "state" can be extracted from the results
the slice represents a range ("2021-05-29T03:44:45Z" - "2021-06-29T03:44:45Z"), whereas the output state only represents the end state.
Do we only care about showing the end state here?

Copy link
Contributor

Choose a reason for hiding this comment

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

should we add endpoints for check and discovery? An alternative would be to specify the operation as part of the request, but I think they're different enough to warrant different endpoints

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note from discussion: check/discover/spec endpoints can be added in a later phase. Phase 1 will only be focused on read. Will add a note about this to the tech spec as well

post:
Copy link
Contributor

Choose a reason for hiding this comment

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

how come we want these endpoints defined as POST calls? Because we're using a request body? Or I guess a real sync is sort of a POST call since the end result has records written to a destination, even if the test one doesnt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I was sort of following the convention in our main Airbyte API where we use post calls and pass all of the inputs in the request body. Especially for this API, since we need to pass the entire connector definition contents to the server I felt like that needed to go in a request body as opposed to being something like a URL parameter. And we can't have a request bodies for get requests

summary: Reads a specific stream in the source
requestBody:
content:
application/json:
schema:
$ref: "#/components/schemas/StreamReadRequestBody"
required: true
responses:
"200":
description: Successful operation
content:
application/json:
schema:
$ref: "#/components/schemas/StreamRead"
"400":
$ref: "#/components/responses/ExceptionResponse"
"422":
$ref: "#/components/responses/InvalidInputResponse"
/v1/streams/list:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This endpoint solves two problems listed in the tech spec:

  1. (tech spec link) allows the webapp to populate the stream dropdown without having to manually parse the connector definition yaml file
  2. (tech spec link) provides the URLs for each stream so that the webapp does not need to manually parse the connector definition yaml file

Since the backend implementation of this endpoint is just parsing the connector definition file to extract the stream info, it should be very fast. This is important because this request will likely need to be submitted on every change to the yaml contents in the webapp (potentially after some short delay), so that the stream list and URLs are always accurate.

post:
summary: List all streams present in the connector definition, along with their specific request URLs
requestBody:
content:
application/json:
schema:
$ref: "#/components/schemas/StreamsListRequestBody"
required: true
responses:
"200":
description: Successful operation
content:
application/json:
schema:
$ref: "#/components/schemas/StreamsListRead"
"400":
$ref: "#/components/responses/ExceptionResponse"
"422":
$ref: "#/components/responses/InvalidInputResponse"

components:
schemas:
StreamRead:
type: object
required:
- slices
properties:
slices:
type: array
description: The stream slices returned from the read command
items:
type: object
required:
- sliceId
- pages
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should have another field describing the slice (it's defined as a json object)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@girarda By "describing the slice", are you talking about the state object that is emitted at the end of the slice? If so, that will be contained in the last page's airbyteMessages field.

Or is there some other information about the slice that should be returned here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean the exact slice object
eg

{
  start_time: "2021-01-01",
  end_time: "2021-01-31"
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I understand now. Yeah, I think that would be useful information to show to the user. I will add a sliceDescriptor here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@girarda added: c63fd4c

properties:
sliceId:
type: string
description: String identifying the current slice
pages:
type: array
description: The pages returned from the read command
items:
type: object
required:
- pageId
- results
- logs
- request
Copy link
Contributor

Choose a reason for hiding this comment

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

@girarda is there already a plan for how these objects specifically will be returned from the CDK?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure that'll require changes on multiple layers of the CDK because Sources are not aware of HTTP requests and responses.

I have a rough proposal here: #17839.

Will bring it up for grooming

- response
properties:
pageId:
type: string
description: String identifying the current page
results:
type: array
description: The RECORD and STATE AirbyteMessages coming from the read operation for this page
items:
type: string
logs:
type: array
description: The LOG AirbyteMessages coming from the read operation for this page
items:
type: string
request:
sherifnada marked this conversation as resolved.
Show resolved Hide resolved
type: object
description: The HTTP request sent to the source API for this page
required:
- url
- headers
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not 100% sure if headers should be required or not, but it should be consistent with the body and request parameters fields

properties:
lmossman marked this conversation as resolved.
Show resolved Hide resolved
url:
type: string
description: URL that the request was sent to
body:
type: object
description: The body of the HTTP request, if present
headers:
type: object
description: The headers of the HTTP request
response:
type: object
description: The response from the source API for this page
required:
- status
- headers
properties:
status:
type: integer
description: The status of the response
body:
type: object
description: The body of the HTTP response, if present
headers:
type: object
description: The headers of the HTTP response
StreamReadRequestBody:
type: object
required:
- definition
- stream
properties:
definition:
$ref: "#/components/schemas/ConnectorDefinitionBody"
description: The config-based connector definition contents
stream:
type: string
description: Name of the stream to read
numSlices:
Copy link
Contributor

Choose a reason for hiding this comment

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

open question: do we want to request a number of slices or specify which slice to request?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What would it look like to specify a specific slice to request here? Passing in a state blob, or something else?

Also, based on our discussion this morning around the Next page / Next slice behavior this morning, I think we'll want to update this part of the API to accommodate that behavior

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure we get much benefit from allowing developers to specify arbitrary middle slices as input via a literal number(or middle pages for that matter). But maybe I'm not thinking about every scenario.

I like the idea of a state object to be passed in during testing seems like it would address both these case to test incremental, but also as a means to test a slice in the middle if necessary

type: integer
description: Number of stream slices to read from the source
default: 1
numPages:
Copy link
Contributor

Choose a reason for hiding this comment

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

this currently does not exist as a first-class-concept in the CDK i.e: we'd need to make some non-trivial changes tot he backend to do this. Any reason we can't just use limit to control the number of records returned?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea was that users may want to test out pagination by first fetching the first page of data, then clicking a "Next page" button to load the next page. Since we don't have any good way of fetching a specific page, we thought that it would be easier to implement changes to the CDK to limit the number of pages.

So when the user clicks "Next page", we would fetch 2 pages of data and display the second. Then if they click "Next page" again, we would fetch 3 pages of data and display the third, and so on.

The other motivation for this is that we may not want to fetch all pages of data every time the user clicks "Test", because some APIs may have hundreds or thousands of pages, which could make requesting all of those pages take many seconds or minutes, which wouldn't be a great user experience. So we wanted to limit the number of pages actually being fetched from the source API to mitigate this and make the "Test" experience snappy.

Though, I think it would be fine for the first MVP two not implement this page-limiting behavior and just fetch all pages instead, and maybe this is something we can add later if there is a need for it. So I can comment out this parameter for now

type: integer
description: Number of pages to read from the source
Copy link
Contributor

Choose a reason for hiding this comment

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

Number of pages to read per-slice from the source

We should probably be very explicit in the description about what this means. Presumably we mean number of pages per slice we will retrieve. i.e. slices = 3, pages = 5 means that for each slice we'll get 5 pages worth of records.

As it reads now, it might be conflated to mean 3 total pages, but pages are kind of a subprocess of slicing

default: 1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added these request params for the eventual goal of allowing users to specify the number of slices/pages to request when testing

state:
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be a proper STATE message?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good callout! Yes it should be, will update

type: object
description: State blob to use for incremental streams
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this state request param as well for testing incremental streams. I'll bring this up in our Low Code Builder Sync, but we will probably eventually want to have a text input where users can enter a JSON state blob, which can be passed in to this parameter.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a good addition, but also noting we'll need to make sure this pane to specify state is only visible/editable when a stream supports incremental and not just full refresh

ConnectorDefinitionBody:
$ref: ../../../../airbyte-cdk/python/airbyte_cdk/sources/declarative/config_component_schema.json
lmossman marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

we'll need to make sure to wire up the builds to regen if this referenced file changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sherifnada Do you have an example or know how that would be done? If not I can make a ticket

StreamsListRequestBody:
type: object
required:
- definition
properties:
definition:
$ref: "#/components/schemas/ConnectorDefinitionBody"
description: The config-based connector definition contents
StreamsListRead:
Copy link
Contributor

Choose a reason for hiding this comment

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

is this how the server will share the schema (whenever we support schema detection)?

Copy link
Contributor Author

@lmossman lmossman Oct 5, 2022

Choose a reason for hiding this comment

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

I don't think so, because detecting the schema will require making an API call to the source.

This /streams/list endpoint is meant to be very fast, as it will likely be called on every change to the yaml editor contents in order to keep the stream list up to date. I think for schema detection, we will likely just want to do that as part of the /stream/read API call, e.g. add another property to the return value of that function that contains the schema of the results.

Copy link
Contributor

Choose a reason for hiding this comment

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

would it be valuable to also return the stream's slices so they can be displayed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean exactly by "return the stream's slices"? Are you suggesting that this should return all of the possible slices that are configured for a given stream, e.g. if they are using a list slicer? And this would be displayed somewhere in the testing panel?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussed this over zoom - the idea here is to have this return a list of all the slices that will be read for this stream, so that they can be displayed somewhere to provide visual feedback on how far through the stream slicing the user has made it. Added a commented-out block here as this can be done in a future phase (will add a note to the tech spec as well)

type: object
required:
- streams
properties:
streams:
type: array
items:
type: object
description: The stream names present in the connector definition
required:
- name
- url
properties:
name:
type: string
description: The name of the stream
url:
type: string
format: uri
description: The URL to which read requests will be made for this stream

# The following exception structs were copied from airbyte-api/src/main/openapi/config.yaml
lmossman marked this conversation as resolved.
Show resolved Hide resolved
InvalidInputProperty:
type: object
required:
- propertyPath
properties:
propertyPath:
type: string
invalidValue:
type: string
message:
type: string
KnownExceptionInfo:
type: object
required:
- message
properties:
message:
type: string
exceptionClassName:
type: string
exceptionStack:
type: array
items:
type: string
InvalidInputExceptionInfo:
type: object
required:
- message
- validationErrors
properties:
message:
type: string
exceptionClassName:
type: string
exceptionStack:
type: array
items:
type: string
validationErrors:
type: array
items:
$ref: "#/components/schemas/InvalidInputProperty"

responses:
InvalidInputResponse:
description: Input failed validation
content:
application/json:
schema:
$ref: "#/components/schemas/InvalidInputExceptionInfo"
ExceptionResponse:
description: Exception occurred; see message for details.
content:
application/json:
schema:
$ref: "#/components/schemas/KnownExceptionInfo"