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 27 commits
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",
"make schema",
"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
9 changes: 7 additions & 2 deletions .github/workflows/push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,19 @@ jobs:
# By default the ajv-cli runs in strict mode which will fail if the schema
# itself is not valid. Strict mode is more strict than the JSON schema
# specification. See for details: https://ajv.js.org/options.html#strict-mode-options
# The ajv-cli is configured to use the markdownDescription keyword which is not part of the JSON schema specification,
# but is used in editors like VSCode to render markdown in the description field
- name: Validate bundle schema
run: |
go run main.go bundle schema > schema.json

# Add markdownDescription keyword to ajv
echo "module.exports=function(a){a.addKeyword('markdownDescription')}" >> keywords.js

for file in ./bundle/internal/schema/testdata/pass/*.yml; do
ajv test -s schema.json -d $file --valid
ajv test -s schema.json -d $file --valid -c=./keywords.js
done

for file in ./bundle/internal/schema/testdata/fail/*.yml; do
ajv test -s schema.json -d $file --invalid
ajv test -s schema.json -d $file --invalid -c=./keywords.js
done
6 changes: 5 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,8 @@ vendor:
integration:
gotestsum --format github-actions --rerun-fails --jsonfile output.json --packages "./integration/..." -- -parallel 4 -timeout=2h

.PHONY: fmt lint lintcheck test testonly coverage build snapshot vendor integration
schema:
@echo "✓ Generating json-schema ..."
@go run ./bundle/internal/schema ./bundle/internal/schema ./bundle/schema/jsonschema.json
Comment on lines +33 to +35
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this need the path to itself?

Copy link
Contributor Author

@ilyakuz-db ilyakuz-db Dec 17, 2024

Choose a reason for hiding this comment

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

I passed this as argument to avoid hardcoding relative paths in Go (to keep an ability to execute both from make schema and from other places like Run from IDE). Other possible option to keep portability is to use embed but I also need write operations so it seems like not an option in my case


.PHONY: fmt lint lintcheck test testonly coverage build snapshot vendor integration schema
204 changes: 204 additions & 0 deletions bundle/internal/schema/annotations.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,204 @@
package main

import (
"bytes"
"fmt"
"os"
"reflect"
"regexp"
"strings"

yaml3 "gopkg.in/yaml.v3"

"github.com/databricks/cli/libs/dyn"
"github.com/databricks/cli/libs/dyn/convert"
"github.com/databricks/cli/libs/dyn/merge"
"github.com/databricks/cli/libs/dyn/yamlloader"
"github.com/databricks/cli/libs/dyn/yamlsaver"
"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"`
Enum []any `json:"enum,omitempty"`
}

type annotationHandler struct {
pietern marked this conversation as resolved.
Show resolved Hide resolved
ref annotationFile
empty annotationFile
}

type annotationFile map[string]map[string]annotation
pietern marked this conversation as resolved.
Show resolved Hide resolved

const Placeholder = "PLACEHOLDER"

// Adds annotations to the JSON schema reading from the annotation files.
// More details https://json-schema.org/understanding-json-schema/reference/annotations
func newAnnotationHandler(sources []string) (*annotationHandler, error) {
prev := dyn.NilValue
for _, path := range sources {
b, err := os.ReadFile(path)
if err != nil {
return nil, err
}
generated, err := yamlloader.LoadYAML(path, bytes.NewBuffer(b))
if err != nil {
return nil, err
}
prev, err = merge.Merge(prev, generated)
if err != nil {
return nil, err
}
}

var data annotationFile

err := convert.ToTyped(&data, prev)
if err != nil {
return nil, err
}

d := &annotationHandler{}
d.ref = data
d.empty = annotationFile{}
return d, nil
}

func (d *annotationHandler) addAnnotations(typ reflect.Type, s jsonschema.Schema) jsonschema.Schema {
refPath := getPath(typ)
shouldHandle := strings.HasPrefix(refPath, "github.com")
if !shouldHandle {
Copy link
Contributor

Choose a reason for hiding this comment

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

Which types don't have this prefix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some primitive types like int64, string etc

return s
}

annotations := d.ref[refPath]
if annotations == nil {
annotations = map[string]annotation{}
}

rootTypeAnnotation, ok := annotations[RootTypeKey]
if ok {
assingAnnotation(&s, rootTypeAnnotation)
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo

Suggested change
assingAnnotation(&s, rootTypeAnnotation)
assignAnnotation(&s, rootTypeAnnotation)

}

for k, v := range s.Properties {
item, ok := annotations[k]
if !ok {
item = annotation{}
}
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.

if item.Description == "" {
item.Description = Placeholder

emptyAnnotations := d.empty[refPath]
if emptyAnnotations == nil {
emptyAnnotations = map[string]annotation{}
d.empty[refPath] = emptyAnnotations
}
emptyAnnotations[k] = item
}
assingAnnotation(v, item)
}
return s
}

// Adds empty annotations with placeholders to the annotation file
func (d *annotationHandler) sync(outputPath string) error {
existingFile, err := os.ReadFile(outputPath)
if err != nil {
return err
}
existing, err := yamlloader.LoadYAML("", bytes.NewBuffer(existingFile))
if err != nil {
return err
}
missingAnnotations, err := convert.FromTyped(&d.empty, dyn.NilValue)
if err != nil {
return err
}

output, err := merge.Merge(existing, missingAnnotations)
if err != nil {
return err
}

err = saveYamlWithStyle(outputPath, output)
if err != nil {
return err
}
return nil
}

func getPath(typ reflect.Type) string {
return typ.PkgPath() + "." + typ.Name()
}

func assingAnnotation(s *jsonschema.Schema, a annotation) {
if a.Description != Placeholder {
s.Description = a.Description
}

if a.Default != nil {
s.Default = a.Default
}
s.MarkdownDescription = convertLinksToAbsoluteUrl(a.MarkdownDescription)
s.Title = a.Title
s.Enum = a.Enum
}

func saveYamlWithStyle(outputPath string, input dyn.Value) error {
style := map[string]yaml3.Style{}
file, _ := input.AsMap()
for _, v := range file.Keys() {
style[v.MustString()] = yaml3.LiteralStyle
}

saver := yamlsaver.NewSaverWithStyle(style)
err := saver.SaveAsYAML(file, outputPath, true)
if err != nil {
return err
}
return nil
}

func convertLinksToAbsoluteUrl(s string) string {
if s == "" {
return s
}
base := "https://docs.databricks.com"
referencePage := "/dev-tools/bundles/reference.html"

// Regular expression to match Markdown-style links like [_](link)
re := regexp.MustCompile(`\[_\]\(([^)]+)\)`)
result := re.ReplaceAllStringFunc(s, func(match string) string {
matches := re.FindStringSubmatch(match)
if len(matches) < 2 {
return match
}
link := matches[1]
var text, absoluteURL string

if strings.HasPrefix(link, "#") {
text = strings.TrimPrefix(link, "#")
absoluteURL = fmt.Sprintf("%s%s%s", base, referencePage, link)

// Handle relative paths like /dev-tools/bundles/resources.html#dashboard
} else if strings.HasPrefix(link, "/") {
absoluteURL = strings.ReplaceAll(fmt.Sprintf("%s%s", base, link), ".md", ".html")
if strings.Contains(link, "#") {
parts := strings.Split(link, "#")
text = parts[1]
} else {
text = "link"
}
} else {
return match
}

return fmt.Sprintf("[%s](%s)", text, absoluteURL)
})

return result
}
pietern marked this conversation as resolved.
Show resolved Hide resolved
Loading
Loading