Skip to content

Give option to create a POST check. - #3856

Closed
slz250 wants to merge 3 commits into
GoogleCloudPlatform:masterfrom
slz250:patch-2
Closed

Give option to create a POST check.#3856
slz250 wants to merge 3 commits into
GoogleCloudPlatform:masterfrom
slz250:patch-2

Conversation

@slz250

Copy link
Copy Markdown
Contributor

In the create call demo -- allow uncommenting of POST check code. This is a new feature and will help drive adoption.

In the create call demo -- allow uncommenting of POST check code. This is a new feature and will help drive adoption.
@slz250
slz250 requested a review from a team as a code ownerMay 21, 2020 18:44
@googlebotgooglebot added the cla: yes This human has signed the Contributor License Agreement. label May 21, 2020
@gguussgguuss added the kokoro:run Add this label to force Kokoro to re-run the tests. label May 21, 2020
@kokoro-teamkokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label May 21, 2020

@gguussgguuss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as lint is passing and this is just a comment change.

Do you want to just expose this as a comment or would it make sense to add an additional example and also add a test for the POST check?

It may be worth getting someone closer to monitoring on the DPE side for a second approval.

@tmatsuotmatsuo added the kokoro:run Add this label to force Kokoro to re-run the tests. label Jun 1, 2020
@kokoro-teamkokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jun 1, 2020
@tmatsuo

Copy link
Copy Markdown
Contributor

As @gguuss mentioned, it might be better to have another sample with a test.
If it's in the comment, there's no way to know if it's working.

@busunkim96busunkim96 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slz250 +1 to @tmatsuo's comment. Could you create a new sample that creates an uptime check config with a post check?

We prefer not to have code commented out because it's not possible to test it. If the library changes for some reason, this sample could break.

@leahecole

Copy link
Copy Markdown
Collaborator

#4082 was just merged

@leahecole

Copy link
Copy Markdown
Collaborator

So I definitely did not intentionally unassign @gguuss - it did that when I commented, and now I can't re-add him. 😬

@slz250slz250 closed this Jul 7, 2020
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yesThis human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@slz250@tmatsuo@leahecole@gguuss@busunkim96@googlebot@kokoro-team@anguillanneuf