-
Notifications
You must be signed in to change notification settings - Fork 63
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
Changes from all 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 12c4373
feat: Introduce separate files for OpenAPI annotations
ilyakuz-db 049e11b
test: Checks missing descriptions in annotation files
ilyakuz-db 8016d41
fix: Linter fix
ilyakuz-db 0221c28
fix: Remove redundant field
ilyakuz-db a3fdc8d
fix: Skip tests on windows
ilyakuz-db 79a88e9
fix: Run package instead of set of files
ilyakuz-db c3d049f
fix: Custom typePath function instead of exposing private `jsonschema…
ilyakuz-db b076813
fix: Change the structure of annotation files, minor fixes and tests
ilyakuz-db 073aeca
feat: Add styles
ilyakuz-db a6c45d5
feat: Yaml styles for open api overrides
ilyakuz-db d164968
Merge branch 'main' of github.com:databricks/cli into feat/custom-ann…
ilyakuz-db 00164a8
fix: Linter fix for missing error handler
ilyakuz-db a579b1c
fix: Use `convert.FromTyped` to generate dyn.Value
ilyakuz-db e15107f
feat: New annotations
ilyakuz-db 9a55037
feat: Format markdown links to absolute
ilyakuz-db 16042b5
fix: Adds 'markdownDescription' field to list of known keywords
ilyakuz-db 0171513
fix: More details about markdownDescription in the comment
ilyakuz-db d2bfb67
fix: Few descriptions fixes
ilyakuz-db 8f59695
Merge branch 'main' of github.com:databricks/cli into feat/custom-ann…
ilyakuz-db 40505b8
fix: Annotations cleanup
ilyakuz-db d3b30f7
fix: Schema regenerate
ilyakuz-db 43c4b58
feat: Use OneOf in interpolation patterns
ilyakuz-db aed0e0a
fix: Missing annotations
ilyakuz-db 5194889
fix: Cleanup
ilyakuz-db 8a33fb3
Merge branch 'main' into feat/custom-annotations-json-schema
pietern 6bc9664
Run make lint
pietern 1b31c09
fix: Example structure in annotationFile comment
ilyakuz-db 7fb1ed8
fix: Typo in assignAnnotation
ilyakuz-db 9f3ec3d
fix: Genkit re-generate
ilyakuz-db 3231cff
fix: Remove unnecessary `ok` check
ilyakuz-db c619a73
fix: More explicit names for annotation data structures
ilyakuz-db 3049184
test: Test cases for markdown link converter
ilyakuz-db 0037671
fix: More explicit names and descriptions for `annotationHandler.sync…
ilyakuz-db 7d8ae48
fix: Use OS-aware path joiner
ilyakuz-db 34d7484
fix: Update test to use dynamic values for comparison instead of raw …
ilyakuz-db 386c7d3
Merge branch 'main' of github.com:databricks/cli into feat/custom-ann…
ilyakuz-db 835fb05
Merge branch 'main' into feat/custom-annotations-json-schema
pietern File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,209 @@ | ||
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
|
||
// Annotations read from all annotation files including all overrides | ||
parsedAnnotations annotationFile | ||
// Missing annotations for fields that are found in config that need to be added to the annotation file | ||
missingAnnotations annotationFile | ||
} | ||
|
||
/** | ||
* Parsed file with annotations, expected format: | ||
* github.com/databricks/cli/bundle/config.Bundle: | ||
* cluster_id: | ||
* description: "Description" | ||
*/ | ||
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.parsedAnnotations = data | ||
d.missingAnnotations = 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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Which types don't have this prefix? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some primitive types like |
||
return s | ||
} | ||
|
||
annotations := d.parsedAnnotations[refPath] | ||
if annotations == nil { | ||
annotations = map[string]annotation{} | ||
} | ||
|
||
rootTypeAnnotation, ok := annotations[RootTypeKey] | ||
if ok { | ||
assignAnnotation(&s, rootTypeAnnotation) | ||
} | ||
|
||
for k, v := range s.Properties { | ||
item := annotations[k] | ||
if item.Description == "" { | ||
item.Description = Placeholder | ||
|
||
emptyAnnotations := d.missingAnnotations[refPath] | ||
if emptyAnnotations == nil { | ||
emptyAnnotations = map[string]annotation{} | ||
d.missingAnnotations[refPath] = emptyAnnotations | ||
} | ||
emptyAnnotations[k] = item | ||
} | ||
assignAnnotation(v, item) | ||
} | ||
return s | ||
} | ||
|
||
// Writes missing annotations with placeholder values back to the annotation file | ||
func (d *annotationHandler) syncWithMissingAnnotations(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.missingAnnotations, 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 assignAnnotation(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
|
Oops, something went wrong.
Oops, something went wrong.
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.
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.
Why does this need the path to itself?
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.
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 useembed
but I also need write operations so it seems like not an option in my case