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

Adds detail to initialScale validation error msgs #12704

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
merged 2 commits into from
Mar 8, 2022
Merged

Adds detail to initialScale validation error msgs #12704

merged 2 commits into from
Mar 8, 2022

Conversation

psschwei
Copy link
Contributor

@psschwei psschwei commented Mar 7, 2022

Signed-off-by: Paul S. Schweigert paulschw@us.ibm.com

Proposed Changes

Originally, validateInitialScale returned an ErrInvalidValue for any
invalid value. This PR adds more specific error messages for the cases
when either a negative value is used or if initialScale=0 when not
allowed by the cluster.

Release Note

Provides more detailed error messages for invalid values of autoscaling.knative.dev/initial-scale

@psschwei
adds detail to initialScale validation error msgs
cbbcacf 
Signed-off-by: Paul S. Schweigert <paulschw@us.ibm.com>

Originally, validateInitialScale returned an ErrInvalidValue for any
invalid value. This PR adds more specific error messages for the cases
when either a negative value is used or if initialScale=0 when not
allowed by the cluster.
@knative-prow-robot knative-prow-robot added area/API API objects and controllers area/autoscale size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 7, 2022
@dprotaso
Copy link
Member

dprotaso commented Mar 8, 2022

unit tests are failures are legit - error messages need updating

@psschwei
Copy link
Contributor Author

psschwei commented Mar 8, 2022

That's what I get for only running the unit buckets in the same directory :)

@psschwei
updating errors in other places
4be61b3 
Signed-off-by: Paul S. Schweigert <paulschw@us.ibm.com>
@codecov
Copy link

codecov bot commented Mar 8, 2022 •
edited
Loading

Codecov Report

Merging #12704 (4be61b3) into main (d59e273) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #12704   +/-   ##
=======================================
  Coverage   87.30%   87.30%           
=======================================
  Files         196      196           
  Lines        9741     9745    +4     
=======================================
+ Hits         8504     8508    +4     
- Misses        951      952    +1     
+ Partials      286      285    -1
Impacted Files Coverage Δ
pkg/apis/autoscaling/annotation_validation.go 100.00% <100.00%> (ø)
pkg/autoscaler/statserver/server.go 77.66% <0.00%> (-1.95%) ⬇️
pkg/reconciler/configuration/configuration.go 86.15% <0.00%> (+1.53%) ⬆️

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 d59e273...4be61b3. Read the comment docs.

@dprotaso
Copy link
Member

dprotaso commented Mar 8, 2022

it happens and that's what presubmits are for :)

/lgtm
/approve

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 8, 2022
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprotaso, 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:

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 Mar 8, 2022
@nader-ziada
Copy link
Member

/lgtm

@knative-prow-robot knative-prow-robot merged commit 25ce427 into knative:main Mar 8, 2022
@psschwei psschwei deleted the expressive-validatation branch March 10, 2022 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@dprotaso dprotaso

@nader-ziada nader-ziada

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/API API objects and controllers area/autoscale lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@psschwei @dprotaso @knative-prow-robot @nader-ziada