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: support multiple edfs argument templates #782
base: master
Are you sure you want to change the base?
feat: support multiple edfs argument templates #782
Conversation
if lastInputValueTypeRef == -1 { | ||
return "", fmt.Errorf(`the event subject "%s" defines the final nested input field "%s" in argument template #%d, but it does not exist`, subject, argumentPath[len(argumentPath)-1], templateNumber+1) | ||
} | ||
argumentName := argumentPath[0] |
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.
Can this panic?
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.
if len(matches) != 2 {
return "", fmt.Errorf("expected a single matching group for argument template")
}
// the path begins with ".", so ignore the first empty string element
argumentPath := strings.Split(matches[1][1:], ".")
We only have a match if the regex matches, which must be [a-zA-Z0-9_]
after a period, which means we should be guaranteed to have at least one item in the argument path. If we don't, the regex didn't match, so we'd never get there.
Either way, this tells me that some regex tests would be useful. I'll add them.
if len(matches) != 1 || len(matches[0]) != 2 { | ||
return "", fmt.Errorf("expected subject to match regex") | ||
func (p *Planner[T]) inputObjectDefinitionRefByFieldDefinitionRef(fieldDefinitionRef int, argumentNameBytes []byte) (inputObjectDefinitionRef int, inputValueTypeRef int) { | ||
inputValueTypeRef = -1 |
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.
Could you use here and below ast.InvalidRef
instead of -1
literal
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.
Yep. I'll change it.
Path: []string{string(variableName)}, | ||
Renderer: renderer, | ||
// substitute the variable templates for dummy values to check that the string is a valid NATS subject | ||
if natsserver.IsValidSubject(variableTemplateRegex.ReplaceAllLiteralString(substitutedSubject, "a")) { |
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.
Can we abstract it in a way that we don't get provider specific in the pubsub implementation? For Kafka, topics should be immutable.
No description provided.