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

chore(open-payments): fetch OpenAPI specs locally #846

Merged
merged 11 commits into from
Dec 16, 2022
Merged

Conversation

wilsonianb
Copy link
Contributor

@wilsonianb wilsonianb commented Dec 13, 2022

Changes proposed in this pull request

  • Fetch OpenAPI specs locally in open-payments
  • Symlink necessary OpenAPI specs in auth and backend
  • pnpm fetch-schemas is no longer required, and Open Payments OpenAPI specs now live in a single place in the monorepo (where they can optionally be modified).

Context

Checklist

  • Related issues linked using fixes #number
  • Tests added/updated
  • Documentation added
  • Make sure that all checks pass

@github-actions github-actions bot added pkg: auth Changes in the GNAP auth package. pkg: backend Changes in the backend package. pkg: open-payments type: ci Changes to the CI type: source Changes business logic type: tests Testing related labels Dec 13, 2022
@wilsonianb wilsonianb marked this pull request as ready for review December 13, 2022 16:17
@wilsonianb wilsonianb requested review from mkurapov and njlie December 13, 2022 16:17
mkurapov
mkurapov previously approved these changes Dec 15, 2022
Copy link
Contributor

@mkurapov mkurapov left a comment

Choose a reason for hiding this comment

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

This is great, keeps it in one place

"prepack": "pnpm build",
"test": "jest --passWithNoTests"
"test": "jest --passWithNoTests",
"copy-files": "cp ./src/openapi/*.yaml ./dist/openapi/"
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 move this up after clean?

"fetch-schemas": "./scripts/fetch-schemas.sh",
"generate:auth-server-types": "openapi-typescript src/openapi/auth-server.yaml --output src/openapi/generated/auth-server-types.ts -t",
"generate:resource-server-types": "openapi-typescript src/openapi/resource-server.yaml --output src/openapi/generated/resource-server-types.ts -t",
"generate:types": "pnpm generate:auth-server-types && pnpm generate:resource-server-types",
Copy link
Contributor

@mkurapov mkurapov Dec 15, 2022

Choose a reason for hiding this comment

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

I went down the rabbit hole to try and put up a fix for getting version openapi-typescript v6.1.0 working (https://github.com/interledger/rafiki/issues/729) to properly generate the external type but no luck. Version 5.4.0 was the last one that was working with the proper type generation. We can keep this here until openapi-ts/openapi-typescript#993 is fixed

"clean": "rm -fr dist/",
"generate:types": "npx ts-node scripts/generate-types.ts",
"fetch-schemas": "./scripts/fetch-schemas.sh",
"generate:auth-server-types": "openapi-typescript src/openapi/auth-server.yaml --output src/openapi/generated/auth-server-types.ts -t",
Copy link
Contributor

Choose a reason for hiding this comment

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

why types over interfaces btw?

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 don't know 😅
-t isn't even an option in the version we're using...

  Generate TypeScript types from Swagger OpenAPI specs

  Usage
    $ openapi-typescript [input] [options]

  Options
    --help                       display this
    --output, -o                 Specify output file (default: stdout)
    --auth                       (optional) Provide an authentication token for private URL
    --headersObject, -h          (optional) Provide a JSON object as string of HTTP headers for remote schema request
    --header, -x                 (optional) Provide an array of or singular headers as an alternative to a JSON object. Each header must follow the key: value pattern
    --httpMethod, -m             (optional) Provide the HTTP Verb/Method for fetching a schema from a remote URL
    --immutable-types, -it       (optional) Generates immutable types (readonly properties and readonly array)
    --additional-properties, -ap (optional) Allow arbitrary properties for all schema objects without "additionalProperties: false"
    --default-non-nullable       (optional) If a schema object has a default value set, don’t mark it as nullable
    --prettier-config, -c        (optional) specify path to Prettier config file
    --raw-schema                 (optional) Parse as partial schema (raw components)
    --version                    (optional) Force schema parsing version

@@ -67,9 +67,6 @@ pnpm clean

# install dependencies
pnpm i

# pull in latest openapi specs:
pnpm fetch-schemas
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 the README in the auth package also has a fetch-schemas reference

@wilsonianb wilsonianb requested a review from mkurapov December 15, 2022 22:59
mkurapov
mkurapov previously approved these changes Dec 16, 2022
"localenv": "docker compose -f ./infrastructure/local/docker-compose.yml -f ./infrastructure/local/peer-docker-compose.yml",
"localenv:build": "docker compose -f ./infrastructure/local/docker-compose.yml -f ./infrastructure/local/peer-docker-compose.yml -f ./infrastructure/local/build-override.yml build",
"localenv:seed:auth": "pnpm -C ./packages/auth knex seed:run --env=development && pnpm -C ./packages/auth knex seed:run --env=peerdevelopment",
"localenv:dbvolumes:remove": "docker volume rm local_database-data && docker volume rm local_tigerbeetle-data",
"sanity": "pnpm fetch-schemas pnpm -r build && pnpm -r test"
Copy link
Contributor

Choose a reason for hiding this comment

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

What was the sanity for, originally? Should anything else belong 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.

I don't know why it was added

@wilsonianb wilsonianb merged commit 78c5eac into main Dec 16, 2022
@wilsonianb wilsonianb deleted the bw-openapi branch December 16, 2022 13:31
@huijing huijing added the type: documentation (archived) Improvements or additions to documentation label Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: auth Changes in the GNAP auth package. pkg: backend Changes in the backend package. type: ci Changes to the CI type: documentation (archived) Improvements or additions to documentation type: source Changes business logic type: tests Testing related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants