Resource schema #139
Merged
Resource schema #139
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
MbolotSuse
force-pushed
the
resource-schema
branch
2 times, most recently
from
3cf0900 to
e683831
Compare
KevinJoiner
suggested changes
Jan 8, 2024
MbolotSuse
force-pushed
the
resource-schema
branch
from
e683831 to
97bb45a
Compare
MbolotSuse
force-pushed
the
resource-schema
branch
2 times, most recently
from
108d104 to
d121dfe
Compare
moio
reviewed
Jan 15, 2024
There was a problem hiding this comment.
Hey @MbolotSuse, tons of great work here - I feel confident this will help significantly in the performance of cases we've observed.
I put a couple hours understanding context and code - I admit this is not a very deep review, as I lacked and still lack a lot of insight. Hopefully comments will still help in some way.
Looking forward to see this tested in the UI!
MbolotSuse
force-pushed
the
resource-schema
branch
2 times, most recently
from
697db4f to
0e49474
Compare
moio
approved these changes
Jan 22, 2024
KevinJoiner
approved these changes
Jan 22, 2024
tomleb
reviewed
Jan 23, 2024
|
|
||
| addBaseSchema("management.cattle.io.globalrole", "management.cattle.io.missingfrommodel") | ||
|
|
||
| tests := []struct { |
There was a problem hiding this comment.
I think we could have a couple more tests wrt. resource version:
- resource.Version == ""
- resource.Version == group.Version
- resource.Version != group.Version
to make sure we properly handle these cases.
There was a problem hiding this comment.
I can look into adding this as a part of MbolotSuse#1
Changes steve to only return top level schema definitions rather than a full defined schema for every field. Also adds basic docs and fixes a bug with schema generation on versioned crds
Introduces new schema definitions endpoint, allowing the caller to get the fields/types/descriptions for a given kubernetes resource.
MbolotSuse
force-pushed
the
resource-schema
branch
from
0e49474 to
099ebc5
Compare
This was referenced Feb 28, 2024
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.
Overview
Related to rancher/rancher#41301. Also see the previous pr #62 for some earlier discussion and context.
Currently, the steve schemas endpoint produces an extremely large amount of data. Since the UI needs this endpoint for a variety of reasons and frequently fetches the entirety of it, this produces a large performance impact where pages will need to wait a significant amount of time, purely for the schemas to load.
This PR reduces the amount of schemas, and causes (most of) the schemas to have no
resourceField. In addition, it introduces a newschemaDefinitionsendpoint which provides all fields/types/descriptions for a single schema.Solution Detail
Podwould be included, but there is no longer a schema for thePodSpec.resourceFieldsfrom the schemas.schemaDefinitionsendpoint which allows users to specify a single schema (i.e.Pod) and provides the full resource fields for that schema./v1/schemaDefinitionswill return an error). This was done to encourage use of only a specific resource definition at a time - the full openapi schema for k8s is still quite large, and this could cause large performance drops if a list was supported.PodincludesPodSpec, this endpoint will return the top-level fields for bothPodandPodSpec.schemasendpoint. In short, if you can see theschemafor an object, you can get it'sschemaDefinitionas well.openapiv2part of the schema retrieval to only add description (previously this was used for resource fields).schemaFieldVisitorwhich translates from an openapi schema to a schema definition.Notes