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

feat: generate cloudevent types from googleapis/google-cloudevents #371

Merged
merged 1 commit into from Oct 28, 2021

Conversation

matthewrobertson
Copy link
Member

@matthewrobertson matthewrobertson commented Oct 25, 2021

This commit adds a new npm run generate_cloudevents script that generates cloudevent interfaces from the schemas in googleapis/google-cloudevents

A draft PR of the generated output is here. Once we are happy with this code gen pipeline, we can run the generator and check the types into this repo.

This is a temporary stopgap until googleapis/google-cloudevents-nodejs is GA. Once that package is ready, we can delete all pipeline code in expermintal/ and replace the generated types in /src/cloudevent_types with types imported from the @google/events npm package.

Copy link
Contributor

@grant grant left a comment

Choose a reason for hiding this comment

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

Thanks for prototyping this! We should probably discuss a bit more first, because there are some implications of going this route for all FFs.

  • Are we going to hardcode the JSON schema URL for generation?
  • JS types are likely going to be easier to model than other languages due to tooling with TS and JSON. Do we expect Go, Java, python, etc to adopt this model?
  • Whats the relation like with updating the schema source with this schema generator? The JSON schema generator and catalog generator are separate to this generator.
  • Why not just use the @google/events library as it is?

scripts/generate_cloudevents/generate.ts Outdated Show resolved Hide resolved
@matthewrobertson
Copy link
Member Author

Why not just use the @google/events library as it is?

There are a few things missing from the googleapis/google-cloudevents-nodejs that would prevent us from taking advantage of discriminated unions to provide a good devx for cloudevent functions:

  • there is no interface generated that connects the cloudevent type to associated data schema. I am generating interfaces to describe that mapping in this PR
  • there is no union of all known cloudevent types. I am generating that type in this PR

I had originally thought we could make the necessary changes in the googleapis/google-cloudevents-nodejs library directly, but the team that owns the library was not keen to accept the changes at this time and they advised against us taking a dependency on that library as it is not currently "production ready". I am proposing this PR as a temporary work around while we get the long term plan for googleapis/google-cloudevents-nodejs sorted out.

JS types are likely going to be easier to model than other languages due to tooling with TS and JSON. Do we expect Go, Java, python, etc to adopt this model?

I don't necessarily think we need to do this for all FFs. But as mentioned above, there are some issues with the current googleapis/google-cloudevents-nodejs libraries that are preventing us from taking advantage of discriminated unions.

Are we going to hardcode the JSON schema URL for generation?

We could pass it in as a parameter to the script, but I am not sure it is worth the extra complexity that would be added to run the script. Are you concerned the location of that file might change?

Whats the relation like with updating the schema source with this schema generator? The JSON schema generator and catalog generator are separate to this generator.

For now I thought we would just manually run this generator periodically and check in the updated types. Eventually I hope we can just take a dependency on googleapis/google-cloudevents-nodejs and this code can all be removed.

Copy link
Contributor

@grant grant left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Per comments, let's:

  • Have a separate folder / package.json with @babel/generator, @babel/types, ts-node etc.
  • Clear PR description / note for how this PR / events may be reverted or changed in the future.

@matthewrobertson matthewrobertson force-pushed the generated-cloud-events branch 2 times, most recently from 32cfa64 to 4c872bd Compare October 28, 2021 01:30
This commit adds a new `npm run generate_cloudevents` script that
generates cloudevent interfaces from the schemas in
googleapis/google-cloudevents
Copy link
Contributor

@grant grant left a comment

Choose a reason for hiding this comment

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

Thanks for the changes. LGTM.

@matthewrobertson matthewrobertson merged commit 7617801 into master Oct 28, 2021
@matthewrobertson matthewrobertson deleted the generated-cloud-events branch December 7, 2021 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants