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

Generate cloud api client from airbyte-cloud if present #19091

Closed
wants to merge 1 commit into from

Conversation

ambirdsall
Copy link
Contributor

What

As of #19026, we've got cloud tooling affordances relying on airbyte and airbyte-cloud being sibling directories in a dev's filesystem. This opens up the possibility of generating API client code for cloud endpoints (defined in airbyte-cloud/cloud-api/src/main/openapi/config.yaml); to date, cloud API client code has been written by hand, leading to the possibility of bugs from server/client code drift.

How

  1. use node's fs.existsSync to check for the airbyte-cloud repo's openapi/config.yaml; if it exists, provide orval config to generate client code.

What might not work?

  1. does an import from a missing AirbyteCloudApiClient.ts break the build even if it's not in the module graph for the OSS react app?

remaining TODOs:

  • test out build failure scenarios for open source users
    • make separate checkout of airbyte away from airbyte-cloud and run npm start to generate api clients and build app
    • try using a cloud endpoint definition in the cloud/ directory somewhere (should be able to transparently replace a wrapped OSS endpoint with the cloud version) to see if explicit imports fail
    • ???

@ambirdsall ambirdsall requested a review from a team as a code owner November 7, 2022 20:34
@github-actions github-actions bot added area/platform issues related to the platform area/frontend Related to the Airbyte webapp labels Nov 7, 2022
@timroes
Copy link
Contributor

timroes commented Nov 7, 2022

Could we clarify what's the idea how this will work for non airbyte employees to still compile the repository as well as how this is supposed to work on CI, which wouldn't have the types and don't have the code checked out?

@ambirdsall
Copy link
Contributor Author

Could we clarify what's the idea how this will work for non airbyte employees to still compile the repository as well as how this is supposed to work on CI, which wouldn't have the types and don't have the code checked out?

@timroes this was exactly why I have those checks in the PR description right now, and why I didn't assign any reviewers yet; I expect that the branch's current state would break the build in those scenarios. I forgot to mark this PR as a draft, which for all practical purposes it is until I've taken the time to circle back, test those scenarios, and see what fixes are needed and which are possible; I'll fix that now and reopen it for review if I can advance this past its current status as a minimal proof-of-concept spike.

@ambirdsall ambirdsall marked this pull request as draft November 7, 2022 23:25
@ambirdsall
Copy link
Contributor Author

While this was a useful spike, it's pretty clearly not the appropriate solution, since writing any code which actually uses the conditionally-generated code will cause tooling headaches for everyone who is missing those files. I suspected that from the start, and probably should have followed the idea to its logical conclusion before opening the PR, so the description would have been a bit more apt. Still, a useful exercise, if only as a reference for how our API code generation system works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend Related to the Airbyte webapp area/platform issues related to the platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants