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: Response sampling #21

Merged
merged 4 commits into from
Jul 26, 2022
Merged

Feature: Response sampling #21

merged 4 commits into from
Jul 26, 2022

Conversation

oscar60310
Copy link
Contributor

@oscar60310 oscar60310 commented Jul 15, 2022

https://www.notion.so/canner/API-response-sampling-88093cca3a8b41aab90d3a0b1cbafde9

User story

  1. User definitions.

    -- YAML
    exampleParameter:
        userId: 1111
    -- SQL
    select * from users where id = '{{ context.params.userId }}';
  2. We’ll send queries while building.

    select * from users where id = '1111';
  3. After sample query responses, we transform and append the response.

    response:
      - name: id
        type: STRING
    ....
  4. (Existed feature) Generate document from schema.

Commits

  1. 3c18dae Pass pagination config into compiler.
  2. 35905a3 Refactor: In order to access template engine from middleware, I update middleware from pure functions to injectable classes.
  3. 2058470 Load compiled data right after compiling because we'll send sample queries while building schema.
  4. d30e700 Sampler implementation. I send the main query with offset/limit pagination:
        const response = await this.templateEngine.execute(
          schema.templateSource,
          { context: { params: schema.exampleParameter } },
          // We only need the columns of this query, so we set offset/limit both to 0 here.
          {
            limit: 0,
            offset: 0,
          }
        );
    Maybe there is a better solution to get the columns.

@oscar60310 oscar60310 changed the title [WIP]: Feature: Response sampling [WIP] Feature: Response sampling Jul 15, 2022
@oscar60310 oscar60310 changed the base branch from develop to feature/dbt July 15, 2022 09:54
@oscar60310 oscar60310 force-pushed the feature/resp-sampling branch from 02ad884 to d30e700 Compare July 18, 2022 09:04
@oscar60310 oscar60310 requested a review from kokokuo July 18, 2022 09:34
@oscar60310 oscar60310 changed the title [WIP] Feature: Response sampling Feature: Response sampling Jul 18, 2022
@oscar60310 oscar60310 marked this pull request as ready for review July 18, 2022 09:35
Copy link
Contributor

@kokokuo kokokuo left a comment

Choose a reason for hiding this comment

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

Excellent, LGTM 👍

this.use(addRequiredValidatorForPath());
this.use(setConstraints(validatorLoader));
// Load middleware
middlewares.forEach(this.use.bind(this));
Copy link
Contributor

Choose a reason for hiding this comment

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

Great and clear way through IoC solution to inject 👍 (also make me could refer in my code refactor 😃 )

export * from './generatePathParameters';
export * from './addRequiredValidatorForPath';
export * from './setConstraints';
export const SchemaParserMiddlewares: ClassType<SchemaParserMiddleware>[] = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could add a comment to let other members know the array order will become the compose order for middleware executing, or if other members modified the order and some middleware have the relationship of reading/writing data, it will have some influence.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I've added.

// The order of middleware here indicates the order of their execution, the first one will be executed first, and so on.
export const SchemaParserMiddlewares: ClassType<SchemaParserMiddleware>[] = [
  GenerateUrl,
  CheckValidator,
  TransformValidator,
  GenerateTemplateSource,
  CheckParameter,
  AddMissingErrors,
  FallbackErrors,
  NormalizeFieldIn,
  GenerateDataType,
  NormalizeDataType,
  GeneratePathParameters,
  AddRequiredValidatorForPath,
  SetConstraints,
  ResponseSampler,
];

@oscar60310 oscar60310 force-pushed the feature/dbt branch 2 times, most recently from 8c21911 to 206c8f1 Compare July 26, 2022 02:11
@oscar60310 oscar60310 force-pushed the feature/resp-sampling branch from d30e700 to caeb4eb Compare July 26, 2022 02:20
@oscar60310
Copy link
Contributor Author

Hi @kokokuo , thanks for reviewing, all issues have been fixed.

Base automatically changed from feature/dbt to develop July 26, 2022 02:24
@kokokuo kokokuo merged commit 22b5a5c into develop Jul 26, 2022
@kokokuo kokokuo deleted the feature/resp-sampling branch July 26, 2022 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants