[ARVADOS] updated: 2.1.0-1837-g4ffe3382f
Git user
git at public.arvados.org
Thu Jan 20 20:41:19 UTC 2022
Summary of changes:
lib/config/cmd.go | 2 +-
lib/config/cmd_test.go | 2 +-
sdk/go/arvados/vocabulary.go | 38 +++++++++++++++++++++++++-------------
sdk/go/arvados/vocabulary_test.go | 35 ++++++++++++++++++++---------------
4 files changed, 47 insertions(+), 30 deletions(-)
via 4ffe3382ff35cebce873668dfdfad2eef2def3d3 (commit)
from 43536303547784d11d190e2cfdadda954005ae5d (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 4ffe3382ff35cebce873668dfdfad2eef2def3d3
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date: Thu Jan 20 17:00:17 2022 -0300
18487: Fixes error reporting to include both JSON & vocabulary errors at once.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>
diff --git a/lib/config/cmd.go b/lib/config/cmd.go
index 2f90bc80d..56a74a21c 100644
--- a/lib/config/cmd.go
+++ b/lib/config/cmd.go
@@ -165,7 +165,7 @@ func (checkCommand) RunCommand(prog string, args []string, stdin io.Reader, stdo
}
_, err = arvados.NewVocabulary(vd, mk)
if err != nil {
- fmt.Fprintf(stderr, "Error loading vocabulary file %q for cluster %s: %s\n", cc.API.VocabularyPath, id, err)
+ fmt.Fprintf(stderr, "Error loading vocabulary file %q for cluster %s:\n%s\n", cc.API.VocabularyPath, id, err)
return 1
}
}
diff --git a/lib/config/cmd_test.go b/lib/config/cmd_test.go
index 6b5ca4e57..2bb596a05 100644
--- a/lib/config/cmd_test.go
+++ b/lib/config/cmd_test.go
@@ -129,7 +129,7 @@ Clusters:
`
code := CheckCommand.RunCommand("arvados config-check", []string{"-config", "-"}, bytes.NewBufferString(in), &stdout, &stderr)
c.Check(code, check.Equals, 1)
- c.Check(stderr.String(), check.Matches, `(?s).*Error loading vocabulary file.*duplicate JSON key\(s\).*`)
+ c.Check(stderr.String(), check.Matches, `(?s).*Error loading vocabulary file.*for cluster.*\nduplicate JSON key "tags.IDfoo".*`)
}
func (s *CommandSuite) TestCheck_DeprecatedKeys(c *check.C) {
diff --git a/sdk/go/arvados/vocabulary.go b/sdk/go/arvados/vocabulary.go
index 4e155f34a..bb1bec789 100644
--- a/sdk/go/arvados/vocabulary.go
+++ b/sdk/go/arvados/vocabulary.go
@@ -67,14 +67,22 @@ func NewVocabulary(data []byte, managedTagKeys []string) (voc *Vocabulary, err e
}
return nil, fmt.Errorf("invalid JSON format: %q", err)
}
- // json.Unmarshal() doesn't error out on duplicate keys.
- err = checkJSONDupedKeys(json.NewDecoder(bytes.NewReader(data)), nil, &[]string{})
- if err != nil {
- return nil, err
- }
if reflect.DeepEqual(voc, &Vocabulary{}) {
return nil, fmt.Errorf("JSON data provided doesn't match Vocabulary format: %q", data)
}
+
+ shouldReportErrors := false
+ errors := []string{}
+
+ // json.Unmarshal() doesn't error out on duplicate keys.
+ dupedKeys := []string{}
+ err = checkJSONDupedKeys(json.NewDecoder(bytes.NewReader(data)), nil, &dupedKeys)
+ if err != nil {
+ shouldReportErrors = true
+ for _, dk := range dupedKeys {
+ errors = append(errors, fmt.Sprintf("duplicate JSON key %q", dk))
+ }
+ }
voc.reservedTagKeys = make(map[string]bool)
for _, managedKey := range managedTagKeys {
voc.reservedTagKeys[managedKey] = true
@@ -82,9 +90,13 @@ func NewVocabulary(data []byte, managedTagKeys []string) (voc *Vocabulary, err e
for systemKey := range voc.systemTagKeys() {
voc.reservedTagKeys[systemKey] = true
}
- err = voc.validate()
+ validationErrs, err := voc.validate()
if err != nil {
- return nil, err
+ shouldReportErrors = true
+ errors = append(errors, validationErrs...)
+ }
+ if shouldReportErrors {
+ return nil, fmt.Errorf("%s", strings.Join(errors, "\n"))
}
return voc, nil
}
@@ -135,19 +147,19 @@ func checkJSONDupedKeys(d *json.Decoder, path []string, errors *[]string) error
}
}
if len(path) == 0 && len(*errors) > 0 {
- return fmt.Errorf("duplicate JSON key(s):\n%s", strings.Join(*errors, "\n"))
+ return fmt.Errorf("duplicate JSON key(s) found")
}
return nil
}
-func (v *Vocabulary) validate() error {
+func (v *Vocabulary) validate() ([]string, error) {
if v == nil {
- return nil
+ return nil, nil
}
tagKeys := map[string]string{}
// Checks for Vocabulary strictness
if v.StrictTags && len(v.Tags) == 0 {
- return fmt.Errorf("vocabulary is strict but no tags are defined")
+ return nil, fmt.Errorf("vocabulary is strict but no tags are defined")
}
// Checks for collisions between tag keys, reserved tag keys
// and tag key labels.
@@ -191,9 +203,9 @@ func (v *Vocabulary) validate() error {
}
}
if len(errors) > 0 {
- return fmt.Errorf("invalid vocabulary:\n%s", strings.Join(errors, "\n"))
+ return errors, fmt.Errorf("invalid vocabulary")
}
- return nil
+ return nil, nil
}
func (v *Vocabulary) getLabelsToKeys() (labels map[string]string) {
diff --git a/sdk/go/arvados/vocabulary_test.go b/sdk/go/arvados/vocabulary_test.go
index da62d5c25..84b9bf229 100644
--- a/sdk/go/arvados/vocabulary_test.go
+++ b/sdk/go/arvados/vocabulary_test.go
@@ -6,6 +6,8 @@ package arvados
import (
"encoding/json"
+ "regexp"
+ "strings"
check "gopkg.in/check.v1"
)
@@ -56,7 +58,7 @@ func (s *VocabularySuite) SetUpTest(c *check.C) {
},
},
}
- err := s.testVoc.validate()
+ _, err := s.testVoc.validate()
c.Assert(err, check.IsNil)
}
@@ -267,7 +269,7 @@ func (s *VocabularySuite) TestNewVocabulary(c *check.C) {
false, `invalid JSON format:.*\(line \d+, column \d+\)`, nil,
},
{
- "Invalid JSON with duplicate keys",
+ "Invalid JSON with duplicate & reserved keys",
`{"tags":{
"type":{
"strict": false,
@@ -277,17 +279,7 @@ func (s *VocabularySuite) TestNewVocabulary(c *check.C) {
"labels": []
}
}}`,
- false, "(?s).*tags.type.labels.0.label\ntags.type", nil,
- },
- {
- "Valid data, but uses reserved key",
- `{"tags":{
- "type":{
- "strict": false,
- "labels": [{"label": "Class"}]
- }
- }}`,
- false, "(?s).*tag key.*is reserved", nil,
+ false, "(?s).*duplicate JSON key \"tags.type.labels.0.label\"\nduplicate JSON key \"tags.type\"\ntag key \"type\" is reserved", nil,
},
}
@@ -489,10 +481,23 @@ func (s *VocabularySuite) TestValidationErrors(c *check.C) {
}
for _, tt := range tests {
c.Log(c.TestName()+" ", tt.name)
- err := tt.voc.validate()
+ validationErrs, err := tt.voc.validate()
c.Assert(err, check.NotNil)
for _, errMatch := range tt.errMatches {
- c.Assert(err, check.ErrorMatches, errMatch)
+ seen := false
+ for _, validationErr := range validationErrs {
+ if regexp.MustCompile(errMatch).MatchString(validationErr) {
+ seen = true
+ break
+ }
+ }
+ if len(validationErrs) == 0 {
+ c.Assert(err, check.ErrorMatches, errMatch)
+ } else {
+ c.Assert(seen, check.Equals, true,
+ check.Commentf("Expected to see error matching %q:\n%s",
+ errMatch, strings.Join(validationErrs, "\n")))
+ }
}
}
}
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list