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

Custom annotations for bundle-specific JSON schema fields #1957

Merged
merged 38 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
9b588c0
feat: Custom annotations for json-schema fields in bundle
ilyakuz-db Dec 4, 2024
12c4373
feat: Introduce separate files for OpenAPI annotations
ilyakuz-db Dec 6, 2024
049e11b
test: Checks missing descriptions in annotation files
ilyakuz-db Dec 6, 2024
8016d41
fix: Linter fix
ilyakuz-db Dec 6, 2024
0221c28
fix: Remove redundant field
ilyakuz-db Dec 6, 2024
a3fdc8d
fix: Skip tests on windows
ilyakuz-db Dec 6, 2024
79a88e9
fix: Run package instead of set of files
ilyakuz-db Dec 9, 2024
c3d049f
fix: Custom typePath function instead of exposing private `jsonschema…
ilyakuz-db Dec 9, 2024
b076813
fix: Change the structure of annotation files, minor fixes and tests
ilyakuz-db Dec 10, 2024
073aeca
feat: Add styles
ilyakuz-db Dec 10, 2024
a6c45d5
feat: Yaml styles for open api overrides
ilyakuz-db Dec 10, 2024
d164968
Merge branch 'main' of github.com:databricks/cli into feat/custom-ann…
ilyakuz-db Dec 10, 2024
00164a8
fix: Linter fix for missing error handler
ilyakuz-db Dec 10, 2024
a579b1c
fix: Use `convert.FromTyped` to generate dyn.Value
ilyakuz-db Dec 10, 2024
e15107f
feat: New annotations
ilyakuz-db Dec 10, 2024
9a55037
feat: Format markdown links to absolute
ilyakuz-db Dec 10, 2024
16042b5
fix: Adds 'markdownDescription' field to list of known keywords
ilyakuz-db Dec 10, 2024
0171513
fix: More details about markdownDescription in the comment
ilyakuz-db Dec 10, 2024
d2bfb67
fix: Few descriptions fixes
ilyakuz-db Dec 10, 2024
8f59695
Merge branch 'main' of github.com:databricks/cli into feat/custom-ann…
ilyakuz-db Dec 11, 2024
40505b8
fix: Annotations cleanup
ilyakuz-db Dec 11, 2024
d3b30f7
fix: Schema regenerate
ilyakuz-db Dec 11, 2024
43c4b58
feat: Use OneOf in interpolation patterns
ilyakuz-db Dec 12, 2024
aed0e0a
fix: Missing annotations
ilyakuz-db Dec 12, 2024
5194889
fix: Cleanup
ilyakuz-db Dec 12, 2024
8a33fb3
Merge branch 'main' into feat/custom-annotations-json-schema
pietern Dec 17, 2024
6bc9664
Run make lint
pietern Dec 17, 2024
1b31c09
fix: Example structure in annotationFile comment
ilyakuz-db Dec 17, 2024
7fb1ed8
fix: Typo in assignAnnotation
ilyakuz-db Dec 17, 2024
9f3ec3d
fix: Genkit re-generate
ilyakuz-db Dec 17, 2024
3231cff
fix: Remove unnecessary `ok` check
ilyakuz-db Dec 17, 2024
c619a73
fix: More explicit names for annotation data structures
ilyakuz-db Dec 17, 2024
3049184
test: Test cases for markdown link converter
ilyakuz-db Dec 17, 2024
0037671
fix: More explicit names and descriptions for `annotationHandler.sync…
ilyakuz-db Dec 17, 2024
7d8ae48
fix: Use OS-aware path joiner
ilyakuz-db Dec 17, 2024
34d7484
fix: Update test to use dynamic values for comparison instead of raw …
ilyakuz-db Dec 17, 2024
386c7d3
Merge branch 'main' of github.com:databricks/cli into feat/custom-ann…
ilyakuz-db Dec 17, 2024
835fb05
Merge branch 'main' into feat/custom-annotations-json-schema
pietern Dec 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .codegen.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"required": ["go"],
"post_generate": [
"go test -timeout 240s -run TestConsistentDatabricksSdkVersion github.com/databricks/cli/internal/build",
"go run ./bundle/internal/schema/*.go ./bundle/schema/jsonschema.json",
"go run ./bundle/internal/schema/*.go ./bundle/internal/schema/annotations.yml ./bundle/schema/jsonschema.json",
"echo 'bundle/internal/tf/schema/\\*.go linguist-generated=true' >> ./.gitattributes",
"echo 'go.sum linguist-generated=true' >> ./.gitattributes",
"echo 'bundle/schema/jsonschema.json linguist-generated=true' >> ./.gitattributes"
Expand Down
109 changes: 109 additions & 0 deletions bundle/internal/schema/annotations.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
package main

import (
"fmt"
"os"
"reflect"
"strings"

"github.com/ghodss/yaml"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use yamlsaver for saving to disk.

This gives you some control over the node style if you need it (e.g. use blocks for all descriptions).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to yamlsaver with yaml.LiteralStyle to make editing easier, this style seems the best for this purpose


"github.com/databricks/cli/libs/jsonschema"
)

type annotation struct {
Description string `json:"description,omitempty"`
MarkdownDescription string `json:"markdown_description,omitempty"`
Title string `json:"title,omitempty"`
Default any `json:"default,omitempty"`
}

type annotationHandler struct {
pietern marked this conversation as resolved.
Show resolved Hide resolved
filePath string
op *openapiParser
ref map[string]annotation
}

const Placeholder = "PLACEHOLDER"

// Adds annotations to the JSON schema reading from the annotations file.
// More details https://json-schema.org/understanding-json-schema/reference/annotations
func newAnnotationHandler(path string, op *openapiParser) (*annotationHandler, error) {
b, err := os.ReadFile(path)
if err != nil {
return nil, err
}

data := map[string]annotation{}
err = yaml.Unmarshal(b, &data)
if err != nil {
return nil, err
}

if data == nil {
data = map[string]annotation{}
}

d := &annotationHandler{}
d.ref = data
d.op = op
d.filePath = path
return d, nil
}

func (d *annotationHandler) addAnnotations(typ reflect.Type, s jsonschema.Schema) jsonschema.Schema {
// Skips the type if it is a part of OpenAPI spec, as it is already annotated.
_, ok := d.op.findRef(typ)
if ok {
return s
}

refPath := jsonschema.TypePath(typ)
Copy link
Contributor

@shreyas-goenka shreyas-goenka Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather keep typePath a private method because it deals with how to serialize #ref and work with the way our OpenAPI spec is structured.

We can instead directly use typ.PkgPath() + "." + typ.Name() here and error if either of them is an empty string. The rationale being that the annotations override will only be applicable for Go structs and not other types like primitive strings.

items := map[string]*jsonschema.Schema{}
items[refPath] = &s

for k, v := range s.Properties {
itemRefPath := fmt.Sprintf("%s.%s", refPath, k)
items[itemRefPath] = v
}

for k, v := range items {
// Skipping all default types like int, string, etc.
shouldHandle := strings.HasPrefix(refPath, "github.com")
if !shouldHandle {
continue
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value of the map is a value type, so item will already be zero-initialized if it doesn't exist.

The if !ok is not needed.


item, ok := d.ref[k]
if !ok {
d.ref[k] = annotation{
Description: Placeholder,
}
continue
}
if item.Description == Placeholder {
continue
}

v.Description = item.Description
if item.Default != nil {
v.Default = item.Default
}
v.MarkdownDescription = item.MarkdownDescription
v.Title = item.Title
}
return s
}

func (d *annotationHandler) overwrite() error {
data, err := yaml.Marshal(d.ref)
if err != nil {
return err
}

err = os.WriteFile(d.filePath, data, 0644)
if err != nil {
return err
}
return nil
}
Loading
Loading