-
Notifications
You must be signed in to change notification settings - Fork 11.7k
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
DataSource: Use the same struct for both Add+Update commands #88018
Conversation
@@ -4645,6 +4597,58 @@ | |||
}, | |||
"type": "object" | |||
}, | |||
"DataSourceCommand": { |
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.
really just a name change for the command object
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.
The internal changes look fine to me, but could we still specify separate swagger definitions for adding and updating? Firstly, it helps to make it clearer for users which fields can be specified for which operation. Secondly, it means that we don't introduce a breaking change for Terraform where we use the current commands (DS resource in Terraform).
Con: It makes things less explicit/harder for devs to understand what fields to provide for a create vs update and easier to introduce unexpected bugs. |
Closing this in favor of #88036 -- the syntax is annoying, but I think leads to a better solution |
See an alternative approach #88036
In #87718 -- we need to duplicate the validation functions because the commands for Add/Update are almost identical -- but not identical.
An alternative approach would be to make a base class for the common properties and extend the explicit ones for each -- but that seems like more work than it is worth.
The difference is that Update has a version and internal ID -- and Add has a UserID 馃し馃徎