-
Notifications
You must be signed in to change notification settings - Fork 527
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
Schema gen #10336
Merged
Merged
Schema gen #10336
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The following is the initial commit of the describe API. Currently this uses the api server directly, future PRs will remove this dependency and instead talk directly to the controller.
The following generates the schema on build, this ensures that the schema is always is up to date for all potential clients
The following removes the describe-api command. Instead we ensure that there is a json file in apiserver/facades/schema.json that represents the current schema of the api server facades. This file can then be used for pylibjuju to render the client at any point. Included in is the ability to verify the schema is up to date and if they're not, to error out and show the developer what to do.
The following changes use the git to check the source, that way we can know what they're pushing is in fact what they want to check. We also now check against a tmp file so not to spoil the repo.
Closed
manadart
approved these changes
Jun 17, 2019
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.
Nicely done; QA all good.
|
SimonRichardson
added a commit
to SimonRichardson/python-libjuju
that referenced
this pull request
Jun 17, 2019
Schemagen is no longer required because Juju itself has incorporated the schema generation into the workflow of Juju. This should make it a lot easier to build upon pylibjuju if it's always up to date with Juju. Work to do this has already been completed, see: 1. juju/juju#10336 2. juju/juju#10338 This should support 2.6 and onwards. Older releases can be done the old way, but it requires you to have schemagen installed in the old positions. The error messages might be more cryptic because of that.
Awesome! Love a 38kloc PR. :D |
I tried to look at the output json and github told me to sod off. |
jujubot
added a commit
to juju/python-libjuju
that referenced
this pull request
Jun 18, 2019
#315 Schemagen is no longer required because Juju itself has incorporated the schema generation into the workflow of Juju. This should make it a lot easier to build upon pylibjuju if it's always up to date with Juju. Work to do this has already been completed, see: 1. juju/juju#10336 2. juju/juju#10338 This should support 2.6 and onwards. Older releases can be done the old way, but it requires you to have schemagen installed in the old positions. The error messages might be more cryptic because of that.
jujubot
added a commit
that referenced
this pull request
Jun 19, 2019
#10352 ## Description of change Merge 2.6 forward to bring in: - #10340 from hpidcock/1831682-k8s-bootstrap-errors - #10347 from hpidcock/1832777-enable-ha-panic-fix - #10332 from hpidcock/1832608-statefulset-servicename - #10330 from ycliuhw/fix/podspec-cmd-args-parsing - #10335 from babbageclunk/remove-saas-bug - #10343 from hmlanigan/schema - #10337 from SimonRichardson/missing-exit - #10336 from SimonRichardson/schema-gen - #10327 from manadart/2.6-cache-entity-copies - #10328 from ycliuhw/fix/warn-message - #10320 from hpidcock/1832608-statefulset-servicename
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of change
In order to provide a clean workflow for python-libjuju (pylibjuju), we
need to find a way to create a schema (JSON) for pylibjuju so that
when generating the new set of client definitions in pylibjuju we're not
required to jump through hoops at updating and change schemagen.
Schemagen tags juju at a given hash, which means recording all the
correct dependencies from juju along with pylibjuju as well. This isn't
trivial as we also need to work with different branches and can become
confusing when explain the setup.
Instead the work flow will be, juju will always contain a schema.json
and then when making pylibjuju we can just consume it.
QA steps
make rebuild-schema
Then run to verify:
./scripts/verify.sh
Note: this supersedes #10305