-
Notifications
You must be signed in to change notification settings - Fork 6
[Issue #412] Adding library for auto-generation and documentation of script #452
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
base: main
Are you sure you want to change the base?
Conversation
| poetry run python ./validate.py <PATH TO YAML FILE > | ||
|
|
||
| ## Issues to be Resolved | ||
| The following issues are from comparing the generated schemas against the existing hand built schemas. Note this is not an exhaustive list |
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.
These are possibly show stopper issues. The PR should not be merged until it can be proven that spec-compliant schemas can effectively be generated from code.
For reference, issues like these were what caused me to abandon the effort to auto-generate marshmallow schemas several months ago. My experience was that tooling and automation could generate 99.x% of the schemas but the last handful of edge cases were intractable, and it turned into a huge time sink to solve the edge cases.
YMMV but let's not commit new scaffolding and dependencies to the repo until it is proven to work.
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.
Where are the generated schemas? Do they work with the example API implementations? (e.g. simpler-grants-protocol/examples/ca-opportunity-example)
The example API implementations are the main use case for CommonGrants pydantic schemas, therefore acceptance criteria for this or any auto-generation scaffolding or libraries should include validation against one or all of those use cases: generate the schemas, import them into example API, run cg check-spec, confirm no errors.
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.
The thinking was to commit this baseline to the repo in an isolated way that wouldn't impact the existing hand built schemas and their implementations. This would give us a foundation to build off of while not having the completion of this feature become an all consuming task at the expense of other planned work.
The generated schemas are not committed to the repo since that would clog up the codebase with 103 additional files for something that isn't complete and could lead to confusion with the existing schemas. The idea is that for now anyone who wanted to view the schema could run the bash script and it would create the schemas in the generated directory that the readme exists in.
Type Checking DiscoverySteps Taken
|
Summary
Changes proposed
This PR adds the library that auto generates the pydantic objects, a script for calling said library, and a readme tracking the work done and future work to take place as part of this effort.
There is also a baseline for property based testing that we can build off of as part of future work for this objective.
Context for reviewers
You may have to run a new poetry install as part of this change
Additional information