-
Notifications
You must be signed in to change notification settings - Fork 112
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
fixes #37474 - feat: enable updating ansible override values via api #719
base: master
Are you sure you want to change the base?
fixes #37474 - feat: enable updating ansible override values via api #719
Conversation
ce46349
to
e3f4c01
Compare
e3f4c01
to
dbdcf3a
Compare
Signed-off-by: gardar <[email protected]>
dbdcf3a
to
d8176a0
Compare
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 noticed two things:
- When trying to update the value for a variable that doesn't have an override value, we get an error. We should catch that error and print an informative message to the user.
- When a user without the required permissions tries to override a value, the necessary permissions are not specified in the error message.
I was actually originally planning to modify the current POST method of the
In my experience this is actually the case for the other api endpoints as well, the necessary permissions are not specified in the error message. |
Why should we use POST for both? I think we should stick to the expected behavior where POST is for creating and PUT is for updating.
The corresponding POST request for creating the override value specifies the necessary permissions in the error message. We can follow the same functionality for the PUT request as well. |
…date override values Signed-off-by: gardar <[email protected]>
a893b89
to
cbd71d6
Compare
Signed-off-by: gardar <[email protected]>
As reported here:
https://community.theforeman.org/t/update-ansible-variable-overrides-over-the-api-gives-error/
and here:
https://projects.theforeman.org/issues/37474