Skip to content

preparation for Revision api change#11068

Merged
knative-prow-robot merged 2 commits intoknative:mainfrom
leetcope:prepare-revision-api-change
Apr 1, 2021
Merged

preparation for Revision api change#11068
knative-prow-robot merged 2 commits intoknative:mainfrom
leetcope:prepare-revision-api-change

Conversation

@leetcope
Copy link
Contributor

@leetcope leetcope commented Mar 30, 2021

Fixes #10750

Proposed Changes

  • Revision API migration preparation

@google-cla google-cla bot added the cla: yes Indicates the PR's author has signed the CLA. label Mar 30, 2021
@knative-prow-robot knative-prow-robot added area/API API objects and controllers size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 30, 2021
@codecov
Copy link

codecov bot commented Mar 30, 2021 •
edited
Loading

Codecov Report

Merging #11068 (2d4834c) into main (7931069) will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #11068      +/-   ##
==========================================
- Coverage   87.68%   87.66%   -0.03%     
==========================================
  Files         190      190              
  Lines        9155     9159       +4     
==========================================
+ Hits         8028     8029       +1     
- Misses        874      876       +2     
- Partials      253      254       +1
Impacted Files Coverage Δ
pkg/apis/serving/v1/revision_types.go 100.00% <ø> (ø)
pkg/autoscaler/statforwarder/forwarder.go 90.74% <0.00%> (-5.56%) ⬇️
pkg/autoscaler/statforwarder/processor.go 88.88% <0.00%> (-5.56%) ⬇️
cmd/autoscaler/main.go 11.92% <0.00%> (ø)
pkg/autoscaler/metrics/collector.go 98.59% <0.00%> (ø)
pkg/autoscaler/metrics/stats_scraper.go 91.44% <0.00%> (+0.23%) ⬆️
pkg/autoscaler/statforwarder/leases.go 78.41% <0.00%> (+1.43%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7931069...2d4834c. Read the comment docs.

@leetcope
Copy link
Contributor Author

leetcope commented Mar 30, 2021

/test pull-knative-serving-istio-stable-no-mesh

@leetcope
Copy link
Contributor Author

leetcope commented Mar 30, 2021

/assign @dprotaso

dprotaso
dprotaso reviewed Mar 31, 2021
View reviewed changes
pkg/apis/serving/v1/revision_types.go Outdated
Comment on lines +161 to +163
// ActualReplicas reflects the PodAutoScalerStatus.ActualScale value in RevisionStatus.
ActualReplicas int32 `json:"actualReplicas,omitempty"`
// DesiredReplicas reflects the PodAutoScalerStatus.DesiredScale value in RevisionStatus.
Copy link
Member

@dprotaso dprotaso Mar 31, 2021

Choose a reason for hiding this comment

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

Assume user's/clients won't know what the PodAutoscaler is - so I'd change this comment to be explicit about what this number represents. ie. whether does it include terminating replicas etc..

Can we also add an the optional annotation to both these fields

// +optional

@dprotaso dprotaso mentioned this pull request Mar 31, 2021
Merged
@leetcope
Copy link
Contributor Author

leetcope commented Mar 31, 2021

/test pull-knative-serving-istio-stable-no-mesh

dprotaso
dprotaso approved these changes Apr 1, 2021
View reviewed changes
Copy link
Member

@dprotaso dprotaso left a comment

Choose a reason for hiding this comment

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

/lgtm
/hold Just to get a second pair of eyes on this

@knative-prow-robot knative-prow-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. labels Apr 1, 2021
@knative-prow-robot
Copy link
Contributor

knative-prow-robot commented Apr 1, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprotaso

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 1, 2021
vagababov
vagababov reviewed Apr 1, 2021
View reviewed changes
Copy link
Contributor

@vagababov vagababov left a comment

Choose a reason for hiding this comment

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

/lgtm

@dprotaso
Copy link
Member

dprotaso commented Apr 1, 2021

/hold cancel

@knative-prow-robot knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 1, 2021
@knative-prow-robot knative-prow-robot merged commit abf4d9d into knative:main Apr 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dprotaso dprotaso dprotaso approved these changes

+1 more reviewer

@vagababov vagababov vagababov left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@dprotaso dprotaso

@vagababov vagababov

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/API API objects and controllers cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Expose number of replicas in Revision's status

4 participants

@leetcope @knative-prow-robot @dprotaso @vagababov