DataSource: Move the shared parts of Add+Update commands into a base struct #88036
+604
−457
Conversation
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
ryantxu
added
no-backport
ryantxu
requested review from
juanicabanas,
lucychen-grafana,
wbrowne,
andresmgot,
oshirohugo,
jtheory,
rwwiv,
JacobsonMT,
yuri-tceretian,
grobinson-grafana and
suntala
and removed request for
a team
andresmgot
reviewed
May 20, 2024
pkg/services/datasources/models.go
Outdated
| type AddDataSourceCommand struct { | ||
| // The input values shared with both Add+Update commands (also part of the DTO) | ||
| // This part of the command can share validation between add and update actions | ||
| type BaseDataSourceCommand struct { |
There was a problem hiding this comment.
the name may be a bit confusing because it could mean that DeleteDatasource also extends this one. Maybe something like BaseWriteDataSourceCommand?
marefr
reviewed
May 20, 2024
pkg/services/datasources/models.go
Outdated
| ReadOnly bool `json:"-"` | ||
| EncryptedSecureJsonData map[string][]byte `json:"-"` | ||
| UpdateSecretFn UpdateSecretFn `json:"-"` | ||
| } | ||
|
|
||
| // Also acts as api DTO | ||
| type AddDataSourceCommand struct { | ||
| BaseDataSourceCommand |
There was a problem hiding this comment.
Nit. Does this need json inline tag to make the JSON unmarshal work as expected?
pkg/services/datasources/models.go
Outdated
| APIVersion string `json:"apiVersion"` | ||
| // swagger:ignore | ||
| IsPrunable bool | ||
| BaseDataSourceCommand |
There was a problem hiding this comment.
Nit. Does this need json inline tag to make the JSON unmarshal work as expected?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is an alternative approach to #88018
Rather than unify the two commands, this puts the shared parts into a base struct.
This adds a lot more typing when creating commands, but at least makes the difference between the two commands more clear 🤷🏻