Skip to content

Commit

Permalink
a few schema fixes and adding some custom pagination and stream slicer
Browse files Browse the repository at this point in the history
  • Loading branch information
brianjlai committed Dec 9, 2022
1 parent ac099f8 commit 37dedfe
Showing 1 changed file with 41 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,19 @@ description: Low Code connector components
version: 1.0.0
required:
- check
- spec
- streams
- version
properties:
check:
"$ref": "#/definitions/CheckStream"
spec:
"$ref": "#/definitions/Spec"
streams:
type: array
items:
"$ref": "#/definitions/DeclarativeStream"
version:
type: string
spec:
"$ref": "#/definitions/Spec"
definitions:
AddedFieldDefinition:
description: Defines the field to add on a record
Expand Down Expand Up @@ -66,7 +65,7 @@ definitions:
type: string
header:
type: string
BackoffStrategy:
BackoffStrategy: # maybe delete this since it was intended to be just an interface

This comment has been minimized.

Copy link
@maxi297

maxi297 Dec 9, 2022

Contributor

This is an interesting point. I'm wondering if it could still be valuable in terms of maintenance. I don't see a case in our schema right now but let's add a NewShinyObject in our schema like this:

  NewShinyObject:
<...>
    properties:
<...>
      backoff_strategies:
        type: array
        items:
          anyOf:
            - "$ref": "#/definitions/ConstantBackoffStrategy"
            - "$ref": "#/definitions/ExponentialBackoffStrategy"
            - "$ref": "#/definitions/WaitTimeFromHeaderBackoffStrategy"
            - "$ref": "#/definitions/WaitUntilTimeFromHeaderBackoffStrategy"

What I fear might happen is that now, If I add a new JitterBackoffStrategy, I need to check all the objects that would have a BackoffStrategy and understand if it make sense to add the JitterBackoffStrategy there or not. In that case, this would include DefaultErrorHandler and NewShinyObject. If we have the interface, we can simply add the JitterBackoffStrategy to the BackoffStrategy definition and that's it. Not sure this is worth at the moment but maybe just food for thoughts for later

This comment has been minimized.

Copy link
@brianjlai

brianjlai Dec 9, 2022

Author Contributor

Ah I see that is a good point. Thinking about it a bit, I think it does make sense if we have a lot of repeated unions and it is a hassle. Right now in our schema I haven't seen many cases for objects. Although we do have a couple repeats of things like primary_key. My gut reaction is this should hopefully not be a big deal because if we're in the pattern of lots of duplicative fields across different components, we might want to address the root cause of why they need to be defined so frequently in the first place.

Do you know how this affects the models that get generated if we were to use this intermediary "interface" BackoffStrategy object in place of defining the whole list of strategies in the paginator anyOf?

type: object
anyOf:
- "$ref": "#/definitions/ConstantBackoffStrategy"
Expand Down Expand Up @@ -111,6 +110,7 @@ definitions:
type: array
items:
anyOf:
- "$ref": "#/definitions/CustomStreamSlicer"
- "$ref": "#/definitions/DatetimeStreamSlicer"
- "$ref": "#/definitions/ListStreamSlicer"
- "$ref": "#/definitions/SingleSlice"
Expand Down Expand Up @@ -164,15 +164,15 @@ definitions:
$options:
type: object
additionalProperties: true
CursorPaginationStrategy:
CursorPagination:
description: Pagination strategy that evaluates an interpolated string to define the next page token
type: object
required:
- type
- cursor_value
properties:
type:
enum: [CursorPaginationStrategy]
enum: [CursorPagination]
cursor_value:
type: string
page_size:
Expand Down Expand Up @@ -214,6 +214,34 @@ definitions:
$options:
type: object
additionalProperties: true
CustomPaginationStrategy:
type: object
additionalProperties: true
required:
- type
- class_name
properties:
type:
enum: [CustomPaginationStrategy]
class_name:
type: string
$options:
type: object
additionalProperties: true
CustomStreamSlicer:
type: object
additionalProperties: true
required:
- type
- class_name
properties:
type:
enum: [CustomStreamSlicer]
class_name:
type: string
$options:
type: object
additionalProperties: true
DatetimeStreamSlicer:
description: Stream slicer that slices the stream over a datetime range
type: object
Expand Down Expand Up @@ -372,7 +400,8 @@ definitions:
enum: [DefaultPaginator]
pagination_strategy:
anyOf:
- "$ref": "#/definitions/CursorPaginationStrategy"
- "$ref": "#/definitions/CursorPagination"
- "$ref": "#/definitions/CustomPaginationStrategy"
- "$ref": "#/definitions/OffsetIncrement"
- "$ref": "#/definitions/PageIncrement"
url_base:
Expand Down Expand Up @@ -717,24 +746,23 @@ definitions:
items:
type: string
RequestOption:
description:
description: Describes an option to set on a request
type: object
required:
- type
- field_name
- inject_into
properties:
type:
enum: [RequestOptions]
field_name:
type: string
enum: [RequestOption]
inject_into:
enum:
- request_parameter
- header
- path
- body_data
- body_json
field_name:
type: string
SimpleRetriever:
description: Retrieves records by synchronously sending requests to fetch records. The retriever acts as an orchestrator between the requester, the record selector, the paginator, and the stream slicer.
type: object
Expand Down Expand Up @@ -771,6 +799,7 @@ definitions:
stream_slicer:
anyOf:
- "$ref": "#/definitions/CartesianProductStreamSlicer"
- "$ref": "#/definitions/CustomStreamSlicer"
- "$ref": "#/definitions/DatetimeStreamSlicer"
- "$ref": "#/definitions/ListStreamSlicer"
- "$ref": "#/definitions/SingleSlice"
Expand Down

0 comments on commit 37dedfe

Please sign in to comment.