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

chore: add juju 3.1.{3,4,5,6,7,8,9,10} schemas, no code changes #1166

Conversation

james-garner-canonical
Copy link
Contributor

Description

After every Juju 3.X.Y release, python-libjuju should include Juju's apiserver/facades/schema.json file under juju/client/schemas-juju-3.X.Y.json, regenerate the client code, and verify whether any new facades are supported. The latest schemas currently in python-libjuju are 3.1.2 for the 3.1 series, and 3.3.0 for the 3.3 series, so clearly this hasn't been done in a while.

This PR adds (client-only) schemas for 3.1.3 - 3.1.10. There are no facade version changes, so despite regenerating the python-libjuju client code, this PR makes no changes other than adding the schema files themselves.

While it seems kind of wasteful to add these large schema files when they're not making any facade version changes, I think it makes sense to do so as a clear signal that python-libjuju is up to date when it comes to supporting the 3.1 series.

QA Steps

After adding each schema we run:
rm juju/client/_client*.py juju/client/_definitions.py followed by:
make client
and verify that there are no changes to the generated code.

After adding each schema we run:
rm juju/client/_client*.py juju/client/_definitions.py
followed by:
make client
and verify that there are no changes to the generated code.
@dimaqq
Copy link
Contributor

dimaqq commented Oct 17, 2024

If there are no code changes, WDYT about keeping only the oldest and the newest 3.1.x ?

@james-garner-canonical
Copy link
Contributor Author

It seems wasteful and bad for maintenance to check in these large schemas that future maintainers may feel like they need to look through. It would be nice if we could avoid this.

Maybe we want a policy like this:

  • first schema in a minor version series (e.g. 3.1.0)
    • add the schema
  • subsequent schemas:
    • is there any difference from the most recent schema?
      • yes? add the new schema, keeping the most recent as well
      • no? replace the most recent schema with the new schema

Assuming that this practice was followed, then you'd know that the latest schema present indicates the most recent release that's been checked, and that all earlier versions had been checked already. But you'd have to trust that the versions in between the schemas present had been handled correctly.


In the 3.1.X series, there are only differences in the schemas for two version bumps:

  • 3.1.0 -> 3.1.1
  • 3.1.5 -> 3.1.6

The diffs are quite small, so I've attached them at the end of this message. The 3.1.4 -> 3.1.6 change is just removing some deprecated fields (or something?) which for some reason doesn't result in any change to the generated code. This raises the question of what we consider a difference from the previous schemas. We could treat codegen as the source of truth here, but to shield against bugs in the codegen maybe it would be better to count any difference in the schemas.

Following this policy, for the 3.1.X series we'd have:

  • 3.1.0
  • 3.1.1 (added because different from 3.1.0)
  • 3.1.5 (replaced 3.1.4, which replaced 3.1.3, which replaced 3.1.2, which replaced 3.1.1)
  • 3.1.6 (added because different from 3.1.5)
  • 3.1.10 (replaced 3.1.9, which ...)

Future maintainers would know that there are three different versions of the 3.1.X schema and be able to diff the files to see the difference. They'd also know that 3.1.10 was the latest Juju release from the 3.1.X series that had been brought into python-libjuju.


Benefits:

  • less lines of stuff checked into this repo, so less for people to look at and wonder what's going on.

Concerns:

  • can't verify that all the Juju releases have been covered properly just by looking at the schemas present -- would codegen be any different if you added the missing schemas? It shouldn't be, but you either have to trust the previous maintainers, try it, or perhaps dig into git history to see what schemas have been present and if there were any differences that were lost
  • more complicated / error prone release procedure, making it harder to trust that it was followed correctly

But maybe we could add some CI checks to catch easy to make errors?
Stuff like:

  • catch accidental replace instead of add: if schema-A.B.C is removed, then there should be a schema-A.B.(C+1) with no differences
  • catch accidental add instead of replace: if schema-A.B.C is added, then either C == 0 or there is some difference with schema-A.B.(C-1)

Alternatively, or perhaps as well, I think it would be good to put these schemas somewhere. Or provide a script for generating them on the fly from Juju repo maybe? Because of the change to client-only schemas, you can't just grab the schema.json from the relevant Juju release.

My process has been to check out the Juju release (either juju-$VERSION branch or v$VERSION tag), make a new branch $VERSION-client-only-schemas, add a target to the Makefile to generate the client-only schemas (and commit), and then generate the client-only schema as client-schema.json and commit. Oh and if generation fails, install and switch to the appropriate go version and try again.


Diffs

3.1.0 -> 3.1.1

1238,1245d1237
<                 "Destroy": {
<                     "type": "object",
<                     "properties": {
<                         "Params": {
<                             "$ref": "#/definitions/"
<                         }
<                     }
<                 },
1291,1298d1282
<                 "DestroyUnits": {
<                     "type": "object",
<                     "properties": {
<                         "Params": {
<                             "$ref": "#/definitions/"
<                         }
<                     }
<                 },
1505,1508d1488
<                 "": {
<                     "type": "object",
<                     "additionalProperties": false
<                 },
10123c10103
<         "Description": "KeyManagerAPI implements the KeyUpdater interface and is the concrete\nimplementation of the api end point.",
---
>         "Description": "KeyManagerAPI provides api endpoints for manipulating ssh keys",

3.1.5 -> 3.1.6

7784,7786d7783
<                         "deprecated": {
<                             "type": "boolean"
<                         },
10827,10829d10823
<                         },
<                         "deprecated": {
<                             "type": "boolean"

@dimaqq
Copy link
Contributor

dimaqq commented Oct 17, 2024

I would rather add a markdown file in the directory noting that 3.1.{1,2,3,4,5} and 3.1.{6,7,8,9,10} are the same than shipping a bunch of identical files.

Same markdown file could clearly state what Juju versions are present in this codebase.

WDYT?

@james-garner-canonical
Copy link
Contributor Author

Would you be happy with shipping the following schemas for 3.1 series:

  • 3.1.0
  • 3.1.5 (replaces 3.1.4, which replaces 3.1.3, which replaces 3.1.2, which replaces 3.1.1 -- 3.1.1 is different to 3.1.0)
  • 3.1.10 (replaces 3.1.9, which replaced 3.1.8, which replaced 3.1.7, which replaced 3.1.6 -- 3.1.6 is different to 3.1.5)
  • schemas.md containing, e.g.
# 3.1
3.1.0
3.1.1
3.1.2 (identical to 3.1.1)
3.1.3 (identical to 3.1.1)
3.1.4 (identical to 3.1.1)
3.1.5 (identical to 3.1.1)
3.1.6
3.1.7 (identical to 3.1.6)
3.1.8 (identical to 3.1.6)
3.1.9 (identical to 3.1.6)
3.1.10 (identical to 3.1.6)

If so, are you happy with this policy?

  • first schema in a minor version series (e.g. 3.1.0)
    • add the schema
    • add section to schemas.md (# 3.1) and a line (3.1.0)
  • subsequent schemas:
    • is there any difference from the most recent schema in the series?
      • yes?
        • add the new schema, keeping the most recent as well
        • add a new line to schemas.md for the new schema
      • no? replace the most recent schema with the new schema
        • add a new line to schemas.md for the new schema noting that it's identical to previous

If so, that's what I'll go with for now and that's what I'll add to the release documentation. I'll just open issues about potentially catching errors with adding schemas in CI, and not worry further about that for now.

@dimaqq
Copy link
Contributor

dimaqq commented Oct 17, 2024

Yes, I'm happy with that.

@james-garner-canonical
Copy link
Contributor Author

Closing in favour of:

jujubot added a commit that referenced this pull request Oct 18, 2024
…ema-release-process

#1169

#### Description

We want to ensure that `python-libjuju` supports the latest Juju schemas, and that it's clear to `python-libjuju` maintainers and users what versions are supported. This PR adds documentation on how to do this (`docs/CONTRIBUTING.md`), and follows this process for the existing schemas, updating the `3.1` series (`juju/client/SCHEMAS.md` and `juju/client/schemas-juju-*.json`)

#### QA Steps

No changes to generated code are introduced in this PR. `make client` should not produce any changes to the repo state.

Test should all pass.

#### Notes & Discussion

Replaces:
- #1166

Implicitly depends on:
- #1168
Since we add a schema for `3.1.10` here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants