-
Notifications
You must be signed in to change notification settings - Fork 165
feat: generate cloudevent types from googleapis/google-cloudevents #371
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
There was a problem hiding this 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?
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:
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.
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.
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?
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. |
71d07f4
to
e1aa0df
Compare
There was a problem hiding this 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.
32cfa64
to
4c872bd
Compare
This commit adds a new `npm run generate_cloudevents` script that generates cloudevent interfaces from the schemas in googleapis/google-cloudevents
4c872bd
to
c6c365d
Compare
There was a problem hiding this 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.
This commit adds a new
npm run generate_cloudevents
script that generates cloudevent interfaces from the schemas in googleapis/google-cloudeventsA 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.