这是indexloc提供的服务,不要输入任何密码
Skip to content

[AIP-163] Approve the change validation AIP. #437

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

Merged
merged 3 commits into from
Mar 27, 2020
Merged

Conversation

lukesneeringer
Copy link
Contributor

This issue is intended for public comment, and should remain open through March 27, 2020.

@lukesneeringer lukesneeringer added this to the March 2020 Newsletter milestone Feb 21, 2020
@lukesneeringer lukesneeringer requested a review from a team February 21, 2020 18:15
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 21, 2020
@nimahak
Copy link

nimahak commented Feb 29, 2020

transferring from issue-573078166:

It would be great to expand the guidance in AIP-163 on how to handle standard methods and general resource lifecycle.

For example, what is the expected behavior of a Create or Delete with validation_only flag set?

  • Should Create return 200 OK on dryrun success?
  • Is it OK for a subsequent GET to return 404 if Create returned 200?
  • Would it be advised (or OK) for Create to return 412 PRECONDITION FAILED or similar instead?

A couple places where an amendment for dry run could help:

https://aip.dev/151:

When creating or deleting a resource with a long-running operation, the resource should be included in List and Get calls; however, the resource should indicate that it is not usable, generally with a state enum.

https://aip.dev/135:

The Delete method should succeed if and only if a resource was present and was successfully deleted.

@lukesneeringer
Copy link
Contributor Author

Happy to add something for this. Incidentally. the answers to these questions are:

Should Create return 200 OK on dryrun success?

Yes.

Is it OK for a subsequent GET to return 404 if Create returned 200?

Yes (and anything else would not make sense).

Would it be advised (or OK) for Create to return 412 PRECONDITION FAILED or similar instead?

No. The dry run succeeded.

@jasonmc
Copy link

jasonmc commented Mar 2, 2020

a request using validate_only must fail if the actual request would fail for any reason.

This seems overly strong: a validate_only call typically would not create state in a database, but certain classes of errors can only happen when a call to commit a transaction is actually made.

@lukesneeringer
Copy link
Contributor Author

lukesneeringer commented Mar 5, 2020

This seems overly strong: a validate_only call typically would not create state in a database, but certain classes of errors can only happen when a call to commit a transaction is actually made.

This is a great point, thanks @jasonmc. I have amended the language to: "a request using validate_only must fail if it determines that the actual request would fail."

@lukesneeringer
Copy link
Contributor Author

It would be great to expand the guidance in AIP-163 on how to handle standard methods and general resource lifecycle.

I did a minor re-word to try and be more clear about this; let me know if you think it is clear enough.

@lukesneeringer lukesneeringer merged commit c1d69ac into master Mar 27, 2020
@lukesneeringer lukesneeringer deleted the aip-163-approval branch March 27, 2020 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants