-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat: add secrets / config to a verbs metadata #2999
Conversation
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.
This is great, thanks Stuart.
@@ -130,6 +130,7 @@ func genErrorf(pos token.Pos, format string, args ...any) error { | |||
|
|||
var tmpl = template.Must(template.New("proto"). | |||
Parse(` | |||
// THIS FILE IS GENERATED; DO NOT MODIFY |
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.
Good call.
internal/schema/metadataconfig.go
Outdated
func (m *MetadataConfig) Position() Position { return m.Pos } | ||
func (m *MetadataConfig) String() string { | ||
out := &strings.Builder{} | ||
fmt.Fprint(out, "+calls ") |
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.
fmt.Fprint(out, "+calls ") | |
fmt.Fprint(out, "+config ") |
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.
Ugh, sorry, this PR is mostly just copypasta from from the calls metadata.
internal/schema/metadataconfig.go
Outdated
out := &strings.Builder{} | ||
fmt.Fprint(out, "+calls ") | ||
w := 6 | ||
for i, call := range m.Config { |
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.
s/call/config/g
internal/schema/metadatasecrets.go
Outdated
func (m *MetadataSecrets) Position() Position { return m.Pos } | ||
func (m *MetadataSecrets) String() string { | ||
out := &strings.Builder{} | ||
fmt.Fprint(out, "+calls ") |
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.
fmt.Fprint(out, "+calls ") | |
fmt.Fprint(out, "+secrets ") |
internal/schema/metadatasecrets.go
Outdated
type MetadataSecrets struct { | ||
Pos Position `parser:"" protobuf:"1,optional"` | ||
|
||
Secrets []*Ref `parser:"'+' 'config' @@ (',' @@)*" protobuf:"2"` |
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.
Secrets []*Ref `parser:"'+' 'config' @@ (',' @@)*" protobuf:"2"` | |
Secrets []*Ref `parser:"'+' 'secrets' @@ (',' @@)*" protobuf:"2"` |
internal/schema/metadatasecrets.go
Outdated
out := &strings.Builder{} | ||
fmt.Fprint(out, "+calls ") | ||
w := 6 | ||
for i, call := range m.Secrets { |
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.
s/call/ref/g
@@ -341,8 +341,11 @@ func ValidateModule(module *Module) error { | |||
|
|||
case *Data: | |||
for _, md := range n.Metadata { | |||
if md, ok := md.(*MetadataCalls); ok { | |||
switch md.(type) { | |||
case *MetadataCalls, *MetadataSecrets, *MetadataConfig: |
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'm wondering if we should have some kind of mapping for this instead...a map of which metadata types are valid on which schema node types.
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.
Yea, there are definitely gaps in the current logic, I don't think it is high priority though.
58e9ed0
to
b9b0718
Compare
func (m *MetadataSecrets) Position() Position { return m.Pos } | ||
func (m *MetadataSecrets) String() string { | ||
out := &strings.Builder{} | ||
fmt.Fprint(out, "+secrets ") |
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 forgot to say, there are tests that should be updated to exercise this parsing, in particular TestParserRoundtrip et al and TestProtoRoundTrip.
b9b0718
to
8e83519
Compare
27df487
to
3986925
Compare
fixes: #1083
This just adds them for JVM languages for now, as the go implementation will change when they move to an injection model.