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

Automate external API versioning #8908

Merged
merged 58 commits into from
Nov 8, 2024
Merged

Conversation

kurtisassad
Copy link
Contributor

@kurtisassad kurtisassad commented Aug 20, 2024

Link to Issue

Closes: #8890
Closes: #9500

Description of Changes

  • Adds script which bumps the version in external-api-config.json (used as OpenAPI spec version)
    • Compares production OAS with locally generated OAS
  • On startup of DYNO=web.1 in production, dispatch the SDK publishing workflow in the hicommonwealth/api-client repo if the latest version of the @commonxyz/api-client package is not equal to the current OpenAPI spec version.
  • Adds Versioning.yml` action which only triggers on PRs that target the production branch
    • The action bumps the openAPI spec and api-client/package.json versions if necessary
    • Pushes directly to the PR
  • Deleted libs/api-client since all of that configuration is moved to https://github.com/hicommonwealth/api-client

Test Plan

Versioning (Versioning.yml)

Publish workflow
Hope it works it on the next deployment. If it fails due to the commonwealth side function, dispatch the workflow manually from the api-client repository and fix the dispatch function in a PR.
- I tested everything in the dispatchSDKPublishWorkflow function except the API call to dispatch the workflow

Deployment Plan

  • Make the version bumping CI job required before merging into the production branch
  • Set API_CLIENT_REPO_TOKEN -> use the Common Workflow Dispatcher GitHub app token (@timolegros)

@kurtisassad kurtisassad requested review from Rotorsoft, timolegros and rbennettcw and removed request for Rotorsoft August 20, 2024 12:33
@Rotorsoft
Copy link
Contributor

We might be able to get this diff without having to run the server. The spec can be generated by utils in /trpc/middleware via scripts. Maybe we can maintain the current master spec as a code artifact, and generate the diff by generating and comparing to the branch we are trying to merge, then update versions and spec with new spec.

@kurtisassad
Copy link
Contributor Author

kurtisassad commented Aug 20, 2024

I think updating the branch with what we are trying to merge will lead to inflating our version numbering because it will basically bump the version numbering each time we make a new PR that changes the external API. I think it would be better to compare with the server.

I also see no reason not to run the server end to end. Not only is it easier it also guarantees we are comparing the same spec.

Anyways maybe we can discuss during platform hours

@timolegros
Copy link
Collaborator

What's the hold-up on this PR - can we get it out of draft or close it?

# Conflicts:
#	packages/commonwealth/server/api/external-router.ts
#	pnpm-lock.yaml
@kurtisassad kurtisassad marked this pull request as ready for review September 24, 2024 16:04
@timolegros
Copy link
Collaborator

I think updating the branch with what we are trying to merge will lead to inflating our version numbering because it will basically bump the version numbering each time we make a new PR that changes the external API. I think it would be better to compare with the server.

I also see no reason not to run the server end to end. Not only is it easier it also guarantees we are comparing the same spec.

Anyways maybe we can discuss during platform hours

One thing to note is that without the GENERATE_PRODUCTION_SDK=true environment variable, the generated SDK will have an API URL that defaults to local rather than production. So, in order to properly compare specs with production you need to start the localhost server with GENERATE_PRODUCTION_SDK=true.

@timolegros timolegros added the deployment plan (PRs only) requires manual infrastructure changes on release label Oct 18, 2024
@timolegros timolegros marked this pull request as draft November 6, 2024 20:27
@timolegros timolegros marked this pull request as ready for review November 8, 2024 00:50
@timolegros timolegros merged commit d20c361 into master Nov 8, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployment plan (PRs only) requires manual infrastructure changes on release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API Client CI Publishing Pipeline Add CI action to enforce correct external API versioning
4 participants