Fixing #5537: Adding mandatory target annotation and their units. #5629

erharjotsingh · 2023-07-17T11:02:10Z

"Fixes #5537 : Adding mandatory target annotation and their units."

Proposed Changes

Add sample code for target annotation which is mandatory
Added comments for specifying units of the autoscaling types

…ry target values As per issue knative#5537, creating this PR for adding mandatory target annotation.

Issue# knative#5537: Update autoscaling-metrics.md to add the mandatory targ…

linux-foundation-easycla · 2023-07-17T11:02:15Z

The committers listed above are authorized under a signed CLA.

✅ login: erharjotsingh (07a85cb, 6bdd528, 0c051b8, b1b3cab, 4616ed9, 56029c7, fe6885c, 47a487c, 7d32ad5, c43c549, 08e71ee, 7debc35, 394ef47, 279d18f, 9ba7691, c92dfc5)

netlify · 2023-07-17T11:02:17Z

✅ Deploy Preview for knative ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`c92dfc5`
🔍 Latest deploy log	https://app.netlify.com/sites/knative/deploys/64c53e72d053ae0008259bc6
😎 Deploy Preview	https://deploy-preview-5629--knative.netlify.app/docs/serving/autoscaling/autoscaling-metrics
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

knative-prow · 2023-07-17T11:02:19Z

Welcome @erharjotsingh! It looks like this is your first PR to knative/docs 🎉

psschwei · 2023-07-18T13:08:15Z

docs/serving/autoscaling/autoscaling-metrics.md

@@ -32,6 +32,8 @@ For more information about KPA and HPA, see the documentation on [Supported Auto
        metadata:
          annotations:
            autoscaling.knative.dev/metric: "concurrency"
+            autoscaling.knative.dev/target-utilization-percentage: "70"
+            #autoscaling.knative.dev/target-utilization-percentage sepcifies a percentage value which when reached autoscaling is performed 


rather than making these types of comments in the yaml (which isn't super easy to read), it might be better to put this info after the yaml block

Hello @psschwei
Many thanks for your feedback. I have performed the suggested changes. Kindly review and thanks again!

Fixing knative#5537: Implementing PR comments

Fixing knative#5634: Update home.html to fix title issue with knative

Reverting the commit for other issue so that I can fork one branch per issue

psschwei · 2023-07-24T14:53:22Z

docs/serving/autoscaling/autoscaling-metrics.md

    ```
+# autoscaling.knative.dev/target-utilization-percentage annotation for "Concurrency" sepcifies a percentage value which when reached autoscaling is performed


This breaks the page formatting -- these need to be indented and have the # removed

A preview gets built on each commit you add: #5629 (comment) (or you can build locally and preview that way too)

Thanks @psschwei for sharing information about preview feature. Currently I am redirected to knative homepage when I click the preview link. https://deploy-preview-5629--knative.netlify.app/

This is the direct link to the preview of that page: https://deploy-preview-5629--knative.netlify.app/docs/serving/autoscaling/autoscaling-metrics/

Hello @psschwei Thanks for showing the link. I have submitted another PR. Kindly review. Thanks!
https://deploy-preview-5629--knative.netlify.app/docs/serving/autoscaling/autoscaling-metrics/

psschwei · 2023-07-26T13:38:31Z

docs/serving/autoscaling/autoscaling-metrics.md

+            autoscaling.knative.dev/target-utilization-percentage: "70" 
    ```
+autoscaling.knative.dev/target-utilization-percentage annotation for "Concurrency" specifies a percentage value


You'll need to indent all the comments, otherwise it breaks the layout. It also might be nice to format them as notes, i.e.

Suggested change

autoscaling.knative.dev/target-utilization-percentage: "70"

```

autoscaling.knative.dev/target-utilization-percentage annotation for "Concurrency" specifies a percentage value

autoscaling.knative.dev/target-utilization-percentage: "70"

```

!!! note

The `autoscaling.knative.dev/target-utilization-percentage` annotation for "Concurrency" specifies a percentage value

This should be done for all the updates

Hello @psschwei
Sure, I added the note section and content to next line by splitting it by br tag. enter was not working to move the content to next line same line as viewed in preview section.

psschwei

Again, you need to align these properly so as to not break the formatting... please check the preview to ensure the page is rendering properly (compare with the existing doc page if needed)

psschwei · 2023-07-28T00:05:54Z

docs/serving/autoscaling/autoscaling-metrics.md

+            autoscaling.knative.dev/target-utilization-percentage: "70" 
    ```
+!!! note <br/>


Suggested change

autoscaling.knative.dev/target-utilization-percentage: "70"

```

!!! note 

autoscaling.knative.dev/target-utilization-percentage: "70"

```

!!! note

Thanks @psschwei , I have compared the existing page and changes looks fine to me. Kindly review. Thanks again for your help!

Update autoscaling-metrics.md for formatting changes

psschwei · 2023-07-28T12:41:18Z

docs/serving/autoscaling/autoscaling-metrics.md

    ```
+    !!! note <br/>
+             The `autoscaling.knative.dev/target-utilization-percentage` annotation for "Concurrency" 
+              <br/> specifies a percentage value


These still aren't rendering properly...

Indents blocks are four spaces, and you don't need the the   tags at all.

Suggested change

```

!!! note 

The `autoscaling.knative.dev/target-utilization-percentage` annotation for "Concurrency"

 specifies a percentage value

```

!!! note

The `autoscaling.knative.dev/target-utilization-percentage` annotation for "Concurrency" specifies a percentage value

It should look like this:

Sure @psschwei Performed the suggested change. Thanks.

psschwei

/lgtm
/approve

knative-prow · 2023-07-31T11:19:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: erharjotsingh, psschwei

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~docs/serving/OWNERS~~ [psschwei]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

erharjotsingh added 2 commits July 17, 2023 16:26

Issue# knative#5537: Update autoscaling-metrics.md to add the mandato…

07a85cb

…ry target values As per issue knative#5537, creating this PR for adding mandatory target annotation.

Merge pull request #1 from erharjotsingh/erharjotsingh-patch-1

6bdd528

Issue# knative#5537: Update autoscaling-metrics.md to add the mandatory targ…

knative-prow bot requested review from nak3 and ReToCode July 17, 2023 11:02

knative-prow bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 17, 2023

psschwei requested changes Jul 18, 2023

View reviewed changes

knative-prow bot assigned psschwei Jul 18, 2023

erharjotsingh added 3 commits July 24, 2023 10:42

Fixing knative#5537: Implementing PR comments

0c051b8

Merge pull request #2 from erharjotsingh/erharjotsingh-patch-2

b1b3cab

Fixing knative#5537: Implementing PR comments

Merge branch 'knative:main' into main

4616ed9

erharjotsingh requested a review from psschwei July 24, 2023 05:20

erharjotsingh added 2 commits July 24, 2023 11:25

Fixing knative#5634: Update home.html

56029c7

Merge pull request #3 from erharjotsingh/erharjotsingh-patch-3

fe6885c

Fixing knative#5634: Update home.html to fix title issue with knative

knative-prow bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 24, 2023

Reverting the commit for other issue

47a487c

Reverting the commit for other issue so that I can fork one branch per issue

knative-prow bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 24, 2023

psschwei requested changes Jul 24, 2023

View reviewed changes

Fixing knative#5537: Adding review comments

7d32ad5

erharjotsingh requested a review from psschwei July 26, 2023 13:36

psschwei requested changes Jul 26, 2023

View reviewed changes

erharjotsingh added 2 commits July 27, 2023 09:33

Merge branch 'knative:main' into main

c43c549

Fixing#5537: Updating note formatting

08e71ee

knative-prow bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 27, 2023

erharjotsingh requested a review from psschwei July 27, 2023 04:24

psschwei requested changes Jul 28, 2023

View reviewed changes

erharjotsingh added 2 commits July 28, 2023 09:46

Update autoscaling-metrics.md

7debc35

Merge pull request #4 from erharjotsingh/erharjotsingh-patch-4

394ef47

Update autoscaling-metrics.md for formatting changes

erharjotsingh requested a review from psschwei July 28, 2023 04:21

erharjotsingh added 2 commits July 28, 2023 09:56

Update autoscaling-metrics.md

279d18f

Update autoscaling-metrics.md

9ba7691

psschwei requested changes Jul 28, 2023

View reviewed changes

Update autoscaling-metrics.md

c92dfc5

erharjotsingh requested a review from psschwei July 29, 2023 16:34

psschwei reviewed Jul 31, 2023

View reviewed changes

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jul 31, 2023

knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 31, 2023

knative-prow bot merged commit 9c9f95e into knative:main Jul 31, 2023
16 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing #5537: Adding mandatory target annotation and their units. #5629

Fixing #5537: Adding mandatory target annotation and their units. #5629

erharjotsingh commented Jul 17, 2023

linux-foundation-easycla bot commented Jul 17, 2023 •

edited

Loading

netlify bot commented Jul 17, 2023 •

edited

Loading

knative-prow bot commented Jul 17, 2023

psschwei Jul 18, 2023

erharjotsingh Jul 24, 2023 •

edited

Loading

psschwei Jul 24, 2023

psschwei Jul 24, 2023

erharjotsingh Jul 25, 2023

psschwei Jul 25, 2023 •

edited

Loading

erharjotsingh Jul 26, 2023 •

edited

Loading

psschwei Jul 26, 2023

erharjotsingh Jul 27, 2023

psschwei left a comment

psschwei Jul 28, 2023

erharjotsingh Jul 28, 2023

psschwei Jul 28, 2023

erharjotsingh Jul 29, 2023

psschwei left a comment

knative-prow bot commented Jul 31, 2023

		```
		# autoscaling.knative.dev/target-utilization-percentage annotation for "Concurrency" sepcifies a percentage value which when reached autoscaling is performed

Fixing #5537: Adding mandatory target annotation and their units. #5629

Fixing #5537: Adding mandatory target annotation and their units. #5629

Conversation

erharjotsingh commented Jul 17, 2023

Proposed Changes

linux-foundation-easycla bot commented Jul 17, 2023 • edited Loading

netlify bot commented Jul 17, 2023 • edited Loading

✅ Deploy Preview for knative ready!

knative-prow bot commented Jul 17, 2023

Choose a reason for hiding this comment

erharjotsingh Jul 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

psschwei Jul 25, 2023 • edited Loading

Choose a reason for hiding this comment

erharjotsingh Jul 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

psschwei left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

psschwei left a comment

Choose a reason for hiding this comment

knative-prow bot commented Jul 31, 2023

linux-foundation-easycla bot commented Jul 17, 2023 •

edited

Loading

netlify bot commented Jul 17, 2023 •

edited

Loading

erharjotsingh Jul 24, 2023 •

edited

Loading

psschwei Jul 25, 2023 •

edited

Loading

erharjotsingh Jul 26, 2023 •

edited

Loading