CDS-58185-https-polling #1784

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.

Sign up for GitHub

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

Jump to bottom

Merged

roshnisarangadharan merged 4 commits into main from CDS-58185-http-poll

May 23, 2023

Contributor

roshnisarangadharan commented May 18, 2023

Harness Developer Pull Request

Thanks for helping us make the Developer Hub better. The PR will be looked at
by the maintainers.

What Type of PR is This?

Issue
Feature
Maintenance/Chore

If tied to an Issue, list the Issue(s) here:

Issue(s)

House Keeping

Some items to keep track of. Screen shots of changes are optional but would help the maintainers review quicker.

Tested Locally
Optional Screen Shoot.


          CDS-58185-https-polling

fd69ce3

roshnisarangadharan added documentation DO NOT MERGE labels

roshnisarangadharan requested a review from thisrohangupta

May 18, 2023 17:09

roshnisarangadharan self-assigned this

bot-gitexp-user commented May 18, 2023

Please check the Execution Link of the Pipeline for the Website Draft URL. This is located in the Preview Step and also is available in #hdh_alerts. E.g Website Draft URL: https://unique-id--harness-developer.netlify.app

thisrohangupta previously approved these changes

View reviewed changes

Collaborator

thisrohangupta left a comment

This looks good

deepakbolangady approved these changes

View reviewed changes

Contributor

deepakbolangady left a comment

Some minor things to check. I'm approving assuming that you'll make the changes. Let me know if I need to approve again. Thank you.

docs/continuous-delivery/x-platform-cd-features/cd-steps/utilities/http-step.md Outdated

               Capability checks are basic accessibility checks and do not follow multiple redirects. Hence, Harness returns from the first `302 Found` response during capability checks.
+              ## HTTP polling
+              HTTP step supports polling. When you create the HTTP step for polling, the client will keep requesting the resource at regular intervals.

Contributor

deepakbolangady May 23, 2023

Suggested change

      
            HTTP step supports polling. When you create the HTTP step for polling, the client will keep requesting the resource at regular intervals. 
          
            The HTTP step supports polling. When you create the HTTP step for polling, the client will keep requesting the resource at regular intervals.

About "When you create the HTTP step for polling": the when clause means "At the time that ..." So it's more a point in time thing, right? We are describing events that continue to happen subsequently. Should we remove that clause or reword it? (Check my suggestion at the end of this comment.)
"Will keep requesting": check whether "requests" works just as well. The simple present works in most cases.

Overall, check whether something like this works:
The HTTP step supports polling. When polling is enabled, the client requests the resource at regular intervals.

Contributor Author

roshnisarangadharan May 23, 2023

No, polling is not enabled. Here, the idea is creating an HTTP step for the purpose of HTTP polling.

Contributor

deepakbolangady May 23, 2023

Then, can we write "During the execution of the step" or "when the step is being executed"? The idea is not to identify a single point in time with "when you create ..." but to identify the period of time during which the client makes periodic requests.

docs/continuous-delivery/x-platform-cd-features/cd-steps/utilities/http-step.md Outdated Show resolved Hide resolved

docs/continuous-delivery/x-platform-cd-features/cd-steps/utilities/http-step.md Outdated


		To support HTTP polling:

		1. In the step's Step Parameters > Optional Configuration, enter the following details:

Contributor

deepakbolangady May 23, 2023 •

edited

Loading

I had to read the first few words more than once, but it might be an issue only with me, or maybe the multiple asterisks threw me off. Consider the following:
In the step, go to Step Parameters > Optional Configuration, and then enter the following details
In general, prefer not to use UI element names in running text.

docs/continuous-delivery/x-platform-cd-features/cd-steps/utilities/http-step.md Outdated

+                      * `<+httpUrl>`
+                      * `<+httpMethod>`
+                      * `<+httpResponseBody>`
+                  * **Headers (optional)**: Enter the key and value for the headers for the message.

Contributor

deepakbolangady May 23, 2023

Would "in the message" be the right wording? (Unless I've wrongly interpreted this text and the headers are going to be added/inserted. If we are evaluating the response and matching with key-value pairs, "in" will be a better choice. Please take a call.) Two fors one after another can make a sentence hard to interpret.

docs/continuous-delivery/x-platform-cd-features/cd-steps/utilities/http-step.md Outdated

+                      * `<+httpResponseBody>`
+                  * **Headers (optional)**: Enter the key and value for the headers for the message.
+                    For example, in **Key**, enter token, in **Value**, enter secret references such as `<+secrets.getValue("aws-playground_AWS_secret_key")>`.

Contributor

deepakbolangady May 23, 2023

Suggested change

      
                  For example, in **Key**, enter token, in **Value**, enter secret references such as `<+secrets.getValue("aws-playground_AWS_secret_key")>`.
          
                  For example, in **Key**, enter `token`, and, in **Value**, enter secret references such as `<+secrets.getValue("aws-playground_AWS_secret_key")>`.

Contributor Author

roshnisarangadharan May 23, 2023

If we say token that would mean we want the users to enter token. They're supposed to enter the token value. I'll change the text to .. in Key, enter the token...

Contributor

deepakbolangady May 23, 2023

Yes, that works. Thanks.

docs/continuous-delivery/x-platform-cd-features/cd-steps/utilities/http-step.md Outdated Show resolved Hide resolved

docs/continuous-delivery/x-platform-cd-features/cd-steps/utilities/http-step.md Outdated

+. In **Advanced >** **Failure Strategy**:
+                  * In **On failure of type**, select **All Errors**.
+                  * In **Perform Action**, select **Retry**.
+                  * Enter the **Retry count** and **Retry intervals**.

Contributor

deepakbolangady May 23, 2023 •

edited

Loading

Please let the UX team know that we should use the singular of "intervals".
Avoid using UI element names in flowing text. I don't know how else to describe it. Consider "In Retry count and Retry intervals, specify ..., respectively. (Or split the instruction into two instructions.)

docs/continuous-delivery/x-platform-cd-features/cd-steps/utilities/http-step.md Outdated Show resolved Hide resolved

docs/continuous-delivery/x-platform-cd-features/cd-steps/utilities/http-step.md Show resolved Hide resolved

docs/continuous-delivery/x-platform-cd-features/cd-steps/utilities/http-step.md Show resolved Hide resolved

deepakbolangady previously approved these changes

View reviewed changes

Contributor

deepakbolangady left a comment

Some minor things to check. I'm approving assuming that you'll make the changes. Let me know if I need to approve again. Thank you.

roshnisarangadharan commented

View reviewed changes

docs/continuous-delivery/x-platform-cd-features/cd-steps/utilities/http-step.md Show resolved Hide resolved

roshnisarangadharan commented

View reviewed changes

docs/continuous-delivery/x-platform-cd-features/cd-steps/utilities/http-step.md Outdated Show resolved Hide resolved

roshnisarangadharan dismissed stale reviews from deepakbolangady and thisrohangupta via

03e611b

May 23, 2023 07:40


          Apply suggestions from code review

03e611b

Co-authored-by: Deepak Bolangady <deepak.bolangady@harness.io>

bot-gitexp-user commented May 23, 2023

Please check the Execution Link of the Pipeline for the Website Draft URL. This is located in the Preview Step and also is available in #hdh_alerts. E.g Website Draft URL: https://unique-id--harness-developer.netlify.app


          review

1a9f9b4

roshnisarangadharan commented

View reviewed changes

docs/continuous-delivery/x-platform-cd-features/cd-steps/utilities/http-step.md Outdated Show resolved Hide resolved

roshnisarangadharan commented

View reviewed changes

docs/continuous-delivery/x-platform-cd-features/cd-steps/utilities/http-step.md Outdated Show resolved Hide resolved


          Apply suggestions from code review

db44634

roshnisarangadharan requested a review from deepakbolangady

May 23, 2023 07:58

bot-gitexp-user commented May 23, 2023

Please check the Execution Link of the Pipeline for the Website Draft URL. This is located in the Preview Step and also is available in #hdh_alerts. E.g Website Draft URL: https://unique-id--harness-developer.netlify.app

1 similar comment

bot-gitexp-user commented May 23, 2023

Please check the Execution Link of the Pipeline for the Website Draft URL. This is located in the Preview Step and also is available in #hdh_alerts. E.g Website Draft URL: https://unique-id--harness-developer.netlify.app

roshnisarangadharan merged commit c85edf8 into main

roshnisarangadharan deleted the CDS-58185-http-poll branch

May 23, 2023 09:04

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels