-
Notifications
You must be signed in to change notification settings - Fork 42
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
Add datasources CLI stubs #5020
Conversation
We can ignore the "proto-breaking-changes" check as the code is not used and still under development. |
cmd/cli/app/datasource/common.go
Outdated
defer closer() | ||
|
||
ds := &minderv1.DataSource{} | ||
if err := minderv1.ParseResource(reader, ds); err != nil { |
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.
We need to use ParseResourceProto
instead. This won't work because of the oneof
usage.
proto/minder/v1/minder.proto
Outdated
} | ||
|
||
message ListDataSourcesRequest { | ||
ContextV2 context = 1; | ||
} | ||
|
||
message ListDataSourcesResponse { | ||
// TODO | ||
repeated DataSource data_sources = 1; | ||
} | ||
|
||
message UpdateDataSourceRequest { | ||
ContextV2 context = 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.
Interesting, do we need the Context here since we have it as part of the DataSource?
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.
That's true, I missed this. Thanks
---Testing locally--- Results are as expected:
Server message
|
Summary
Provide a brief overview of the changes and the issue being addressed.
Explain the rationale and any background necessary for understanding the changes.
List dependencies required by this change, if any.
Fixes https://github.com/stacklok/minder-stories/issues/135
Change Type
Mark the type of change your PR introduces:
Testing
Outline how the changes were tested, including steps to reproduce and any relevant configurations.
Attach screenshots if helpful.
Review Checklist: